[PATCH] D20196: [clang-tidy] Inefficient string operation

[Alexander Kornienko via cfe-commits]

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