[analyzer] Prefer returns values to out-params in CheckerRegistry.cpp

Renaming collectCheckers to getEnabledCheckers
Changing the functionality to acquire all enabled checkers, rather then collect
checkers for a specific CheckerOptInfo (for example, collecting all checkers for
{ "core", true }, which meant enabling all checkers from the core package, which
was an unnecessary complication).
Removing CheckerOptInfo, instead of storing whether the option was claimed via a
field, we handle errors immediately, as getEnabledCheckers can now access a
DiagnosticsEngine. Realize that the remaining information it stored is directly
accessible through AnalyzerOptions.CheckerControlList.
Fix a test with -analyzer-disable-checker -verify accidentally left in.

llvm-svn: 349274

6 files changed, 75 insertions, 101 deletions

diff --git a/clang/include/clang/StaticAnalyzer/Core/CheckerRegistry.h b/clang/include/clang/StaticAnalyzer/Core/CheckerRegistry.h
index 740754090da..589a258852a 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerRegistry.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerRegistry.h
@@ -95,6 +95,7 @@ public:
   };
 
   using CheckerInfoList = std::vector<CheckerInfo>;
+  using CheckerInfoSet = llvm::SetVector<const CheckerRegistry::CheckerInfo *>;
 
 private:
   template <typename T>
@@ -131,9 +132,13 @@ public:
   /// 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, const AnalyzerOptions &opts) const;
+  void printList(raw_ostream &out, const AnalyzerOptions &opts,
+                 DiagnosticsEngine &diags) const;
 
 private:
+  CheckerInfoSet getEnabledCheckers(const AnalyzerOptions &Opts,
+                                    DiagnosticsEngine &diags) const;
+
   mutable CheckerInfoList Checkers;
   mutable llvm::StringMap<size_t> Packages;
 };
diff --git a/clang/include/clang/StaticAnalyzer/Frontend/FrontendActions.h b/clang/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
index 8fd45bf1025..20b71ef69d0 100644
--- a/clang/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
+++ b/clang/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
@@ -54,7 +54,8 @@ private:
 
 void printCheckerHelp(raw_ostream &OS, ArrayRef<std::string> plugins);
 void printEnabledCheckerList(raw_ostream &OS, ArrayRef<std::string> plugins,
-                             const AnalyzerOptions &opts);
+                             const AnalyzerOptions &opts,
+                             DiagnosticsEngine &diags);
 void printAnalyzerConfigList(raw_ostream &OS);
 
 } // end GR namespace
