[PATCH] D20795: Added basic capabilities to detect source code clones.

[Artem Dergachev via cfe-commits]

NoQ added a comment.

The idea with strings as keys is curious! I've got a few more minor comments :)


================
Comment at: lib/Analysis/CloneDetection.cpp:104
@@ +103,3 @@
+    // Storage for the signatures of the direct child statements. This is only
+    // needed if the current statemnt is a CompoundStmt.
+    std::vector<CloneDetector::CloneSignature> ChildSignatures;
----------------
Typo: statement.

================
Comment at: lib/Analysis/CloneDetection.cpp:147
@@ +146,3 @@
+      const CompoundStmt *CS,
+      std::vector<CloneDetector::CloneSignature> ChildSignatures) {
+
----------------
Maybe pass `ChildSignatures` by reference?

================
Comment at: lib/Analysis/CloneDetection.cpp:178
@@ +177,3 @@
+
+  bool VisitFunctionDecl(FunctionDecl *D) {
+    // If we found a function, we start the clone search on its body statement.
----------------
You'd probably want to add `ObjCMethodDecl` and `BlockDecl`, because they aren't sub-classes of `FunctionDecl` (and probably even tests for that).

Because this part of the code essentially re-implements `AnalysisConsumer`, (a `RecursiveASTVisitor` that tells us what code bodies to analyze with the static analyzer - i should've looked there earlier!!).

Alternatively, you can just rely on `AnalysisConsumer`, which would eliminate the recursive visitor completely: {F2205103} This way you'd be analyzing exactly the same code bodies that the analyzer itself would analyze; if you'd want to extend to various declarations, you'd be able to do that by subscribing on `check::ASTDecl`. But i dare not to predict what kind of different flexibility you'd need next.

================
Comment at: lib/Analysis/CloneDetection.cpp:188
@@ +187,3 @@
+void CloneDetector::analyzeTranslationUnit(TranslationUnitDecl *TU) {
+  DataCollectVisitor visitor(*this, TU->getASTContext());
+  visitor.TraverseDecl(TU);
----------------
Micro-nit: Capitalize 'V' in 'visitor'?


https://reviews.llvm.org/D20795