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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
