alexfh added inline comments.

================
Comment at: clang-tidy/ClangTidyOptions.cpp:269
@@ +268,3 @@
+          ->CheckSources[ClangTidyOptions::ConfigCommandlineOptionOrFile]
+          .push_back(ClangTidyOptions::StringPair(*ParsedOptions->Checks,
+                                                  ConfigFile.c_str()));
----------------
Can we use `.emplace_back` here?

================
Comment at: clang-tidy/ClangTidyOptions.h:91
@@ +90,3 @@
+  // The priority from low to high:
+  //   DefaultBinary > ConfigCommandlineOptionOrFile > ChecksCommandlineOption
+  enum CheckSourceType {
----------------
If it's from low to high, I'd use `<` instead of `>` to avoid confusion ;)

================
Comment at: clang-tidy/ClangTidyOptions.h:99
@@ +98,3 @@
+
+  // \brief Stores each check filter and its source for every check source 
type.
+  // clang-tidy has 3 types of check sources:
----------------
1. Use `///` for doxygen comments.
2. Insert an empty comment line after this line to mark the end of the `\brief` 
block.
3. Mark the list items with `*`.

================
Comment at: clang-tidy/ClangTidyOptions.h:104
@@ +103,3 @@
+  //    '-checks' commandline option
+  std::vector<StringPair> CheckSources[CheckSourceTypeEnd];
+
----------------
Can we store a single vector of string pairs instead of three different 
vectors? The second item of the pair already says enough about the source. And 
this being an array doesn't seem to be useful.

================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:281
@@ +280,3 @@
+        .push_back(ClangTidyOptions::StringPair(
+            Checks, "command-line option '-checks'"));
+  }
----------------
Please pull this string literal to a constant: it's used more than once in the 
code. Maybe also pull the "clang-tidy binary" string too, for consistency.


http://reviews.llvm.org/D18694



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

Reply via email to