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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
