Reland "[analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow …

…CheckerManager to be constructed for non-analysis purposes"

Originally commited in rG57b8a407493c34c3680e7e1e4cb82e097f43744a, but
it broke the modules bot. This is solved by putting the contructors of
the CheckerManager class to the Frontend library.

Differential Revision: https://reviews.llvm.org/D75360

clang/include/clang/StaticAnalyzer/Core/CheckerManager.h

@@ -14,6 +14,7 @@
 #define LLVM_CLANG_STATICANALYZER_CORE_CHECKERMANAGER_H
 
 #include "clang/Analysis/ProgramPoint.h"
+#include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/Store.h"
@@ -121,14 +122,36 @@ enum class ObjCMessageVisitKind {
 };
 
 class CheckerManager {
-  ASTContext &Context;
+  ASTContext *Context;
   const LangOptions LangOpts;
   AnalyzerOptions &AOptions;
   CheckerNameRef CurrentCheckerName;
+  DiagnosticsEngine &Diags;
+  std::unique_ptr<CheckerRegistry> Registry;
 
 public:
+  // These constructors are defined in the Frontend library, because
+  // CheckerRegistry, a crucial component of the initialization is in there.
+  // CheckerRegistry cannot be moved to the Core library, because the checker
+  // registration functions are defined in the Checkers library, and the library
+  // dependencies look like this: Core -> Checkers -> Frontend.
+
+  CheckerManager(
+      ASTContext &Context, AnalyzerOptions &AOptions,
+      ArrayRef<std::string> plugins,
+      ArrayRef<std::function<void(CheckerRegistry &)>> checkerRegistrationFns);
+
+  /// Constructs a CheckerManager that ignores all non TblGen-generated
+  /// checkers. Useful for unit testing, unless the checker infrastructure
+  /// itself is tested.
   CheckerManager(ASTContext &Context, AnalyzerOptions &AOptions)
-      : Context(Context), LangOpts(Context.getLangOpts()), AOptions(AOptions) {}
+      : CheckerManager(Context, AOptions, {}, {}) {}
+
+  /// Constructs a CheckerManager without requiring an AST. No checker
+  /// registration will take place. Only useful for retrieving the
+  /// CheckerRegistry and print for help flags where the AST is unavalaible.
+  CheckerManager(AnalyzerOptions &AOptions, const LangOptions &LangOpts,
+                 DiagnosticsEngine &Diags, ArrayRef<std::string> plugins);
 
   ~CheckerManager();
 
@@ -141,7 +164,12 @@ class CheckerManager {
 
   const LangOptions &getLangOpts() const { return LangOpts; }
   AnalyzerOptions &getAnalyzerOptions() const { return AOptions; }
-  ASTContext &getASTContext() const { return Context; }
+  const CheckerRegistry &getCheckerRegistry() const { return *Registry; }
+  DiagnosticsEngine &getDiagnostics() const { return Diags; }
+  ASTContext &getASTContext() const {
+    assert(Context);
+    return *Context;
+  }
 
   /// Emits an error through a DiagnosticsEngine about an invalid user supplied
   /// checker option value.

clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h

@@ -55,7 +55,7 @@ class AnalysisASTConsumer : public ASTConsumer {
 std::unique_ptr<AnalysisASTConsumer>
 CreateAnalysisConsumer(CompilerInstance &CI);
 
-} // end GR namespace
+} // namespace ento
 
 } // end clang namespace
 

clang/include/clang/StaticAnalyzer/Frontend/AnalyzerHelpFlags.h

@@ -0,0 +1,30 @@
+//===-- AnalyzerHelpFlags.h - Query functions for --help flags --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_STATICANALYZER_FRONTEND_ANALYZERHELPFLAGS_H
+#define LLVM_CLANG_STATICANALYZER_FRONTEND_ANALYZERHELPFLAGS_H
+
+namespace llvm {
+class raw_ostream;
+} // namespace llvm
+
+namespace clang {
+
+class CompilerInstance;
+
+namespace ento {
+
+void printCheckerHelp(llvm::raw_ostream &OS, CompilerInstance &CI);
+void printEnabledCheckerList(llvm::raw_ostream &OS, CompilerInstance &CI);
+void printAnalyzerConfigList(llvm::raw_ostream &OS);
+void printCheckerConfigList(llvm::raw_ostream &OS, CompilerInstance &CI);
+
+} // namespace ento
+} // namespace clang
+
+#endif

clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h

@@ -10,7 +10,6 @@
 #define LLVM_CLANG_STATICANALYZER_CORE_CHECKERREGISTRY_H
 
 #include "clang/Basic/LLVM.h"
