[PATCH] D22514: CloneDetection now respects statement specific data when searching for clones.

[Vassil Vassilev via cfe-commits]

v.g.vassilev added a comment.

Could we stress test the implementation by running on files from llvm's/clang's test suite. For example, for a test TST we could do something like: cat TST >> tmp_TST;  cat TST >> tmp_TST and run our our copy-paste checker on tmp_TST. This way we know how many clones to expect and we can cross-check how many we are able to detect.


================
Comment at: lib/Analysis/CloneDetection.cpp:98
@@ +97,3 @@
+  ///        FoldingSetNodeID.
+  StmtDataCollector(Stmt const *S, ASTContext &Context,
+                    llvm::FoldingSetNodeID &D) : Context(Context), D(D) {
----------------
I'd prefer to see `const Stmt *S`. Could you please edit all occurrences?

================
Comment at: lib/Analysis/CloneDetection.cpp:109
@@ +108,3 @@
+
+  void addData(llvm::StringRef const &String) {
+    D.AddString(String);
----------------
Use just `llvm::StringRef`, we don't need `const &`.

================
Comment at: lib/Analysis/CloneDetection.cpp:113
@@ +112,3 @@
+
+  void addData(std::string const &String) {
+    D.AddString(String);
----------------
Same here, `const std::string &String`. Is this caught by clang-format?

================
Comment at: test/Analysis/copypaste/test-catch.cpp:29
@@ +28,2 @@
+  return true;
+}
----------------
Could you add an example like:

```
void non_compound_stmt1(int x)
  try { x / 0; } catch(...) { x++; }

void non_compound_stmt2(int x, float z)
  try { z / 0; } catch(...) { x++; }
```



https://reviews.llvm.org/D22514