diff --git a/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp b/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
index df360de857a..5239a7e6f85 100644
--- a/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ b/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -246,7 +246,8 @@ bool ExecuteCompilerInvocation(CompilerInstance *Clang) {
   if (Clang->getAnalyzerOpts()->ShowEnabledCheckerList) {
     ento::printEnabledCheckerList(llvm::outs(),
                                   Clang->getFrontendOpts().Plugins,
-                                  *Clang->getAnalyzerOpts());
+                                  *Clang->getAnalyzerOpts(),
+                                  Clang->getDiagnostics());
   }
 
   // Honor -analyzer-config-help.
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerRegistry.cpp b/clang/lib/StaticAnalyzer/Core/CheckerRegistry.cpp
index e7dfac74797..6a38e96efc5 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerRegistry.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerRegistry.cpp
@@ -18,50 +18,11 @@
 #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 constexpr char PackageSeparator = '.';
 
 static bool checkerNameLT(const CheckerRegistry::CheckerInfo &a,
                           const CheckerRegistry::CheckerInfo &b) {
@@ -85,40 +46,49 @@ static bool isInPackage(const CheckerRegistry::CheckerInfo &checker,
   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);
-
-  // If we didn't even find a possible package, give up.
-  if (i == end)
-    return;
-
-  // If what we found doesn't actually start the package, give up.
-  if (!isInPackage(*i, opt.getName()))
-    return;
-
-  // There is at least one checker in the package; claim the option.
-  opt.claim();
-
-  // See how large the package is.
-  // If the package doesn't exist, assume the option refers to a single checker.
-  size_t size = 1;
-  llvm::StringMap<size_t>::const_iterator packageSize =
-    packageSizes.find(opt.getName());
-  if (packageSize != packageSizes.end())
-    size = packageSize->getValue();
-
-  // Step through all the checkers in the package.
-  for (auto checkEnd = i+size; i != checkEnd; ++i)
-    if (opt.isEnabled())
-      collected.insert(&*i);
-    else
-      collected.remove(&*i);
+CheckerRegistry::CheckerInfoSet CheckerRegistry::getEnabledCheckers(
+                                              const AnalyzerOptions &Opts,
+                                              DiagnosticsEngine &diags) const {
+
+  assert(std::is_sorted(Checkers.begin(), Checkers.end(), checkerNameLT) &&
+         "In order to efficiently gather checkers, this function expects them "
+         "to be already sorted!");
+
+  CheckerInfoSet enabledCheckers;
+  const auto end = Checkers.cend();
+
+  for (const std::pair<std::string, bool> &opt : Opts.CheckersControlList) {
+    // Use a binary search to find the possible start of the package.
+    CheckerRegistry::CheckerInfo packageInfo(nullptr, opt.first, "");
+    auto firstRelatedChecker =
+      std::lower_bound(Checkers.cbegin(), end, packageInfo, checkerNameLT);
+
+    if (firstRelatedChecker == end ||
+        !isInPackage(*firstRelatedChecker, opt.first)) {
+      diags.Report(diag::err_unknown_analyzer_checker) << opt.first;
+      diags.Report(diag::note_suggest_disabling_all_checkers);
+      return {};
+    }
+
+    // See how large the package is.
+    // If the package doesn't exist, assume the option refers to a single
+    // checker.
+    size_t size = 1;
+    llvm::StringMap<size_t>::const_iterator packageSize =
+      Packages.find(opt.first);
+    if (packageSize != Packages.end())
+      size = packageSize->getValue();
+
+    // Step through all the checkers in the package.
+    for (auto lastRelatedChecker = firstRelatedChecker+size;
+         firstRelatedChecker != lastRelatedChecker; ++firstRelatedChecker)
+      if (opt.second)
+        enabledCheckers.insert(&*firstRelatedChecker);
+      else
+        enabledCheckers.remove(&*firstRelatedChecker);
+  }
+
+  return enabledCheckers;
 }
 
 void CheckerRegistry::addChecker(InitializationFunction fn, StringRef name,
@@ -140,26 +110,14 @@ void CheckerRegistry::initializeManager(CheckerManager &checkerMgr,
   // 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 : checkerOpts)
-    collectCheckers(Checkers, Packages, i, enabledCheckers);
+  CheckerInfoSet enabledCheckers = getEnabledCheckers(Opts, diags);
 
   // Initialize the CheckerManager with all enabled checkers.
-  for (const auto *i :enabledCheckers) {
+  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,
@@ -222,14 +180,13 @@ void CheckerRegistry::printHelp(raw_ostream &out,
 }
 
 void CheckerRegistry::printList(raw_ostream &out,
-                                const AnalyzerOptions &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 : checkerOpts)
-    collectCheckers(Checkers, Packages, i, enabledCheckers);
+  CheckerInfoSet enabledCheckers = getEnabledCheckers(opts, diags);
 
   for (const auto *i : enabledCheckers)
     out << i->FullName << '\n';
diff --git a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
index 70997e34a18..c82ed12cac0 100644
--- a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
@@ -130,10 +130,11 @@ void ento::printCheckerHelp(raw_ostream &out, ArrayRef<std::string> plugins) {
 
 void ento::printEnabledCheckerList(raw_ostream &out,
                                    ArrayRef<std::string> plugins,
-                                   const AnalyzerOptions &opts) {
+                                   const AnalyzerOptions &opts,
+                                   DiagnosticsEngine &diags) {
   out << "OVERVIEW: Clang Static Analyzer Enabled Checkers List\n\n";
 
-  ClangCheckerRegistry(plugins).printList(out, opts);
+  ClangCheckerRegistry(plugins).printList(out, opts, diags);
 }
 
 void ento::printAnalyzerConfigList(raw_ostream &out) {
diff --git a/clang/test/Analysis/disable-all-checks.c b/clang/test/Analysis/disable-all-checks.c
index eb55799c259..fba53429ab5 100644
--- a/clang/test/Analysis/disable-all-checks.c
+++ b/clang/test/Analysis/disable-all-checks.c
@@ -1,9 +1,18 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region -analyzer-disable-all-checks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-disable-all-checks -analyzer-checker=core -analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region \
+// RUN:   -analyzer-disable-all-checks -verify %s
+//
+// RUN: %clang_analyze_cc1 -analyzer-disable-all-checks -analyzer-checker=core \
+// RUN:   -analyzer-store=region -verify %s
+//
 // RUN: %clang_analyze_cc1 -analyzer-disable-all-checks -verify %s
-// RUN: not %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region -analyzer-disable-checker -verify %s 2>&1 | FileCheck %s
+//
+// RUN: not %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region \
+// RUN:   -analyzer-disable-checker non.existant.Checker -verify %s 2>&1 \
+// RUN:   | FileCheck %s
+//
 // expected-no-diagnostics
 
+// CHECK: no analyzer checkers are associated with 'non.existant.Checker'
 // CHECK: use -analyzer-disable-all-checks to disable all static analyzer checkers
 int buggy() {
   int x = 0;