alexfh requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: clang-tidy/misc/SuspiciousStringCompareCheck.cpp:94
@@ +93,3 @@
+  std::vector<std::string> FunctionNames = utils::option::parseNames(
+      (llvm::Twine(KnownStringCompareFunctions) + StringCompareLikeFunctions)
+          .str());
----------------
Please add a comment that the `KnownStringCompareFunctions` string should end 
with a semicolon.

================
Comment at: clang-tidy/utils/OptionUtils.h:18
@@ +17,3 @@
+namespace utils {
+namespace option {
+
----------------
"options" would convey the idea slightly better, imo.

================
Comment at: clang-tidy/utils/OptionUtils.h:21
@@ +20,3 @@
+/// \brief Parse a list of names separated by ";" character.
+std::vector<std::string> parseNames(StringRef Option);
+
----------------
I don't think this code is specific to "names". Maybe `parseStrings`, 
`parseList` or `parseSemicolonSeparatedList`?

================
Comment at: clang-tidy/utils/OptionUtils.h:23
@@ +22,3 @@
+
+/// \brief Serialize a sequence of names that can be parse by parseNames.
+std::string serializeNames(const std::vector<std::string> &Names);
----------------
1. s/parse/parsed/
2. Please enclose parseName in backquotes.
3. It's more common to use `ArrayRef` from llvm/ADT/ArrayRef.h to pass a 
reference to a number of sequential elements.


http://reviews.llvm.org/D19846



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

Reply via email to