[PATCH] D66042: [analyzer] Analysis: Silence checkers

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:483-504
+  if (!AnOpts.RawSilencedCheckersAndPackages.empty()) {
+    std::vector<StringRef> Checkers =
+        AnOpts.getRegisteredCheckers(/*IncludeExperimental=*/true);
+    std::vector<StringRef> Packages =
+        AnOpts.getRegisteredPackages(/*IncludeExperimental=*/true);
+
+    SmallVector<StringRef, 16> CheckersAndPackages;
----------------
NoQ wrote:
> Charusso wrote:
> > Szelethus wrote:
> > > Szelethus wrote:
> > > > Charusso wrote:
> > > > > Szelethus wrote:
> > > > > > Szelethus wrote:
> > > > > > > Szelethus wrote:
> > > > > > > > The reason why I suggested validating this in CheckerRegistry is that CheckerRegistry is the only class knowing the actual list of checkers and packages, and is able to emit diagnostics before the analysis starts. This solution wouldn't work with plugin checkers/packages.
> > > > > > > I don't see this being addressed actually?
> > > > > > > 
> > > > > > > I think it would be totally fine to just omit the validation part as I said earlier, the patch will be leaner, and cases in which we're using the silencing of checkers are very exotic anyways.
> > > > > > Also, we should probably compliment such validation by actually writing tests for plugins.
> > > > > > 
> > > > > > I've been through that process once. It isn't fun. Really-really isn't :^) Let's just collectively agree to "forget" this :)
> > > > > Checker validation is in `CheckerRegistry`, configuration validation is in `parseAnalyzerConfigs()`. I have made a configuration, rather than a checker flag, so that I have not found more appropriate place and its does the job well.  If it will be needed externally, I hope someone could do better.
> > > > Well isn't this checker validation?
> > > In any case, could just throw in a FIXME before commiting please? :)
> > @NoQ, does the `AnalyzerOptions::getRegisteredCheckers()` contain the plugins?
> //*/me doesn't know much about plugins*//
> 
> Normally enable-disable flags do work on plugins, so plugin checkers must make it into the registry at some point.
So do we need a FIXME or most likely it working with plugins?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66042/new/

https://reviews.llvm.org/D66042