[PATCH] D34182: [analyzer] Performance optimizations for the CloneChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Totally makes sense :)



================
Comment at: include/clang/Analysis/CloneDetection.h:258-260
+/// This constraint is also available to be executed in two phases, see
+/// RecursiveCloneTypeIIHashConstraint and RecursiveCloneTypeIIVerifyConstraint
+/// for more.
----------------
Since this constraint is a trivial composition of the other two now, is it useful at all as a separate class? Maybe just remove it?


================
Comment at: include/clang/Analysis/CloneDetection.h:266-273
+/// This constraint performs only the hashing part of the
+/// RecursiveCloneTypeIIConstraint.
+///
+/// It is supposed to be fast and can be used at the front of the constraint
+/// chain. However, it has a tiny chance to generate false-positives where the
+/// clones in a clone group are not actually type II clones of each other.
+/// This happens only due to hash collisions and they can be removed by the
----------------
As a personal preference, i'd probably like to see a more straightforward answer to "what exactly does this do?", rather than "what is this part of?" and "how fast this is?".

Eg., "RecursiveCloneTypeIIHashConstraint computes a hash of each statement sequence; sequences with different hash values are moved into separate clone groups. Collisions are possible, and this constraint does nothing to address this them. Add the slower RecursiveCloneTypeIIVerifyConstraint later in the constraint chain, not necessarily immediately, to eliminate hash collisions through a more detailed analysis."


https://reviews.llvm.org/D34182