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

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 25 07:33:35 PST 2016


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





More information about the cfe-commits mailing list