[analyzer] List checkers in 3 categories: released, alpha, developer

[Szelethus]

Summary

Implementation of the proposed checker categories. Quote from the mail:

-analyzer-checker-help: Displays production ready (non-modeling) checkers, and beta checkers with a disclaimer for each description. Don't forget to patch clang-tidy!
-analyzer-checker-help-developer: Displays only developer (modeling, debug) checkers (renamed from *-hidden).
-analyzer-checker-help-alpha: Displays only alpha (non-modeling) checkers with very scary disclaimers both around the list and for each checker description.

Note how each of these is mutually exclusive. I think we shouldn't make a flag that displays all checkers, but should allow the following invocation to do so:

clang -cc1 -analyzer-checker-help -analyzer-checker-help-alpha -analyzer-checker-help-developer

Came to realize that this won't affect clang-tidy at all.

[Szelethus]

Let's come to a consensus on this, and followup patches, but as soon as that's done, this REALLY NEEDS to be documented in the release notes.

[NoQ]

Looks great, thanks!

lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

526 ↗

(On Diff #200125)

this -> these :)

527–530 ↗

(On Diff #200125)

Hmm, just thought about this: by this logic, should we hide all core checkers? Because we don't really support users tinkering with whether they should be enabled :)

[Szelethus]

lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

527–530 ↗

(On Diff #200125)

Thats actually a very cool idea. We should totally do that!

[Szelethus]

Removed the beta phase as discussed on the mailing list.

[Szelethus]

lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

527–530 ↗

(On Diff #200125)

Well umm. In a sense, sure, but wouldn't we want to display the checker descriptions at least? Let's not rush this for now.

[NoQ]

Thx!~

lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

527–530 ↗

(On Diff #200125)

Yup. I guess we just separate models from checkers eventually.

[Szelethus]

rC361566 is incorrectly marked for this revision instead of D62094.

cfe/trunk/include/clang/Driver/CC1Options.td

def analyzer_disable_checker_EQ : Joined<["-"], "analyzer-disable-checker=">,
   Alias<analyzer_disable_checker>;
 
 def analyzer_disable_all_checks : Flag<["-"], "analyzer-disable-all-checks">,
   HelpText<"Disable all static analyzer checks">;
 
 def analyzer_checker_help : Flag<["-"], "analyzer-checker-help">,
   HelpText<"Display the list of analyzer checkers that are available">;
 
-def analyzer_checker_help_hidden : Flag<["-"], "analyzer-checker-help-hidden">,
-  HelpText<"Display the list of analyzer checkers that are available, "
-           "including modeling checkers">;
+def analyzer_checker_help_alpha : Flag<["-"], "analyzer-checker-help-alpha">,
+  HelpText<"Display the list of in development analyzer checkers. These "
+           "are NOT considered safe, they are unstable and will emit incorrect "
+           "reports. Enable ONLY FOR DEVELOPMENT purposes">;
+
+def analyzer_checker_help_developer : Flag<["-"], "analyzer-checker-help-developer">,
+  HelpText<"Display the list of developer-only checkers such as modeling "
+           "and debug checkers">;
 
 def analyzer_config_help : Flag<["-"], "analyzer-config-help">,
   HelpText<"Display the list of -analyzer-config options">;
 
 def analyzer_list_enabled_checkers : Flag<["-"], "analyzer-list-enabled-checkers">,
   HelpText<"Display the list of enabled analyzer checkers">;
 
 def analyzer_config : Separate<["-"], "analyzer-config">,

cfe/trunk/include/clang/StaticAnalyzer/Checkers/CheckerBase.td

 ///   def InnerPointerChecker : Checker<"InnerPointer">,
 ///     HelpText<"Check for inner pointers of C++ containers used after "
 ///              "re/deallocation">,
 ///     Dependencies<[MallocBase]>;
 class Dependencies<list<Checker> Deps = []> {
   list<Checker> Dependencies = Deps;
 }
 
-/// Marks a checker or a package hidden. Hidden entries won't be displayed in
-/// -analyzer-checker-help, which is desirable for alpha or modeling checkers.
+/// Marks a checker or a package hidden. Hidden entries are meant for developers
+/// only, and aren't exposed to end users.
 class Hidden { bit Hidden = 1; }

cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

public:
   /// Disable all analyzer checks.
   ///
   /// This flag allows one to disable analyzer checks on the code processed by
   /// the given analysis consumer. Note, the code will get parsed and the
   /// command-line options will get checked.
   unsigned DisableAllChecks : 1;
 
   unsigned ShowCheckerHelp : 1;
-  unsigned ShowCheckerHelpHidden : 1;
+  unsigned ShowCheckerHelpAlpha : 1;
+  unsigned ShowCheckerHelpDeveloper : 1;
   unsigned ShowEnabledCheckerList : 1;
   unsigned ShowCheckerOptionList : 1;
   unsigned ShowConfigOptionsList : 1;
   unsigned ShouldEmitErrorsOnInvalidConfigValue : 1;
   unsigned AnalyzeAll : 1;
   unsigned AnalyzerDisplayProgress : 1;
   unsigned AnalyzeNestedBlocks : 1;
 
assert(std::is_sorted(AnalyzerConfigCmdFlags.begin(),
                           AnalyzerConfigCmdFlags.end()));
 
     return !std::binary_search(AnalyzerConfigCmdFlags.begin(),
                                AnalyzerConfigCmdFlags.end(), Name);
   }
 
   AnalyzerOptions()
       : DisableAllChecks(false), ShowCheckerHelp(false),
-        ShowCheckerHelpHidden(false), ShowEnabledCheckerList(false),
-        ShowCheckerOptionList(false), ShowConfigOptionsList(false),
-        AnalyzeAll(false), AnalyzerDisplayProgress(false),
-        AnalyzeNestedBlocks(false), eagerlyAssumeBinOpBifurcation(false),
-        TrimGraph(false), visualizeExplodedGraphWithGraphViz(false),
-        UnoptimizedCFG(false), PrintStats(false), NoRetryExhausted(false) {
+        ShowCheckerHelpAlpha(false), ShowCheckerHelpDeveloper(false),
+        ShowEnabledCheckerList(false), ShowCheckerOptionList(false),
+        ShowConfigOptionsList(false), AnalyzeAll(false),
+        AnalyzerDisplayProgress(false), AnalyzeNestedBlocks(false),
+        eagerlyAssumeBinOpBifurcation(false), TrimGraph(false),
+        visualizeExplodedGraphWithGraphViz(false), UnoptimizedCFG(false),
+        PrintStats(false), NoRetryExhausted(false) {
     llvm::sort(AnalyzerConfigCmdFlags);
   }
 
   /// Interprets an option's string value as a boolean. The "true" string is
   /// interpreted as true and the "false" string is interpreted as false.
   ///
   /// If an option value is not provided, returns the given \p DefaultVal.
   /// @param [in] CheckerName The *full name* of the checker. One may retrieve

cfe/trunk/lib/Frontend/CompilerInvocation.cpp

if (Value == NumInliningModes) {
         << A->getAsString(Args) << Name;
       Success = false;
     } else {
       Opts.InliningMode = Value;
     }
   }
 
   Opts.ShowCheckerHelp = Args.hasArg(OPT_analyzer_checker_help);
-  Opts.ShowCheckerHelpHidden = Args.hasArg(OPT_analyzer_checker_help_hidden);
+  Opts.ShowCheckerHelpAlpha = Args.hasArg(OPT_analyzer_checker_help_alpha);
+  Opts.ShowCheckerHelpDeveloper =
+      Args.hasArg(OPT_analyzer_checker_help_developer);
   Opts.ShowCheckerOptionList = Args.hasArg(OPT_analyzer_checker_option_help);
   Opts.ShowConfigOptionsList = Args.hasArg(OPT_analyzer_config_help);
   Opts.ShowEnabledCheckerList = Args.hasArg(OPT_analyzer_list_enabled_checkers);
   Opts.ShouldEmitErrorsOnInvalidConfigValue =
       /* negated */!llvm::StringSwitch<bool>(
                    Args.getLastArgValue(OPT_analyzer_config_compatibility_mode))
         .Case("true", true)
         .Case("false", false)

cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp

if (!Clang->getFrontendOpts().LLVMArgs.empty()) {
     llvm::cl::ParseCommandLineOptions(NumArgs + 1, Args.get());
   }
 
 #if CLANG_ENABLE_STATIC_ANALYZER
   // These should happen AFTER plugins have been loaded!
 
   AnalyzerOptions &AnOpts = *Clang->getAnalyzerOpts();
   // Honor -analyzer-checker-help and -analyzer-checker-help-hidden.
-  if (AnOpts.ShowCheckerHelp || AnOpts.ShowCheckerHelpHidden) {
+  if (AnOpts.ShowCheckerHelp || AnOpts.ShowCheckerHelpAlpha ||
+      AnOpts.ShowCheckerHelpDeveloper) {
     ento::printCheckerHelp(llvm::outs(),
                            Clang->getFrontendOpts().Plugins,
                            AnOpts,
                            Clang->getDiagnostics(),
                            Clang->getLangOpts());
     return true;
   }
 

cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

for (const auto &Checker : Checkers) {
     // Limit the amount of padding we are willing to give up for alignment.
     //   Package.Name     Description  [Hidden]
     size_t NameLength = Checker.FullName.size();
     if (NameLength <= MaxNameChars)
       OptionFieldWidth = std::max(OptionFieldWidth, NameLength);
   }
 
   const size_t InitialPad = 2;
-  for (const auto &Checker : Checkers) {
-    if (!AnOpts.ShowCheckerHelpHidden && Checker.IsHidden)
-      continue;
 
-    AnalyzerOptions::printFormattedEntry(Out, {Checker.FullName, Checker.Desc},
+  auto Print = [=](llvm::raw_ostream &Out, const CheckerInfo &Checker,
+                   StringRef Description) {
+    AnalyzerOptions::printFormattedEntry(Out, {Checker.FullName, Description},
                                          InitialPad, OptionFieldWidth);
     Out << '\n';
+  };
+
+  for (const auto &Checker : Checkers) {
+    // The order of this if branches is significant, we wouldn't like to display
+    // developer checkers even in the alpha output. For example,
+    // alpha.cplusplus.IteratorModeling is a modeling checker, hence it's hidden
+    // by default, and users (even when the user is a developer of an alpha
+    // checker) shouldn't normally tinker with whether they should be enabled.
+
+    if (Checker.IsHidden) {
+      if (AnOpts.ShowCheckerHelpDeveloper)
+        Print(Out, Checker, Checker.Desc);
+      continue;
+    }
+
+    if (Checker.FullName.startswith("alpha")) {
+      if (AnOpts.ShowCheckerHelpAlpha)
+        Print(Out, Checker,
+              ("(Enable only for development!) " + Checker.Desc).str());
+      continue;
+    }
+
+    if (AnOpts.ShowCheckerHelp)
+        Print(Out, Checker, Checker.Desc);
   }
 }
 
 void CheckerRegistry::printEnabledCheckerList(raw_ostream &Out) const {
   // Collect checkers enabled by the options.
   CheckerInfoSet EnabledCheckers = getEnabledCheckers();
 
   for (const auto *i : EnabledCheckers)

cfe/trunk/test/Analysis/show-checker-list.c

 // RUN: %clang_cc1 -analyzer-checker-help \
-// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-STABLE
 
-// RUN: %clang_cc1 -analyzer-checker-help-hidden \
-// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-HIDDEN
+// RUN: %clang_cc1 -analyzer-checker-help-alpha \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-ALPHA
 
-// CHECK: core.DivideZero
-// CHECK-HIDDEN: core.DivideZero
+// RUN: %clang_cc1 -analyzer-checker-help-developer \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-DEVELOPER
 
-// CHECK-NOT: unix.DynamicMemoryModeling
-// CHECK-HIDDEN: unix.DynamicMemoryModeling
+// RUN: %clang_cc1 -analyzer-checker-help-developer \
+// RUN:   -analyzer-checker-help-alpha \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-DEVELOPER-ALPHA
+
+// RUN: %clang_cc1 -analyzer-checker-help \
+// RUN:   -analyzer-checker-help-alpha \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-STABLE-ALPHA
+
+// RUN: %clang_cc1 -analyzer-checker-help \
+// RUN:   -analyzer-checker-help-developer \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-STABLE-DEVELOPER
+
+// RUN: %clang_cc1 -analyzer-checker-help \
+// RUN:   -analyzer-checker-help-alpha \
+// RUN:   -analyzer-checker-help-developer \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-STABLE-ALPHA-DEVELOPER
+
+// CHECK-STABLE-NOT:    alpha.unix.Chroot
+// CHECK-DEVELOPER-NOT: alpha.unix.Chroot
+// CHECK-ALPHA:         alpha.unix.Chroot
+
+// Note that alpha.cplusplus.IteratorModeling is not only an alpha, but also a
+// hidden checker. In this case, we'd only like to see it in the developer list.
+// CHECK-ALPHA-NOT: alpha.cplusplus.IteratorModeling
+// CHECK-DEVELOPER: alpha.cplusplus.IteratorModeling
+
+// CHECK-STABLE:        core.DivideZero
+// CHECK-DEVELOPER-NOT: core.DivideZero
+// CHECK-ALPHA-NOT:     core.DivideZero
+
+// CHECK-STABLE-NOT: debug.ConfigDumper
+// CHECK-DEVELOPER:  debug.ConfigDumper
+// CHECK-ALPHA-NOT:  debug.ConfigDumper
+
+
+// CHECK-STABLE-ALPHA:         alpha.unix.Chroot
+// CHECK-DEVELOPER-ALPHA:      alpha.unix.Chroot
+// CHECK-STABLE-DEVELOPER-NOT: alpha.unix.Chroot
+
+// CHECK-STABLE-ALPHA:        core.DivideZero
+// CHECK-DEVELOPER-ALPHA-NOT: core.DivideZero
+// CHECK-STABLE-DEVELOPER:    core.DivideZero
+
+// CHECK-STABLE-ALPHA-NOT: debug.ConfigDumper
+// CHECK-DEVELOPER-ALPHA:  debug.ConfigDumper
+// CHECK-STABLE-DEVELOPER: debug.ConfigDumper
+
+
+// CHECK-STABLE-ALPHA-DEVELOPER: alpha.unix.Chroot
+// CHECK-STABLE-ALPHA-DEVELOPER: core.DivideZero
+// CHECK-STABLE-ALPHA-DEVELOPER: debug.ConfigDumper