[analyzer][NFC] Move CheckerOptInfo to CheckerRegistry.cpp, and make it local

[Szelethus]

Summary

TL;DR: CheckerOptInfo feels very much out of place in CheckerRegistration.cpp, so I moved it to CheckerRegistry.h.

Details:

While on the quest of fixing checker options the same way I cleaned up non-checker options, although I'm making good progress, I ran into a wall: In order to emit warnings on incorrect checker options, we need to ensure that checkers actually acquire their options properly -- but, I unearthed a huge bug in checker registration: dependencies are currently implemented in a way that breaks the already very fragile registration infrastructure.

Here is where the problem really originates from: this is a snippet from CheckerRegistry::initializeManager.

// Initialize the CheckerManager with all enabled checkers.
for (const auto *i :enabledCheckers) {
  checkerMgr.setCurrentCheckName(CheckName(i->FullName));
  i->Initialize(checkerMgr);
}

Note that Initialize is a function pointer to register##CHECKERNAME, like registerInnerPointerChecker. Since for each register function call the current checker name is only set once, when InnerPointerChecker attempts to also register it's dependent MallocChecker, both it and MallocChecker will receive the cplusplus.InnerPointer name. This results in MallocChecker trying to acquire it's Optimistic option as cplusplus.InnerPointerChecker:Optimistic.

Clearly, this problem has to be solved at a higher level -- it makes no sense to be able to affect other checkers in a registry function. Since I'll already make changes to how checker registration works, I'll probably tune some things for the current C++ standard, add much needed documentation, and try to make the code a lot less confusing.

include/clang/StaticAnalyzer/Core/CheckerOptInfo.h

-//===--- CheckerOptInfo.h - Specifies which checkers to use -----*- C++ -*-===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_CLANG_STATICANALYZER_CORE_CHECKEROPTINFO_H
-#define LLVM_CLANG_STATICANALYZER_CORE_CHECKEROPTINFO_H
-
-#include "clang/Basic/LLVM.h"
-#include "llvm/ADT/StringRef.h"
-
-namespace clang {
-namespace ento {
-
-/// Represents a request to include or exclude a checker or package from a
-/// specific analysis run.
-///
-/// \sa CheckerRegistry::initializeManager
-class CheckerOptInfo {
-  StringRef Name;
-  bool Enable;
-  bool Claimed;
-
-public:
-  CheckerOptInfo(StringRef name, bool enable)
-    : Name(name), Enable(enable), Claimed(false) { }
-
-  StringRef getName() const { return Name; }
-  bool isEnabled() const { return Enable; }
-  bool isDisabled() const { return !isEnabled(); }
-
-  bool isClaimed() const { return Claimed; }
-  bool isUnclaimed() const { return !isClaimed(); }
-  void claim() { Claimed = true; }
-};
-
-} // end namespace ento
-} // end namespace clang
-
-#endif

include/clang/StaticAnalyzer/Core/CheckerRegistry.h

 
 namespace clang {
 
 class AnalyzerOptions;
 class DiagnosticsEngine;
 
 namespace ento {
 
-class CheckerOptInfo;
-
 /// 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.
 /// For example, the checker "core.builtin.NoReturnFunctionChecker" will be
 /// included if initializeManager() is called with an option of "core",
 /// "core.builtin", or the full name "core.builtin.NoReturnFunctionChecker".
 class CheckerRegistry {
 public:
void addChecker(StringRef fullName, StringRef desc) {
     // http://msdn.microsoft.com/en-us/library/850cstw1(v=VS.80).aspx
     addChecker(&CheckerRegistry::initializeManager<T>, fullName, desc);
   }
 
   /// Initializes a CheckerManager by calling the initialization functions for
   /// all checkers specified by the given CheckerOptInfo list. The order of this
   /// list is significant; later options can be used to reverse earlier ones.
   /// This can be used to exclude certain checkers in an included package.
-  void initializeManager(CheckerManager &mgr,
-                         SmallVectorImpl<CheckerOptInfo> &opts) const;
+  void initializeManager(CheckerManager &mgr, const AnalyzerOptions &Opts,
+                         DiagnosticsEngine &diags) const;
 
   /// Check if every option corresponds to a specific checker or package.
   void validateCheckerOptions(const AnalyzerOptions &opts,
                               DiagnosticsEngine &diags) const;
 
   /// Prints the name and description of all checkers in this registry.
   /// This output is not intended to be machine-parseable.
   void printHelp(raw_ostream &out, size_t maxNameChars = 30) const;
-  void printList(raw_ostream &out,
-                 SmallVectorImpl<CheckerOptInfo> &opts) const;
+  void printList(raw_ostream &out, const AnalyzerOptions &opts) const;
 
 private:
   mutable CheckerInfoList Checkers;
   mutable llvm::StringMap<size_t> Packages;
 };
 
 } // namespace ento
 
 } // namespace clang
 
 #endif // LLVM_CLANG_STATICANALYZER_CORE_CHECKERREGISTRY_H

