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

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 13 05:31:30 PDT 2016


alexfh requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:41
@@ +40,3 @@
+
+  const auto PlusOperatorMatcher =
+      cxxOperatorCallExpr(
----------------
s/Matcher//

================
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:48
@@ +47,3 @@
+
+  const auto AssingOperator = cxxOperatorCallExpr(
+      hasOverloadedOperatorName("="), hasDescendant(BasicStringPlusOperator),
----------------
s/Assing/Assign/

================
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:57
@@ +56,3 @@
+
+  if (!StrictMode) {
+    Finder->addMatcher(
----------------
Please avoid unnecessary negation by putting the positive branch first, this will make the logic slightly simpler.

================
Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:67
@@ +66,3 @@
+    Finder->addMatcher(
+        exprWithCleanups(anyOf(hasDescendant(AssingOperator),
+                               hasDescendant(PlusOperatorMatcher))),
----------------
1. The `anyOf(hasAncestor(A), hasAncestor(B), ...)` construct is still there. Please replace it with `hasAncestor(anyOf(A, B, ...))`.
2. Is there really no way to change from hasDescendant / hasAncestor to more strict patterns? I believe, running the check on LLVM doesn't help finding performance issues, since LLVM specifically avoids this pattern by using Twine.


http://reviews.llvm.org/D20196





More information about the cfe-commits mailing list