-#include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
@@ -74,6 +73,8 @@ class LangOptions;
 
 namespace ento {
 
+class CheckerManager;
+
 /// Manages a set of available checkers for running a static analysis.
 /// The checkers are organized into packages by full name, where including
 /// a package will recursively include all subpackages and checkers within it.
@@ -83,10 +84,15 @@ namespace ento {
 class CheckerRegistry {
 public:
   CheckerRegistry(ArrayRef<std::string> plugins, DiagnosticsEngine &diags,
-                  AnalyzerOptions &AnOpts, const LangOptions &LangOpts,
+                  AnalyzerOptions &AnOpts,
                   ArrayRef<std::function<void(CheckerRegistry &)>>
                       checkerRegistrationFns = {});
 
+  /// Collects all enabled checkers in the field EnabledCheckers. It preserves
+  /// the order of insertion, as dependencies have to be enabled before the
+  /// checkers that depend on them.
+  void initializeRegistry(const CheckerManager &Mgr);
+
   /// Initialization functions perform any necessary setup for a checker.
   /// They should include a call to CheckerManager::registerChecker.
   using InitializationFunction = void (*)(CheckerManager &);
@@ -205,14 +211,20 @@ class CheckerRegistry {
 
   using PackageInfoList = llvm::SmallVector<PackageInfo, 0>;
 
-  template <typename T> static void addToCheckerMgr(CheckerManager &mgr) {
-    mgr.registerChecker<T>();
+private:
+  /// Default initialization function for checkers -- since CheckerManager
+  /// includes this header, we need to make it a template parameter, and since
+  /// the checker must be a template parameter as well, we can't put this in the
+  /// cpp file.
+  template <typename MGR, typename T> static void initializeManager(MGR &mgr) {
+    mgr.template registerChecker<T>();
   }
 
-  static bool returnTrue(const LangOptions &LO) {
+  template <typename T> static bool returnTrue(const LangOptions &) {
     return true;
   }
 
+public:
   /// Adds a checker to the registry. Use this non-templated overload when your
   /// checker requires custom initialization.
   void addChecker(InitializationFunction Fn, ShouldRegisterFunction sfn,
@@ -221,13 +233,16 @@ class CheckerRegistry {
 
   /// Adds a checker to the registry. Use this templated overload when your
   /// checker does not require any custom initialization.
+  /// This function isn't really needed and probably causes more headaches than
+  /// the tiny convenience that it provides, but external plugins might use it,
+  /// and there isn't a strong incentive to remove it.
   template <class T>
   void addChecker(StringRef FullName, StringRef Desc, StringRef DocsUri,
                   bool IsHidden = false) {
     // Avoid MSVC's Compiler Error C2276:
     // http://msdn.microsoft.com/en-us/library/850cstw1(v=VS.80).aspx
-    addChecker(&CheckerRegistry::addToCheckerMgr<T>,
-               &CheckerRegistry::returnTrue, FullName, Desc, DocsUri, IsHidden);
+    addChecker(&initializeManager<CheckerManager, T>, &returnTrue<T>, FullName,
+               Desc, DocsUri, IsHidden);
   }
 
   /// Makes the checker with the full name \p fullName depends on the checker
@@ -263,7 +278,7 @@ class CheckerRegistry {
   void addPackageOption(StringRef OptionType, StringRef PackageFullName,
                         StringRef OptionName, StringRef DefaultValStr,
                         StringRef Description, StringRef DevelopmentStatus,
-                         bool IsHidden = false);
+                        bool IsHidden = false);
 
   // FIXME: This *really* should be added to the frontend flag descriptions.
   /// Initializes a CheckerManager by calling the initialization functions for
@@ -283,11 +298,6 @@ class CheckerRegistry {
   void printCheckerOptionList(raw_ostream &Out) const;
 
 private:
-  /// Collect all enabled checkers. The returned container preserves the order
-  /// of insertion, as dependencies have to be enabled before the checkers that
-  /// depend on them.
-  CheckerInfoSet getEnabledCheckers() const;
-
   /// Return an iterator range of mutable CheckerInfos \p CmdLineArg applies to.
   /// For example, it'll return the checkers for the core package, if
   /// \p CmdLineArg is "core".
@@ -314,7 +324,7 @@ class CheckerRegistry {
 
   DiagnosticsEngine &Diags;
   AnalyzerOptions &AnOpts;
-  const LangOptions &LangOpts;
+  CheckerInfoSet EnabledCheckers;
 };
 
 } // namespace ento

clang/include/clang/StaticAnalyzer/Frontend/FrontendActions.h

@@ -51,22 +51,7 @@ class ParseModelFileAction : public ASTFrontendAction {
   llvm::StringMap<Stmt *> &Bodies;
 };
 
-void printCheckerHelp(raw_ostream &OS,
-                      ArrayRef<std::string> plugins,
-                      AnalyzerOptions &opts,
-                      DiagnosticsEngine &diags,
-                      const LangOptions &LangOpts);
-void printEnabledCheckerList(raw_ostream &OS, ArrayRef<std::string> plugins,
-                             AnalyzerOptions &opts,
-                             DiagnosticsEngine &diags,
-                             const LangOptions &LangOpts);
-void printAnalyzerConfigList(raw_ostream &OS);
-void printCheckerConfigList(raw_ostream &OS, ArrayRef<std::string> plugins,
-                            AnalyzerOptions &opts,
-                            DiagnosticsEngine &diags,
-                            const LangOptions &LangOpts);
-
-} // end GR namespace
+} // namespace ento
 
 } // end namespace clang
 

clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp

@@ -23,6 +23,7 @@
 #include "clang/Frontend/Utils.h"
 #include "clang/FrontendTool/Utils.h"
 #include "clang/Rewrite/Frontend/FrontendActions.h"
+#include "clang/StaticAnalyzer/Frontend/AnalyzerHelpFlags.h"
 #include "clang/StaticAnalyzer/Frontend/FrontendActions.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
@@ -243,35 +244,24 @@ bool ExecuteCompilerInvocation(CompilerInstance *Clang) {
   // 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.ShowCheckerHelpAlpha ||
       AnOpts.ShowCheckerHelpDeveloper) {
-    ento::printCheckerHelp(llvm::outs(),
-                           Clang->getFrontendOpts().Plugins,
-                           AnOpts,
-                           Clang->getDiagnostics(),
-                           Clang->getLangOpts());
+    ento::printCheckerHelp(llvm::outs(), *Clang);
     return true;
   }
 
   // Honor -analyzer-checker-option-help.
   if (AnOpts.ShowCheckerOptionList || AnOpts.ShowCheckerOptionAlphaList ||
       AnOpts.ShowCheckerOptionDeveloperList) {
-    ento::printCheckerConfigList(llvm::outs(),
-                                 Clang->getFrontendOpts().Plugins,
-                                 *Clang->getAnalyzerOpts(),
-                                 Clang->getDiagnostics(),
-                                 Clang->getLangOpts());
+    ento::printCheckerConfigList(llvm::outs(), *Clang);
     return true;
   }
 
   // Honor -analyzer-list-enabled-checkers.
   if (AnOpts.ShowEnabledCheckerList) {
-    ento::printEnabledCheckerList(llvm::outs(),
-                                  Clang->getFrontendOpts().Plugins,
-                                  AnOpts,
-                                  Clang->getDiagnostics(),
-                                  Clang->getLangOpts());
+    ento::printEnabledCheckerList(llvm::outs(), *Clang);
     return true;
   }
 

clang/lib/StaticAnalyzer/Core/CheckerManager.cpp

@@ -64,10 +64,9 @@ void CheckerManager::reportInvalidCheckerOptionValue(
     const CheckerBase *C, StringRef OptionName,
     StringRef ExpectedValueDesc) const {
 
-  Context.getDiagnostics()
-      .Report(diag::err_analyzer_checker_option_invalid_input)
-          << (llvm::Twine() + C->getTagDescription() + ":" + OptionName).str()
-          << ExpectedValueDesc;
+  getDiagnostics().Report(diag::err_analyzer_checker_option_invalid_input)
+      << (llvm::Twine() + C->getTagDescription() + ":" + OptionName).str()
+      << ExpectedValueDesc;
 }
 
 //===----------------------------------------------------------------------===//
@@ -903,8 +902,3 @@ CheckerManager::getCachedStmtCheckersFor(const Stmt *S, bool isPreVisit) {
       Checkers.push_back(Info.CheckFn);
   return Checkers;
 }
-
-CheckerManager::~CheckerManager() {
-  for (const auto &CheckerDtor : CheckerDtors)
-    CheckerDtor();
-}

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

@@ -33,7 +33,6 @@
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
-#include "clang/StaticAnalyzer/Frontend/CheckerRegistration.h"
 #include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Support/FileSystem.h"
@@ -209,8 +208,8 @@ class AnalysisConsumer : public AnalysisASTConsumer,
 
   void Initialize(ASTContext &Context) override {
     Ctx = &Context;
-    checkerMgr = createCheckerManager(
-        *Ctx, *Opts, Plugins, CheckerRegistrationFns, PP.getDiagnostics());
+    checkerMgr = std::make_unique<CheckerManager>(*Ctx, *Opts, Plugins,
+                                                  CheckerRegistrationFns);
 
     Mgr = std::make_unique<AnalysisManager>(*Ctx, PP, PathConsumers,
                                             CreateStoreMgr, CreateConstraintMgr,