Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs,
baloghadamsoftware.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy,
mikhail.ramalho, a.sidorin, mgrang, szepet, whisperity.
Szelethus retitled this revision from "[analyzer][NFC] Fully initialize
CheckerRegistry in by the end of construction, make all methods const" to
"[analyzer][NFC] Fully initialize CheckerRegistry in by the end of
construction".
One of the bugs that I left in in my dependency patch D54438
<https://reviews.llvm.org/D54438> was luckily discovered thanks to
@george.karpenkov commiting a patch a couple days before I attempted to commit:
Namely, the rigid structure of how the user can enable/disable checkers wasn't
respected, and chaos ensued when a dependency was disabled, but the checker
that was depending on it was enabled.
This patch moves all the code that modifies the state of `CheckerRegistry` into
it's constructor, ensuring that once constructed, it'll be in its final state.
The important thing here is that I added a new enum to `CheckerInfo`, so we can
easily track whether the check is explicitly enabled, explicitly disabled, or
isn't specified in this regard. Checkers belonging in the latter category may
be implicitly enabled through dependencies in the followup patch.
Repository:
rC Clang
https://reviews.llvm.org/D56989
Files:
include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -38,12 +38,66 @@
return strcmp(versionString, CLANG_ANALYZER_API_VERSION_STRING) == 0;
}
+static bool checkerNameLT(const CheckerRegistry::CheckerInfo &a,
+ const CheckerRegistry::CheckerInfo &b) {
+ return a.FullName < b.FullName;
+}
+
+static constexpr char PackageSeparator = '.';
+
+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;
+}
+
+CheckerRegistry::CheckerInfoListRange
+CheckerRegistry::getMutableCheckersForCmdLineArg(StringRef CmdLineArg) {
+
+ assert(std::is_sorted(Checkers.begin(), Checkers.end(), checkerNameLT) &&
+ "In order to efficiently gather checkers, this function expects them "
+ "to be already sorted!");
+
+ // Use a binary search to find the possible start of the package.
+ CheckerRegistry::CheckerInfo
+ packageInfo(nullptr, nullptr, CmdLineArg, "", "");
+ auto it = std::lower_bound(Checkers.begin(), Checkers.end(),
+ packageInfo, checkerNameLT);
+
+ if (!isInPackage(*it, CmdLineArg))
+ return { Checkers.end(), Checkers.end() };
+
+ // 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(CmdLineArg);
+
+ if (packageSize != Packages.end())
+ size = packageSize->getValue();
+
+ return { it, it + size };
+}
+
CheckerRegistry::CheckerRegistry(ArrayRef<std::string> plugins,
DiagnosticsEngine &diags,
AnalyzerOptions &AnOpts,
const LangOptions &LangOpts)
: Diags(diags), AnOpts(AnOpts), LangOpts(LangOpts) {
+ // Register builtin checkers.
#define GET_CHECKERS
#define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI) \
addChecker(register##CLASS, shouldRegister##CLASS, FULLNAME, HELPTEXT, \
@@ -52,6 +106,7 @@
#undef CHECKER
#undef GET_CHECKERS
+ // Register checkers from plugins.
for (ArrayRef<std::string>::iterator i = plugins.begin(), e = plugins.end();
i != e; ++i) {
// Get access to the plugin.
@@ -81,73 +136,35 @@
if (registerPluginCheckers)
registerPluginCheckers(*this);
}
-}
-
-static constexpr char PackageSeparator = '.';
-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;
+ // Sort checkers for efficient collection.
+ llvm::sort(Checkers, checkerNameLT);
- // Is the package actually just the name of a specific checker?
- if (checker.FullName.size() == packageName.size())
- return true;
+ // Parse '-analyzer-checker' and '-analyzer-disable-checker' options from the
+ // command line.
+ for (const std::pair<std::string, bool> &opt : AnOpts.CheckersControlList) {
+ CheckerInfoListRange checkersForCmdLineArg =
+ getMutableCheckersForCmdLineArg(opt.first);
- // Is the checker in the package (or a subpackage)?
- if (checker.FullName[packageName.size()] == PackageSeparator)
- return true;
+ if (checkersForCmdLineArg.begin() == checkersForCmdLineArg.end()) {
+ Diags.Report(diag::err_unknown_analyzer_checker) << opt.first;
+ Diags.Report(diag::note_suggest_disabling_all_checkers);
+ }
- return false;
+ for (CheckerInfo &checker : checkersForCmdLineArg) {
+ checker.State = opt.second ? StateFromCmdLine::State_Enabled :
+ StateFromCmdLine::State_Disabled;
+ }
+ }
}
CheckerRegistry::CheckerInfoSet CheckerRegistry::getEnabledCheckers() 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 : AnOpts.CheckersControlList) {
- // Use a binary search to find the possible start of the package.
- CheckerRegistry::CheckerInfo
- packageInfo(nullptr, 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) {
- if (firstRelatedChecker->ShouldRegister(LangOpts))
- enabledCheckers.insert(&*firstRelatedChecker);
- } else {
- enabledCheckers.remove(&*firstRelatedChecker);
- }
+ for (CheckerInfo &checker : Checkers) {
+ if (checker.isEnabled(LangOpts))
+ enabledCheckers.insert(&checker);
}
return enabledCheckers;
@@ -168,8 +185,6 @@
}
void CheckerRegistry::initializeManager(CheckerManager &checkerMgr) const {
- // Sort checkers for efficient collection.
- llvm::sort(Checkers, checkerNameLT);
// Collect checkers enabled by the options.
CheckerInfoSet enabledCheckers = getEnabledCheckers();
Index: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
===================================================================
--- include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -90,11 +90,25 @@
using ShouldRegisterFunction = bool (*)(const LangOptions &);
struct CheckerInfo {
+ enum class StateFromCmdLine {
+ // This checker wasn't explicitly enabled or disabled.
+ State_Unspecified,
+ // This checker was explicitly disabled.
+ State_Disabled,
+ // This checker was explicitly enabled.
+ State_Enabled
+ };
+
InitializationFunction Initialize;
ShouldRegisterFunction ShouldRegister;
StringRef FullName;
StringRef Desc;
StringRef DocumentationUri;
+ StateFromCmdLine State = StateFromCmdLine::State_Unspecified;
+
+ bool isEnabled(const LangOptions &LO) {
+ return State == StateFromCmdLine::State_Enabled && ShouldRegister(LO);
+ }
CheckerInfo(InitializationFunction Fn, ShouldRegisterFunction sfn,
StringRef Name, StringRef Desc, StringRef DocsUri)
@@ -102,7 +116,9 @@
DocumentationUri(DocsUri) {}
};
+ using StateFromCmdLine = CheckerInfo::StateFromCmdLine;
using CheckerInfoList = std::vector<CheckerInfo>;
+ using CheckerInfoListRange = llvm::iterator_range<CheckerInfoList::iterator>;
using CheckerInfoSet = llvm::SetVector<const CheckerRegistry::CheckerInfo *>;
private:
@@ -133,6 +149,7 @@
&CheckerRegistry::returnTrue<T>, FullName, Desc, DocsUri);
}
+ // FIXME: This *really* should be added to the frontend flag descriptions.
/// 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.
@@ -150,6 +167,11 @@
private:
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".
+ CheckerInfoListRange getMutableCheckersForCmdLineArg(StringRef CmdLineArg);
+
mutable CheckerInfoList Checkers;
mutable llvm::StringMap<size_t> Packages;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