lib/StaticAnalyzer/Core/CheckerRegistry.cpp

 //===- CheckerRegistry.cpp - Maintains all available checkers -------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Core/CheckerRegistry.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
-#include "clang/StaticAnalyzer/Core/CheckerOptInfo.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cstddef>
 #include <tuple>
 
 using namespace clang;
 using namespace ento;
 
 static const char PackageSeparator = '.';
 
 using CheckerInfoSet = llvm::SetVector<const CheckerRegistry::CheckerInfo *>;
 
+namespace {
+/// Represents a request to include or exclude a checker or package from a
+/// specific analysis run.
+///
+/// \sa CheckerRegistry::initializeManager
+class CheckerOptInfo {
+  StringRef Name;
+  bool Enable;
+  bool Claimed;
+
+public:
+  CheckerOptInfo(StringRef name, bool enable)
+    : Name(name), Enable(enable), Claimed(false) { }
+
+  StringRef getName() const { return Name; }
+  bool isEnabled() const { return Enable; }
+  bool isDisabled() const { return !isEnabled(); }
+
+  bool isClaimed() const { return Claimed; }
+  bool isUnclaimed() const { return !isClaimed(); }
+  void claim() { Claimed = true; }
+};
+
+} // end of anonymous namespace
+
+static SmallVector<CheckerOptInfo, 8>
+getCheckerOptList(const AnalyzerOptions &opts) {
+  SmallVector<CheckerOptInfo, 8> checkerOpts;
+  for (unsigned i = 0, e = opts.CheckersControlList.size(); i != e; ++i) {
+    const std::pair<std::string, bool> &opt = opts.CheckersControlList[i];
+    checkerOpts.push_back(CheckerOptInfo(opt.first, opt.second));
+  }
+  return checkerOpts;
+}
+
 static bool checkerNameLT(const CheckerRegistry::CheckerInfo &a,
                           const CheckerRegistry::CheckerInfo &b) {
   return a.FullName < b.FullName;
 }
 
 static bool isInPackage(const CheckerRegistry::CheckerInfo &checker,
                         StringRef packageName) {
   // Does the checker's full name have the package as a prefix?
   if (!checker.FullName.startswith(packageName))
     return false;
 
   // Is the package actually just the name of a specific checker?
   if (checker.FullName.size() == packageName.size())
     return true;
 
   // Is the checker in the package (or a subpackage)?
   if (checker.FullName[packageName.size()] == PackageSeparator)
     return true;
 
   return false;
 }
 
