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

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Now that I had ever more time to think about this patch, I see a great potential in it for development use, for example, we could silence a checker before splitting it up to see whether we could disable it altogether or really having to go through the process of splitting it into a modeling and reporting portion.



================
Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:303
 // Static Analyzer Core
-def err_unknown_analyzer_checker : Error<
+def err_unknown_analyzer_checker_or_package : Error<
     "no analyzer checkers or packages are associated with '%0'">;
----------------
Cheers!


================
Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:167
   static std::vector<StringRef>
-  getRegisteredCheckers(bool IncludeExperimental = false);
+  getRegisteredCheckers(bool IncludeExperimental = false) {
+    static const StringRef StaticAnalyzerChecks[] = {
----------------
This has been bugging me for a while. Registered checkers are checkers that were, well, registered into the analyzer, that may also include plugins. This should rather be called `getBuiltinCheckers`, because it only retrieves the list of checkers declared in `Checkers.td`.

This is just thinking aloud, not related to your revision.


================
Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:177-178
+    for (StringRef CheckerName : StaticAnalyzerChecks) {
+      if (!CheckerName.startswith("debug.") &&
+          (IncludeExperimental || !CheckerName.startswith("alpha.")))
+        Checkers.push_back(CheckerName);
----------------
I wonder why the decision was made to hide debug checkers for clang-tidy. It so happened once that I wanted to enable one through clang-tidy's interface, but couldn't.

But this isn't the time to change it, just again, thinking aloud.


================
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;
----------------
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.


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

https://reviews.llvm.org/D66042