[PATCH] D18694: [ClangTidy] Add an 'explain-checks' option to diagnose where each checks comes from.

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 7 10:55:24 PDT 2016


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





More information about the cfe-commits mailing list