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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to