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

Manuel Klimek klimek at google.com
Thu Dec 19 11:07:40 PST 2013



================
Comment at: clang-tidy/ClangTidy.h:90
@@ +89,3 @@
+/// filters are applied.
+void getCheckNames(StringRef EnableChecksRegex, StringRef DisableChecksRegex,
+                   SmallVectorImpl<std::string> &CheckNames);
----------------
Alexander Kornienko wrote:
> 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.
Well, the reason is that a return-by-value interface is substantially simpler: you don't need to explain to the caller what happens if the handed in vector is not empty before, and it often makes the call-sites simpler, which I think is more important than making the implementation simpler. So I pretty strongly vote for returning things by-value unless there's a compelling reason not to do so.

================
Comment at: clang-tidy/ClangTidyModule.h:70
@@ +69,3 @@
+/// \brief Filters checks by name.
+class ChecksFilter {
+public:
----------------
Alexander Kornienko wrote:
> 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?
I think that the upside of "hiding" the implementation is not worth the extra cost of an interface here. To me, the default is not to introduce inheritance until we need at least 2 different implementations.


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



More information about the cfe-commits mailing list