[PATCH] D16152: [clang-tidy] Add check performance-faster-string-find

Samuel Benzaquen via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 14 08:11:49 PST 2016


sbenza marked an inline comment as done.

================
Comment at: clang-tidy/performance/FasterStringFindCheck.cpp:51
@@ +50,3 @@
+  const auto StringFindFunctions =
+      anyOf(hasName("find"), hasName("rfind"), hasName("find_first_of"),
+            hasName("find_first_not_of"), hasName("find_last_of"),
----------------
alexfh wrote:
> Probably out of scope of this patch, but I wonder how many times `hasName` is still more effective than one `matchesName`? Maybe we should add a `matchesName` variant for unqualified names or hasName taking a list of strings?
I have no idea how much more expensive is the regex engine.
It also makes the code harder to read.
I would totally be in favor of a variadic hasAnyName(). It is shorter and can be much faster to run.

================
Comment at: clang-tidy/performance/FasterStringFindCheck.cpp:75
@@ +74,3 @@
+
+  diag(Literal->getLocStart(), "char overload is more efficient")
+      << FixItHint::CreateReplacement(
----------------
alexfh wrote:
> The message might be confusing in some situations (e.g. macros). I think, it should at least mention what method (and maybe of what class) is in question, e.g. "std::string::find() called with a string literal consisting of a single character; consider using the more effective overload accepting a character".
Used unqualified function name instead.
The qualified name for find is std::basic_string<blah, blah, blah>::find, which might be distracting.


http://reviews.llvm.org/D16152





More information about the cfe-commits mailing list