+/// Collects the checkers for the supplied \p opt option into \p collected.
 static void collectCheckers(const CheckerRegistry::CheckerInfoList &checkers,
                             const llvm::StringMap<size_t> &packageSizes,
                             CheckerOptInfo &opt, CheckerInfoSet &collected) {
   // Use a binary search to find the possible start of the package.
   CheckerRegistry::CheckerInfo packageInfo(nullptr, opt.getName(), "");
   auto end = checkers.cend();
   auto i = std::lower_bound(checkers.cbegin(), end, packageInfo, checkerNameLT);
 
void CheckerRegistry::addChecker(InitializationFunction fn, StringRef name,
   std::tie(packageName, leafName) = name.rsplit(PackageSeparator);
   while (!leafName.empty()) {
     Packages[packageName] += 1;
     std::tie(packageName, leafName) = packageName.rsplit(PackageSeparator);
   }
 }
 
 void CheckerRegistry::initializeManager(CheckerManager &checkerMgr,
-                                  SmallVectorImpl<CheckerOptInfo> &opts) const {
+                                        const AnalyzerOptions &Opts,
+                                        DiagnosticsEngine &diags) const {
   // Sort checkers for efficient collection.
   llvm::sort(Checkers, checkerNameLT);
 
+  llvm::SmallVector<CheckerOptInfo, 8> checkerOpts = getCheckerOptList(Opts);
   // Collect checkers enabled by the options.
   CheckerInfoSet enabledCheckers;
-  for (auto &i : opts)
+  for (auto &i : checkerOpts)
     collectCheckers(Checkers, Packages, i, enabledCheckers);
 
   // Initialize the CheckerManager with all enabled checkers.
   for (const auto *i :enabledCheckers) {
     checkerMgr.setCurrentCheckName(CheckName(i->FullName));
     i->Initialize(checkerMgr);
   }
+
+  for (unsigned i = 0, e = checkerOpts.size(); i != e; ++i) {
+    if (checkerOpts[i].isUnclaimed()) {
+      diags.Report(diag::err_unknown_analyzer_checker)
+          << checkerOpts[i].getName();
+      diags.Report(diag::note_suggest_disabling_all_checkers);
+    }
+
+  }
 }
 
 void CheckerRegistry::validateCheckerOptions(const AnalyzerOptions &opts,
                                              DiagnosticsEngine &diags) const {
   for (const auto &config : opts.Config) {
     size_t pos = config.getKey().find(':');
     if (pos == StringRef::npos)
       continue;
if (pad < 0) {
       pad = optionFieldWidth + initialPad;
     }
     out.indent(pad + 2) << i.Desc;
 
     out << '\n';
   }
 }
 
-void CheckerRegistry::printList(
-    raw_ostream &out, SmallVectorImpl<CheckerOptInfo> &opts) const {
+void CheckerRegistry::printList(raw_ostream &out,
+                                const AnalyzerOptions &opts) const {
   llvm::sort(Checkers, checkerNameLT);
 
+  llvm::SmallVector<CheckerOptInfo, 8> checkerOpts = getCheckerOptList(opts);
   // Collect checkers enabled by the options.
   CheckerInfoSet enabledCheckers;
-  for (auto &i : opts)
+  for (auto &i : checkerOpts)
     collectCheckers(Checkers, Packages, i, enabledCheckers);
 
   for (const auto *i : enabledCheckers)
     out << i->FullName << '\n';
 }

lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp

 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Frontend/CheckerRegistration.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/StaticAnalyzer/Checkers/ClangCheckers.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
-#include "clang/StaticAnalyzer/Core/CheckerOptInfo.h"
 #include "clang/StaticAnalyzer/Core/CheckerRegistry.h"
 #include "clang/StaticAnalyzer/Frontend/FrontendActions.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/FormattedStream.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include <memory>
void ClangCheckerRegistry::warnIncompatible(DiagnosticsEngine *diags,
 
   diags->Report(diag::warn_incompatible_analyzer_plugin_api)
       << llvm::sys::path::filename(pluginPath);
   diags->Report(diag::note_incompatible_analyzer_plugin_api)
       << CLANG_ANALYZER_API_VERSION_STRING
       << pluginAPIVersion;
 }
 
-static SmallVector<CheckerOptInfo, 8>
-getCheckerOptList(const AnalyzerOptions &opts) {
-  SmallVector<CheckerOptInfo, 8> checkerOpts;
-  for (unsigned i = 0, e = opts.CheckersControlList.size(); i != e; ++i) {
-    const std::pair<std::string, bool> &opt = opts.CheckersControlList[i];
-    checkerOpts.push_back(CheckerOptInfo(opt.first, opt.second));
-  }
-  return checkerOpts;
-}
-
 std::unique_ptr<CheckerManager> ento::createCheckerManager(
     ASTContext &context,
     AnalyzerOptions &opts,
     ArrayRef<std::string> plugins,
     ArrayRef<std::function<void(CheckerRegistry &)>> checkerRegistrationFns,
     DiagnosticsEngine &diags) {
   auto checkerMgr = llvm::make_unique<CheckerManager>(context, opts);
 
-  SmallVector<CheckerOptInfo, 8> checkerOpts = getCheckerOptList(opts);
-
   ClangCheckerRegistry allCheckers(plugins, &diags);
 
   for (const auto &Fn : checkerRegistrationFns)
     Fn(allCheckers);
 
-  allCheckers.initializeManager(*checkerMgr, checkerOpts);
+  allCheckers.initializeManager(*checkerMgr, opts, diags);
   allCheckers.validateCheckerOptions(opts, diags);
   checkerMgr->finishedCheckerRegistration();
 
-  for (unsigned i = 0, e = checkerOpts.size(); i != e; ++i) {
-    if (checkerOpts[i].isUnclaimed()) {
-      diags.Report(diag::err_unknown_analyzer_checker)
-          << checkerOpts[i].getName();
-      diags.Report(diag::note_suggest_disabling_all_checkers);
-    }
-
-  }
-
   return checkerMgr;
 }
 
 void ento::printCheckerHelp(raw_ostream &out, ArrayRef<std::string> plugins) {
   out << "OVERVIEW: Clang Static Analyzer Checkers List\n\n";
   out << "USAGE: -analyzer-checker <CHECKER or PACKAGE,...>\n\n";
 
   ClangCheckerRegistry(plugins).printHelp(out);
 }
 
 void ento::printEnabledCheckerList(raw_ostream &out,
                                    ArrayRef<std::string> plugins,
                                    const AnalyzerOptions &opts) {
   out << "OVERVIEW: Clang Static Analyzer Enabled Checkers List\n\n";
 
-  SmallVector<CheckerOptInfo, 8> checkerOpts = getCheckerOptList(opts);
-  ClangCheckerRegistry(plugins).printList(out, checkerOpts);
+  ClangCheckerRegistry(plugins).printList(out, opts);
 }
 
 void ento::printAnalyzerConfigList(raw_ostream &out) {
   out << "OVERVIEW: Clang Static Analyzer -analyzer-config Option List\n\n";
   out << "USAGE: clang -cc1 [CLANG_OPTIONS] -analyzer-config "
                                         "<OPTION1=VALUE,OPTION2=VALUE,...>\n\n";
   out << "       clang -cc1 [CLANG_OPTIONS] -analyzer-config OPTION1=VALUE, "
                                       "-analyzer-config OPTION2=VALUE, ...\n\n";