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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to