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

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: clang-tidy/performance/FasterStringFindCheck.cpp:30
@@ +29,3 @@
+    Class = Class.trim();
+    if (!Class.empty())
+      Result.push_back(Class);
----------------
alexfh wrote:
> aaron.ballman wrote:
> > > Also changed the separator to be ';' instead of ','.
> > > The latter could be part of a type name. Eg. Foo<A,B>::Bar
> > 
> > That's a great catch!
> > 
> > @alexfh -- do you think we should try to get our checkers to be consistent with their choice of list separators? Specifically, I am thinking about D16113. It seems like it would improve the user experience to not have to wonder "do I use comma, or semi colon, for this list?"
> We can try to be consistent with the choice of delimiters, but I'm not sure we can use the same delimiter always without introducing more syntax complexities like escaping or the use of quotes (like in CSV). In any case, we should clearly document the format for each option and think how we can issue diagnostics when we suspect incorrect format used in the option value.
I think we should strive for consistency with ";" for right now, and agree that documentation/assistance is a reasonable fallback.


http://reviews.llvm.org/D16152