Skip to content

Commit

Permalink
[analyzer][NFC] Keep track of whether enabling a checker was explictl…
Browse files Browse the repository at this point in the history
…y specified in command line arguments

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. I also made sure that this is done within
CheckerRegisty's constructor, leading to very significant simplifications in
its query-like methods.

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

llvm-svn: 352282
  • Loading branch information
Szelethus committed Jan 26, 2019
1 parent 7d6c58e commit 3daa245
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 69 deletions.
26 changes: 24 additions & 2 deletions clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,19 +90,35 @@ class CheckerRegistry {
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) const {
return State == StateFromCmdLine::State_Enabled && ShouldRegister(LO);
}

CheckerInfo(InitializationFunction Fn, ShouldRegisterFunction sfn,
StringRef Name, StringRef Desc, StringRef DocsUri)
: Initialize(Fn), ShouldRegister(sfn), FullName(Name), Desc(Desc),
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:
Expand Down Expand Up @@ -133,6 +149,7 @@ class CheckerRegistry {
&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.
Expand All @@ -150,8 +167,13 @@ class CheckerRegistry {
private:
CheckerInfoSet getEnabledCheckers() const;

mutable CheckerInfoList Checkers;
mutable llvm::StringMap<size_t> Packages;
/// 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);

CheckerInfoList Checkers;
llvm::StringMap<size_t> Packages;

DiagnosticsEngine &Diags;
AnalyzerOptions &AnOpts;
Expand Down
144 changes: 77 additions & 67 deletions clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,66 @@ static bool isCompatibleAPIVersion(const char *versionString) {
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, \
Expand All @@ -52,6 +106,7 @@ CheckerRegistry::CheckerRegistry(ArrayRef<std::string> plugins,
#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.
Expand Down Expand Up @@ -81,73 +136,38 @@ CheckerRegistry::CheckerRegistry(ArrayRef<std::string> plugins,
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.
// FIXME: Alphabetical sort puts 'experimental' in the middle.
// Would it be better to name it '~experimental' or something else
// that's ASCIIbetically last?
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 (const CheckerInfo &checker : Checkers) {
if (checker.isEnabled(LangOpts))
enabledCheckers.insert(&checker);
}

return enabledCheckers;
Expand All @@ -168,8 +188,6 @@ void CheckerRegistry::addChecker(InitializationFunction Rfn,
}

void CheckerRegistry::initializeManager(CheckerManager &checkerMgr) const {
// Sort checkers for efficient collection.
llvm::sort(Checkers, checkerNameLT);

// Collect checkers enabled by the options.
CheckerInfoSet enabledCheckers = getEnabledCheckers();
Expand Down Expand Up @@ -203,11 +221,6 @@ void CheckerRegistry::validateCheckerOptions() const {

void CheckerRegistry::printHelp(raw_ostream &out,
size_t maxNameChars) const {
// FIXME: Alphabetical sort puts 'experimental' in the middle.
// Would it be better to name it '~experimental' or something else
// that's ASCIIbetically last?
llvm::sort(Checkers, checkerNameLT);

// FIXME: Print available packages.

out << "CHECKERS:\n";
Expand Down Expand Up @@ -240,9 +253,6 @@ void CheckerRegistry::printHelp(raw_ostream &out,
}

void CheckerRegistry::printList(raw_ostream &out) const {
// Sort checkers for efficient collection.
llvm::sort(Checkers, checkerNameLT);

// Collect checkers enabled by the options.
CheckerInfoSet enabledCheckers = getEnabledCheckers();

Expand Down

0 comments on commit 3daa245

Please sign in to comment.