Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, MTC.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, rnkovacs, szepet, whisperity.
This bugged me for a long time, so it's time to put an end to it:
`collectCheckers` was cryptic and hard to understand. This is done by
- 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`.
- Removing the suggestion to disable all checkers when a checker option is left
unclaimed. I personally found this super annoying, and I'd be very surprised if
this was a welcome suggestion to anyone.
- Fix a test with `-analyzer-disable-checker -verify` accidentally left in.
Repository:
rC Clang
https://reviews.llvm.org/D54401
Files:
include/clang/StaticAnalyzer/Core/CheckerRegistry.h
include/clang/StaticAnalyzer/Frontend/FrontendActions.h
lib/FrontendTool/ExecuteCompilerInvocation.cpp
lib/StaticAnalyzer/Core/CheckerRegistry.cpp
lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
test/Analysis/disable-all-checks.c
Index: test/Analysis/disable-all-checks.c
===================================================================
--- test/Analysis/disable-all-checks.c
+++ test/Analysis/disable-all-checks.c
@@ -1,10 +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: use -analyzer-disable-all-checks to disable all static analyzer checkers
+// CHECK: no analyzer checkers are associated with 'non.existant.Checker'
int buggy() {
int x = 0;
return 5/x; // no warning
Index: lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
@@ -130,10 +130,11 @@
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) {
Index: lib/StaticAnalyzer/Core/CheckerRegistry.cpp
===================================================================
--- lib/StaticAnalyzer/Core/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Core/CheckerRegistry.cpp
@@ -9,7 +9,6 @@
#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/AnalyzerOptions.h"
@@ -19,51 +18,14 @@
#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 = '.';
+static constexpr 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;
@@ -86,40 +48,50 @@
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);
+static CheckerInfoSet getEnabledCheckers(
+ const CheckerRegistry::CheckerInfoList &checkers,
+ const llvm::StringMap<size_t> &packages,
+ const AnalyzerOptions &Opts,
+ DiagnosticsEngine &diags) {
+
+ 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;
+ 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,
@@ -141,26 +113,15 @@
// 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(Checkers, Packages, 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,
@@ -223,14 +184,14 @@
}
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(Checkers, Packages, opts, diags);
for (const auto *i : enabledCheckers)
out << i->FullName << '\n';
Index: lib/FrontendTool/ExecuteCompilerInvocation.cpp
===================================================================
--- lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -246,7 +246,8 @@
if (Clang->getAnalyzerOpts()->ShowEnabledCheckerList) {
ento::printEnabledCheckerList(llvm::outs(),
Clang->getFrontendOpts().Plugins,
- *Clang->getAnalyzerOpts());
+ *Clang->getAnalyzerOpts(),
+ Clang->getDiagnostics());
}
// Honor -analyzer-config-help.
Index: include/clang/StaticAnalyzer/Frontend/FrontendActions.h
===================================================================
--- include/clang/StaticAnalyzer/Frontend/FrontendActions.h
+++ include/clang/StaticAnalyzer/Frontend/FrontendActions.h
@@ -54,7 +54,8 @@
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
Index: include/clang/StaticAnalyzer/Core/CheckerRegistry.h
===================================================================
--- include/clang/StaticAnalyzer/Core/CheckerRegistry.h
+++ include/clang/StaticAnalyzer/Core/CheckerRegistry.h
@@ -131,7 +131,8 @@
/// 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:
mutable CheckerInfoList Checkers;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
