[PATCH] Clang-tidy: added --disable-checks, --list-checks options.

Alexander Kornienko alexfh at google.com
Thu Dec 19 10:45:15 PST 2013



================
Comment at: clang-tidy/ClangTidy.h:90
@@ +89,3 @@
+/// filters are applied.
+void getCheckNames(StringRef EnableChecksRegex, StringRef DisableChecksRegex,
+                   SmallVectorImpl<std::string> &CheckNames);
----------------
Manuel Klimek wrote:
> Any reason not to return by value?
It used to, but this required a bit more code (local variable + return in each of the two getCheckNames methods) for no real reason. I slightly prefer this variant.

================
Comment at: clang-tidy/ClangTidy.h:88
@@ -87,1 +87,3 @@
 
+/// \brief Fills the list of check names, that are enabled when the provided
+/// filters are applied.
----------------
Manuel Klimek wrote:
> Nit: s/,//
Done.

================
Comment at: clang-tidy/ClangTidy.cpp:112
@@ +111,3 @@
+    for (unsigned i = 0; i < Checkers.size(); ++i) {
+      std::string Checker(Twine(AnalyzerCheckerNamePrefix, Checkers[i]).str());
+      AnalyzerChecksEnabled |=
----------------
Manuel Klimek wrote:
> I'd slightly prefer using (... + ...).str() here (but feel free to leave as is).
Done.

================
Comment at: clang-tidy/ClangTidyModule.h:70
@@ +69,3 @@
+/// \brief Filters checks by name.
+class ChecksFilter {
+public:
----------------
Manuel Klimek wrote:
> Is there any reason to have this interface? It looks to me like we only ever create a single instance...
The actual implementation isn't important to the code that uses it, so I thought it's a good idea to leave only the interface here. Do you think it makes sense to move the actual class here?


http://llvm-reviews.chandlerc.com/D2444



More information about the cfe-commits mailing list