alexfh added inline comments.
================
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:50
@@ +49,3 @@
+ hasOverloadedOperatorName("="), hasDescendant(BasicStringPlusOperator),
+ allOf(hasArgument(
+ 0, allOf(declRefExpr(BasicStringType),
----------------
1. No need for `allOf`, node matchers are implicitly "all of".
2. `hasDescendant()` is potentially the most expensive part here and should go
after all other constraints.
================
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:68
@@ +67,3 @@
+ this);
+ }
+}
----------------
For 1: if `hasAncestor(anyOf(cxxForRangeStmt(), ...))` doesn't work, you can
try `hasAncestor(stmt(anyOf(cxxForRangeStmt(), ...))`. In any case, repeated
`hasAncestor` matchers don't make the check faster ;)
For 2: `exprWithCleanups` and other implicit nodes can be skipped using the
`ignoringImplicit()` matchers. Leave `hasAncestor` for the cases, where there
can actually be arbitrary nodes in the path.
Here you're matching some arbitrary intermediate node (`exprWithCleanups`) and
then you're constraining its parents and its children. This doesn't make much
sense and it's rather inefficient. Since you're interested in the operators in
the first place, try rearranging the matcher like this:
```
cxxOperatorCallExpr(
anyOf(AssignOperator, PlusOperator),
hasAncestor(stmt(anyOf(cxxForRangeStmt(), whileStmt(), forStmt()))))
```
http://reviews.llvm.org/D20196
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
