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

[Raphael Isemann via cfe-commits]

teemperor added a comment.

Binary size increases from 33017160 Bytes to 33060464 Bytes (+40 KiB).

I'm not sure if that's too much for such a minor feature, so I've added two new revisions:

- One is the original patch with the other mentioned issues fixed (so, same +40 KiB size increase here).

- The other uses only one collect function which reduces the binary size increase to +2 KiB (but uses chained `if`'s with `dyn_cast`).

The problem is that the LLVM coding standard isn't a big fan chained `if`s with `dyn_cast` and recommends doing it in the same way as the original patch for performance reasons.


================
Comment at: lib/Analysis/CloneDetection.cpp:145
@@ +144,3 @@
+  }
+  #include "clang/AST/StmtNodes.inc"
+
----------------
v.g.vassilev wrote:
> Why do we need this? Wouldn't `callAddDataStmt` properly dispatch to the concrete method call?
`callAddDataFoo` would call all `addDataXXX` functions for `Foo` and all parent classes of `Foo`. So `callAddDataStmt` would only call `addDataStmt` because `Stmt` has no parent class. `callAddCompoundStmt` would call `addDataStmt` (through `callAddDataStmt`) and `addDataCompoundStmt` and so on.

================
Comment at: lib/Analysis/CloneDetection.cpp:306
@@ -112,1 +305,3 @@
     for (Stmt const *Child : Parent->children()) {
+      if (!Child)
+        continue;
----------------
v.g.vassilev wrote:
> In which case this happens?
For example
```
if (a < b) return 1;
``` 
would have two nullptr children in the AST (the missing 'init' and 'else' Stmts):


```
    `-IfStmt 0x2d595f8 <line:3:3, col:22>
      |-<<<NULL>>>
      |-BinaryOperator 0x2d59598 <col:8, col:12> '_Bool' '>'
      | |-ImplicitCastExpr 0x2d59568 <col:8> 'int' <LValueToRValue>
      | | `-DeclRefExpr 0x2d59518 <col:8> 'int' lvalue Var 0x2d59418 'a' 'int'
      | `-ImplicitCastExpr 0x2d59580 <col:12> 'int' <LValueToRValue>
      |   `-DeclRefExpr 0x2d59540 <col:12> 'int' lvalue Var 0x2d59488 'b' 'int'
      |-ReturnStmt 0x2d595e0 <col:15, col:22>
      | `-IntegerLiteral 0x2d595c0 <col:22> 'int' 1
      `-<<<NULL>>>
```


https://reviews.llvm.org/D22514