NoQ added inline comments.
================
Comment at: lib/Analysis/CloneDetection.cpp:91
@@ +90,3 @@
+ ASTContext &Context;
+ std::vector<unsigned> &CollectedData;
+
----------------
Maybe it's a good idea to introduce a typedef for `unsigned` here, because it'd
be nice to be able to change it (eg., some day we may desire to switch to
uint64_t).
================
Comment at: lib/Analysis/CloneDetection.cpp:119
@@ +118,3 @@
+ // with zeroes.
+ if (Str.size() % sizeof(unsigned))
+ CollectedData.push_back(0);
----------------
Maybe just allocate the right amount of bytes during resize? Some usual trick
with rounding up, eg. `(x + y - 1) / y`.
================
Comment at: lib/Analysis/CloneDetection.cpp:134
@@ +133,3 @@
+ DEF_ADD_DATA(Stmt, { addData(S->getStmtClass()); })
+ DEF_ADD_DATA(Expr, { addData(S->getType()); })
+
----------------
I noticed something: with this patch, you no longer consider the kind of the
statement for supported statement types (even for, say, `Expr`), because the
visitor doesn't automatically fall back to parent classes when a method for the
child class is defined. Is this intentional? Or you may want to visit some
statements with parent methods manually.
================
Comment at: lib/Analysis/CloneDetection.cpp:148
@@ +147,3 @@
+ DEF_ADD_DATA(CallExpr,
+ { addData(S->getDirectCallee()->getQualifiedNameAsString()); })
+
----------------
I wonder how much we can save if we use a few pointers-based hashes instead of
hashing long fully-qualified names, for stuff like function decls or types.
Uhm, by the way, in fact C++ overloaded functions have same qualified names, do
we consider that? I guess sub-statement hashing would most likely take care of
that.
================
Comment at: test/Analysis/copypaste/test-asm.cpp:1
@@ +1,2 @@
+// RUN: %clang_cc1 -analyze -std=c++11
-analyzer-checker=alpha.clone.CloneChecker -verify %s
+
----------------
Do we really need to prefix all test file names with "`test-`"? It's kind of
obvious :)
https://reviews.llvm.org/D22514
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
