NoQ 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; ---------------- 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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66042/new/ https://reviews.llvm.org/D66042 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits