teemperor added a comment.
Very well done, I really like this patch! I added a few remarks mostly about
the comments that need some small adjusting.
I'm wondering what would be a nice way of creating a StmtDataCollector that is
faster but only works for single translation units (e.g. it only hashes the
pointer values of decls instead of their qualified name)? The use case here
would be Stmt::Profile and the CloneDetector which could be set to a
single-TU-mode in the CloneChecker. For Stmt::Profile it would ensure we don't
degrade performance from the current version, for the CloneChecker we probably
get a reasonable performance boost (as the hashing is currently the last
remaining bottle neck).
@arphaman Any suggestions who could review/approve the additions to `AST/`?
================
Comment at: include/clang/AST/DataCollection.h:19
+
+namespace clang {
+
----------------
I think we need another namespace here because I don't like a generic function
name like `addDataToConsumer` in the `clang` namespace. Maybe
`clang::data_collection::addDataToConsumer` or something like that.
================
Comment at: lib/AST/StmtDataCollectors.inc:5
+ addData(S->getStmtClass());
+ // This ensures that macro generated code isn't identical to macro-generated
+ // code.
----------------
I know this is from my code, but as you're anyway touching this line, could you
fix this comment? It should be "that non-macro-generated code` instead of `that
macro generated`. Thanks!
================
Comment at: lib/Analysis/CloneDetection.cpp:172
+/// Collects the data of a single Stmt.
+///
----------------
I think this line can be removed now.
================
Comment at: lib/Analysis/CloneDetection.cpp:174
+///
+/// This class defines what a code clone is: If it collects for two statements
+/// the same data, then those two statements are considered to be clones of
each
----------------
`what a code clone is ` -> `what a type II code clone is`
================
Comment at: lib/Analysis/CloneDetection.cpp:210
+
+// Override the definition for some visitors.
+#define SKIP(CLASS)
\
----------------
Should be `Type II clones ignore variable names and literals, so let's skip
them`
https://reviews.llvm.org/D36664
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
