[PATCH] D19846: [clang-tidy] Lift parsing of sequence of names functions to utils.

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Tue May 3 01:47:45 PDT 2016


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





More information about the cfe-commits mailing list