-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[analyzer] Prettify checker registration and unittest code #147797
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This commit tweaks the interface of `CheckerRegistry::addChecker` to make it more practical for plugins and tests: - The most general overload (where `DocsUri` appears in the parameter list) is turned into a private method which is only used for loading the checkers described in `Checkers.td`. Note that currently _nothing_ queries the documentation URI from the checker registry (it's only used by the logic that generates the clang-tidy documentation, but that loads it directly from `Checkers.td` without involving the `CheckerRegistry`), so there is no reason to require anyone to provide this value. (In fact, it may be a good idea to remove it completely from the `CheckerRegistry`.) - A new public overload is introduced which differs from the most general one by omitting the `DocsUri` argument and making the `IsHidden` parameter optional (by setting it to `false` by default). This will cover the needs of plugins that want maximal customization. - The templated overload (which takes the checker type as a template paarameter + the checker name and description as parameters) also loses the `DocsUri` parameter. - A new method `addMockChecker<T>` is added for use in unit tests. In addition to propagating these changes (e.g. using `addMockChecker` in unittests), this commit clarifies, corrects and extends lots of comments and performs various minor code quality improvements in the code of unittests and example plugins. I originally wrote the bulk of this commit when I was planning to add an extra parameter to `addChecker` in order to implement some technical details of the CheckerFamily framework. At the end I decided against adding that extra parameter, so this cleanup was left out of the PR llvm#139256 and I'm merging it now as a separate commit (after minor tweaks). This commit is mostly NFC: the only functional change is that the public checker registration functions no longer require (or accept) `DocsUri` as an argument, and the `IsHidden` argument is optional for both public overloads of `addChecker` (and not just the "basic" templated one).
@llvm/pr-subscribers-clang-analysis @llvm/pr-subscribers-clang Author: Donát Nagy (NagyDonat) ChangesThis commit tweaks the interface of
In addition to propagating these changes (e.g. using I originally wrote the bulk of this commit when I was planning to add an extra parameter to This commit is mostly NFC: the only functional change is that the public checker registration functions no longer require (or accept) Patch is 27.34 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/147797.diff 18 Files Affected:
diff --git a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
index 43dbfb1585151..91871c13c9c83 100644
--- a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -38,29 +38,29 @@
// function clang_registerCheckers. For example:
//
// extern "C"
-// void clang_registerCheckers (CheckerRegistry ®istry) {
-// registry.addChecker<MainCallChecker>("example.MainCallChecker",
-// "Disallows calls to functions called main");
+// void clang_registerCheckers(CheckerRegistry &Registry) {
+// Registry.addChecker<MainCallChecker>(
+// "example.MainCallChecker",
+// "Disallows calls to functions called main");
// }
//
-// The first method argument is the full name of the checker, including its
-// enclosing package. By convention, the registered name of a checker is the
-// name of the associated class (the template argument).
-// The second method argument is a short human-readable description of the
-// checker.
+// The first argument of this templated method is the full name of the checker
+// (including its package), while the second argument is a short description
+// that is printed by `-analyzer-checker-help`.
//
-// The clang_registerCheckers function may add any number of checkers to the
-// registry. If any checkers require additional initialization, use the three-
-// argument form of CheckerRegistry::addChecker.
+// A plugin may register several separate checkers by calling `addChecker()`
+// multiple times. If a checker requires custom registration functions (e.g.
+// checker option handling) use the non-templated variant of `addChecker` that
+// takes two callback functions as the first two parameters.
//
// To load a checker plugin, specify the full path to the dynamic library as
// the argument to the -load option in the cc1 frontend. You can then enable
// your custom checker using the -analyzer-checker:
//
-// clang -cc1 -load </path/to/plugin.dylib> -analyze
-// -analyzer-checker=<example.MainCallChecker>
+// clang -cc1 -load /path/to/plugin.dylib -analyze
+// -analyzer-checker=example.MainCallChecker
//
-// For a complete working example, see examples/analyzer-plugin.
+// For complete examples, see clang/lib/Analysis/plugins/SampleAnalyzer
#ifndef CLANG_ANALYZER_API_VERSION_STRING
// FIXME: The Clang version string is not particularly granular;
@@ -112,26 +112,35 @@ class CheckerRegistry {
return true;
}
-public:
- /// Adds a checker to the registry. Use this non-templated overload when your
- /// checker requires custom initialization.
- void addChecker(RegisterCheckerFn Fn, ShouldRegisterFunction sfn,
+ /// Adds a checker to the registry. This private, most general variant is
+ /// intended for loading the checker definitions from `Checkers.td`.
+ /// FIXME: The checker registr should not bother with loading `DocsUri`
+ /// becaus it is (as of now) never queried from the checker registry.
+ void addChecker(RegisterCheckerFn Fn, ShouldRegisterFunction Sfn,
StringRef FullName, StringRef Desc, StringRef DocsUri,
bool IsHidden);
- /// Adds a checker to the registry. Use this templated overload when your
- /// checker does not require any custom initialization.
- /// This function isn't really needed and probably causes more headaches than
- /// the tiny convenience that it provides, but external plugins might use it,
- /// and there isn't a strong incentive to remove it.
+public:
+ /// Adds a checker to the registry. Use this for a checker defined in a
+ /// plugin if it requires custom registration functions.
+ void addChecker(RegisterCheckerFn Fn, ShouldRegisterFunction Sfn,
+ StringRef FullName, StringRef Desc, bool IsHidden = false) {
+ addChecker(Fn, Sfn, FullName, Desc, "NoDocsUri", IsHidden);
+ }
+
+ /// Adds a checker to the registry. Use this for a checker defined in a
+ /// plugin if it doesn't require custom registration functions.
template <class T>
- void addChecker(StringRef FullName, StringRef Desc, StringRef DocsUri,
- bool IsHidden = false) {
- // Avoid MSVC's Compiler Error C2276:
- // http://msdn.microsoft.com/en-us/library/850cstw1(v=VS.80).aspx
+ void addChecker(StringRef FullName, StringRef Desc, bool IsHidden = false) {
addChecker(&CheckerRegistry::initializeManager<CheckerManager, T>,
- &CheckerRegistry::returnTrue<T>, FullName, Desc, DocsUri,
- IsHidden);
+ &CheckerRegistry::returnTrue<T>, FullName, Desc,
+ /*IsHidden=*/IsHidden);
+ }
+
+ /// Add a mock checker to the registry for testing purposes, without
+ /// specifying metadata that is not relevant in simple tests.
+ template <class T> void addMockChecker(StringRef FullName) {
+ addChecker<T>(FullName, "MockCheckerDescription");
}
/// Makes the checker with the full name \p fullName depend on the checker
diff --git a/clang/lib/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandling.cpp b/clang/lib/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandling.cpp
index aacb886f6e122..6ef6f39848802 100644
--- a/clang/lib/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandling.cpp
+++ b/clang/lib/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandling.cpp
@@ -2,6 +2,9 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+// This barebones plugin is used by clang/test/Analysis/checker-plugins.c
+// to test dependency handling among checkers loaded from plugins.
+
using namespace clang;
using namespace ento;
@@ -15,12 +18,11 @@ struct DependendentChecker : public Checker<check::BeginFunction> {
} // end anonymous namespace
// Register plugin!
-extern "C" void clang_registerCheckers(CheckerRegistry ®istry) {
- registry.addChecker<Dependency>("example.Dependency", "", "");
- registry.addChecker<DependendentChecker>("example.DependendentChecker", "",
- "");
+extern "C" void clang_registerCheckers(CheckerRegistry &Registry) {
+ Registry.addMockChecker<Dependency>("example.Dependency");
+ Registry.addMockChecker<DependendentChecker>("example.DependendentChecker");
- registry.addDependency("example.DependendentChecker", "example.Dependency");
+ Registry.addDependency("example.DependendentChecker", "example.Dependency");
}
extern "C" const char clang_analyzerAPIVersionString[] =
diff --git a/clang/lib/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp b/clang/lib/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp
index 82c1058242551..2adb9348f6715 100644
--- a/clang/lib/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp
+++ b/clang/lib/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp
@@ -5,6 +5,9 @@
using namespace clang;
using namespace ento;
+// This barebones plugin is used by clang/test/Analysis/checker-plugins.c
+// to test option handling on checkers loaded from plugins.
+
namespace {
struct MyChecker : public Checker<check::BeginFunction> {
void checkBeginFunction(CheckerContext &Ctx) const {}
@@ -25,13 +28,11 @@ bool shouldRegisterMyChecker(const CheckerManager &mgr) { return true; }
} // end anonymous namespace
// Register plugin!
-extern "C" void clang_registerCheckers(CheckerRegistry ®istry) {
- registry.addChecker(registerMyChecker, shouldRegisterMyChecker,
- "example.MyChecker", "Example Description",
- "example.mychecker.documentation.nonexistent.html",
- /*isHidden*/false);
+extern "C" void clang_registerCheckers(CheckerRegistry &Registry) {
+ Registry.addChecker(registerMyChecker, shouldRegisterMyChecker,
+ "example.MyChecker", "Example Description");
- registry.addCheckerOption(/*OptionType*/ "bool",
+ Registry.addCheckerOption(/*OptionType*/ "bool",
/*CheckerFullName*/ "example.MyChecker",
/*OptionName*/ "ExampleOption",
/*DefaultValStr*/ "false",
diff --git a/clang/lib/Analysis/plugins/SampleAnalyzer/MainCallChecker.cpp b/clang/lib/Analysis/plugins/SampleAnalyzer/MainCallChecker.cpp
index fd210d733fd0a..5f87670031d7d 100644
--- a/clang/lib/Analysis/plugins/SampleAnalyzer/MainCallChecker.cpp
+++ b/clang/lib/Analysis/plugins/SampleAnalyzer/MainCallChecker.cpp
@@ -3,12 +3,16 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+// This simple plugin is used by clang/test/Analysis/checker-plugins.c
+// to test the use of a checker that is defined in a plugin.
+
using namespace clang;
using namespace ento;
namespace {
class MainCallChecker : public Checker<check::PreStmt<CallExpr>> {
- mutable std::unique_ptr<BugType> BT;
+
+ BugType BT{this, "call to main", "example analyzer plugin"};
public:
void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
@@ -33,21 +37,17 @@ void MainCallChecker::checkPreStmt(const CallExpr *CE,
if (!N)
return;
- if (!BT)
- BT.reset(new BugType(this, "call to main", "example analyzer plugin"));
-
auto report =
- std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N);
+ std::make_unique<PathSensitiveBugReport>(BT, BT.getDescription(), N);
report->addRange(Callee->getSourceRange());
C.emitReport(std::move(report));
}
}
// Register plugin!
-extern "C" void clang_registerCheckers(CheckerRegistry ®istry) {
- registry.addChecker<MainCallChecker>(
- "example.MainCallChecker", "Disallows calls to functions called main",
- "");
+extern "C" void clang_registerCheckers(CheckerRegistry &Registry) {
+ Registry.addChecker<MainCallChecker>("example.MainCallChecker",
+ "Example Description");
}
extern "C" const char clang_analyzerAPIVersionString[] =
diff --git a/clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp b/clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp
index 0f05c39df93e0..d15bec02879f2 100644
--- a/clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp
+++ b/clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp
@@ -91,8 +91,7 @@ void addBlockEntranceTester(AnalysisASTConsumer &AnalysisConsumer,
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
Registry.addChecker(®isterChecker<BlockEntranceCallbackTester>,
&shouldAlwaysRegister, "test.BlockEntranceTester",
- "EmptyDescription", "EmptyDocsUri",
- /*IsHidden=*/false);
+ "EmptyDescription");
});
}
@@ -102,8 +101,7 @@ void addBranchConditionTester(AnalysisASTConsumer &AnalysisConsumer,
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
Registry.addChecker(®isterChecker<BranchConditionCallbackTester>,
&shouldAlwaysRegister, "test.BranchConditionTester",
- "EmptyDescription", "EmptyDocsUri",
- /*IsHidden=*/false);
+ "EmptyDescription");
});
}
diff --git a/clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp b/clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp
index 0ef63b049621e..5595363f669f9 100644
--- a/clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp
+++ b/clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp
@@ -119,8 +119,8 @@ class TestAction : public ASTFrontendAction {
std::make_unique<VerifyPathDiagnosticConsumer>(
std::move(ExpectedDiags), Compiler.getSourceManager()));
AnalysisConsumer->AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<InterestingnessTestChecker>("test.Interestingness",
- "Description", "");
+ Registry.addMockChecker<InterestingnessTestChecker>(
+ "test.Interestingness");
});
Compiler.getAnalyzerOpts().CheckersAndPackages = {
{"test.Interestingness", true}};
diff --git a/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp b/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
index 4cb6bd34fa36d..44192408921ec 100644
--- a/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
+++ b/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
@@ -616,8 +616,7 @@ void addCallDescChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"test.CallDescChecker", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<CallDescChecker>("test.CallDescChecker", "Description",
- "");
+ Registry.addMockChecker<CallDescChecker>("test.CallDescChecker");
});
}
diff --git a/clang/unittests/StaticAnalyzer/CallEventTest.cpp b/clang/unittests/StaticAnalyzer/CallEventTest.cpp
index 2843572e5f800..22c50d168af99 100644
--- a/clang/unittests/StaticAnalyzer/CallEventTest.cpp
+++ b/clang/unittests/StaticAnalyzer/CallEventTest.cpp
@@ -55,8 +55,7 @@ void addCXXDeallocatorChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"test.CXXDeallocator", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<CXXDeallocatorChecker>("test.CXXDeallocator",
- "Description", "");
+ Registry.addMockChecker<CXXDeallocatorChecker>("test.CXXDeallocator");
});
}
diff --git a/clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp b/clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp
index e410cca076637..b11eeb1d290b6 100644
--- a/clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp
+++ b/clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp
@@ -33,10 +33,8 @@ void addEvalFooCheckers(AnalysisASTConsumer &AnalysisConsumer,
AnOpts.CheckersAndPackages = {{"test.EvalFoo1", true},
{"test.EvalFoo2", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<EvalCallFoo1>("test.EvalFoo1", "EmptyDescription",
- "EmptyDocsUri");
- Registry.addChecker<EvalCallFoo2>("test.EvalFoo2", "EmptyDescription",
- "EmptyDocsUri");
+ Registry.addMockChecker<EvalCallFoo1>("test.EvalFoo1");
+ Registry.addMockChecker<EvalCallFoo2>("test.EvalFoo2");
});
}
} // namespace
diff --git a/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp b/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp
index b6eeb9ce37386..94ebbf878b92f 100644
--- a/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp
+++ b/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp
@@ -77,8 +77,8 @@ void addExprEngineVisitPreChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"ExprEngineVisitPreChecker", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<ExprEngineVisitPreChecker>("ExprEngineVisitPreChecker",
- "Desc", "DocsURI");
+ Registry.addMockChecker<ExprEngineVisitPreChecker>(
+ "ExprEngineVisitPreChecker");
});
}
@@ -86,8 +86,8 @@ void addExprEngineVisitPostChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"ExprEngineVisitPostChecker", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<ExprEngineVisitPostChecker>(
- "ExprEngineVisitPostChecker", "Desc", "DocsURI");
+ Registry.addMockChecker<ExprEngineVisitPostChecker>(
+ "ExprEngineVisitPostChecker");
});
}
@@ -95,8 +95,7 @@ void addMemAccessChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"MemAccessChecker", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<MemAccessChecker>("MemAccessChecker", "Desc",
- "DocsURI");
+ Registry.addMockChecker<MemAccessChecker>("MemAccessChecker");
});
}
diff --git a/clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp b/clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
index 8f0a96d41e752..44bd9f2821608 100644
--- a/clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
+++ b/clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
@@ -92,8 +92,8 @@ void addFalsePositiveGenerator(AnalysisASTConsumer &AnalysisConsumer,
AnOpts.CheckersAndPackages = {{"test.FalsePositiveGenerator", true},
{"debug.ViewExplodedGraph", false}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<FalsePositiveGenerator>(
- "test.FalsePositiveGenerator", "EmptyDescription", "EmptyDocsUri");
+ Registry.addMockChecker<FalsePositiveGenerator>(
+ "test.FalsePositiveGenerator");
});
}
diff --git a/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp b/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp
index 0f6e49bf42f4a..cfd0c8cf971c6 100644
--- a/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp
+++ b/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp
@@ -45,8 +45,7 @@ void addDescriptiveNameChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"DescriptiveNameChecker", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<DescriptiveNameChecker>("DescriptiveNameChecker",
- "Desc", "DocsURI");
+ Registry.addMockChecker<DescriptiveNameChecker>("DescriptiveNameChecker");
});
}
diff --git a/clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp b/clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp
index a9033425dfb51..42f91af551aba 100644
--- a/clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp
+++ b/clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp
@@ -138,9 +138,9 @@ void addNonThoroughStatefulChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"test.StatefulChecker", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry
- .addChecker<StatefulChecker<NonThoroughErrorNotPreventedFuncVisitor>>(
- "test.StatefulChecker", "Description", "");
+ Registry.addMockChecker<
+ StatefulChecker<NonThoroughErrorNotPreventedFuncVisitor>>(
+ "test.StatefulChecker");
});
}
@@ -232,8 +232,9 @@ void addThoroughStatefulChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"test.StatefulChecker", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<StatefulChecker<ThoroughErrorNotPreventedFuncVisitor>>(
- "test.StatefulChecker", "Description", "");
+ Registry
+ .addMockChecker<StatefulChecker<ThoroughErrorNotPreventedFuncVisitor>>(
+ "test.StatefulChecker");
});
}
diff --git a/clang/unittests/StaticAnalyzer/ObjcBug-124477.cpp b/clang/unittests/StaticAnalyzer/ObjcBug-124477.cpp
index 51bd33210032c..7a6d74faec9aa 100644
--- a/clang/unittests/StaticAnalyzer/ObjcBug-124477.cpp
+++ b/clang/unittests/StaticAnalyzer/ObjcBug-124477.cpp
@@ -36,8 +36,8 @@ void addFlagFlipperChecker(AnalysisASTConsumer &AnalysisConsumer,...
[truncated]
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Donát Nagy (NagyDonat) ChangesThis commit tweaks the interface of
In addition to propagating these changes (e.g. using I originally wrote the bulk of this commit when I was planning to add an extra parameter to This commit is mostly NFC: the only functional change is that the public checker registration functions no longer require (or accept) Patch is 27.34 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/147797.diff 18 Files Affected:
diff --git a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
index 43dbfb1585151..91871c13c9c83 100644
--- a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -38,29 +38,29 @@
// function clang_registerCheckers. For example:
//
// extern "C"
-// void clang_registerCheckers (CheckerRegistry ®istry) {
-// registry.addChecker<MainCallChecker>("example.MainCallChecker",
-// "Disallows calls to functions called main");
+// void clang_registerCheckers(CheckerRegistry &Registry) {
+// Registry.addChecker<MainCallChecker>(
+// "example.MainCallChecker",
+// "Disallows calls to functions called main");
// }
//
-// The first method argument is the full name of the checker, including its
-// enclosing package. By convention, the registered name of a checker is the
-// name of the associated class (the template argument).
-// The second method argument is a short human-readable description of the
-// checker.
+// The first argument of this templated method is the full name of the checker
+// (including its package), while the second argument is a short description
+// that is printed by `-analyzer-checker-help`.
//
-// The clang_registerCheckers function may add any number of checkers to the
-// registry. If any checkers require additional initialization, use the three-
-// argument form of CheckerRegistry::addChecker.
+// A plugin may register several separate checkers by calling `addChecker()`
+// multiple times. If a checker requires custom registration functions (e.g.
+// checker option handling) use the non-templated variant of `addChecker` that
+// takes two callback functions as the first two parameters.
//
// To load a checker plugin, specify the full path to the dynamic library as
// the argument to the -load option in the cc1 frontend. You can then enable
// your custom checker using the -analyzer-checker:
//
-// clang -cc1 -load </path/to/plugin.dylib> -analyze
-// -analyzer-checker=<example.MainCallChecker>
+// clang -cc1 -load /path/to/plugin.dylib -analyze
+// -analyzer-checker=example.MainCallChecker
//
-// For a complete working example, see examples/analyzer-plugin.
+// For complete examples, see clang/lib/Analysis/plugins/SampleAnalyzer
#ifndef CLANG_ANALYZER_API_VERSION_STRING
// FIXME: The Clang version string is not particularly granular;
@@ -112,26 +112,35 @@ class CheckerRegistry {
return true;
}
-public:
- /// Adds a checker to the registry. Use this non-templated overload when your
- /// checker requires custom initialization.
- void addChecker(RegisterCheckerFn Fn, ShouldRegisterFunction sfn,
+ /// Adds a checker to the registry. This private, most general variant is
+ /// intended for loading the checker definitions from `Checkers.td`.
+ /// FIXME: The checker registr should not bother with loading `DocsUri`
+ /// becaus it is (as of now) never queried from the checker registry.
+ void addChecker(RegisterCheckerFn Fn, ShouldRegisterFunction Sfn,
StringRef FullName, StringRef Desc, StringRef DocsUri,
bool IsHidden);
- /// Adds a checker to the registry. Use this templated overload when your
- /// checker does not require any custom initialization.
- /// This function isn't really needed and probably causes more headaches than
- /// the tiny convenience that it provides, but external plugins might use it,
- /// and there isn't a strong incentive to remove it.
+public:
+ /// Adds a checker to the registry. Use this for a checker defined in a
+ /// plugin if it requires custom registration functions.
+ void addChecker(RegisterCheckerFn Fn, ShouldRegisterFunction Sfn,
+ StringRef FullName, StringRef Desc, bool IsHidden = false) {
+ addChecker(Fn, Sfn, FullName, Desc, "NoDocsUri", IsHidden);
+ }
+
+ /// Adds a checker to the registry. Use this for a checker defined in a
+ /// plugin if it doesn't require custom registration functions.
template <class T>
- void addChecker(StringRef FullName, StringRef Desc, StringRef DocsUri,
- bool IsHidden = false) {
- // Avoid MSVC's Compiler Error C2276:
- // http://msdn.microsoft.com/en-us/library/850cstw1(v=VS.80).aspx
+ void addChecker(StringRef FullName, StringRef Desc, bool IsHidden = false) {
addChecker(&CheckerRegistry::initializeManager<CheckerManager, T>,
- &CheckerRegistry::returnTrue<T>, FullName, Desc, DocsUri,
- IsHidden);
+ &CheckerRegistry::returnTrue<T>, FullName, Desc,
+ /*IsHidden=*/IsHidden);
+ }
+
+ /// Add a mock checker to the registry for testing purposes, without
+ /// specifying metadata that is not relevant in simple tests.
+ template <class T> void addMockChecker(StringRef FullName) {
+ addChecker<T>(FullName, "MockCheckerDescription");
}
/// Makes the checker with the full name \p fullName depend on the checker
diff --git a/clang/lib/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandling.cpp b/clang/lib/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandling.cpp
index aacb886f6e122..6ef6f39848802 100644
--- a/clang/lib/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandling.cpp
+++ b/clang/lib/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandling.cpp
@@ -2,6 +2,9 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+// This barebones plugin is used by clang/test/Analysis/checker-plugins.c
+// to test dependency handling among checkers loaded from plugins.
+
using namespace clang;
using namespace ento;
@@ -15,12 +18,11 @@ struct DependendentChecker : public Checker<check::BeginFunction> {
} // end anonymous namespace
// Register plugin!
-extern "C" void clang_registerCheckers(CheckerRegistry ®istry) {
- registry.addChecker<Dependency>("example.Dependency", "", "");
- registry.addChecker<DependendentChecker>("example.DependendentChecker", "",
- "");
+extern "C" void clang_registerCheckers(CheckerRegistry &Registry) {
+ Registry.addMockChecker<Dependency>("example.Dependency");
+ Registry.addMockChecker<DependendentChecker>("example.DependendentChecker");
- registry.addDependency("example.DependendentChecker", "example.Dependency");
+ Registry.addDependency("example.DependendentChecker", "example.Dependency");
}
extern "C" const char clang_analyzerAPIVersionString[] =
diff --git a/clang/lib/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp b/clang/lib/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp
index 82c1058242551..2adb9348f6715 100644
--- a/clang/lib/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp
+++ b/clang/lib/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp
@@ -5,6 +5,9 @@
using namespace clang;
using namespace ento;
+// This barebones plugin is used by clang/test/Analysis/checker-plugins.c
+// to test option handling on checkers loaded from plugins.
+
namespace {
struct MyChecker : public Checker<check::BeginFunction> {
void checkBeginFunction(CheckerContext &Ctx) const {}
@@ -25,13 +28,11 @@ bool shouldRegisterMyChecker(const CheckerManager &mgr) { return true; }
} // end anonymous namespace
// Register plugin!
-extern "C" void clang_registerCheckers(CheckerRegistry ®istry) {
- registry.addChecker(registerMyChecker, shouldRegisterMyChecker,
- "example.MyChecker", "Example Description",
- "example.mychecker.documentation.nonexistent.html",
- /*isHidden*/false);
+extern "C" void clang_registerCheckers(CheckerRegistry &Registry) {
+ Registry.addChecker(registerMyChecker, shouldRegisterMyChecker,
+ "example.MyChecker", "Example Description");
- registry.addCheckerOption(/*OptionType*/ "bool",
+ Registry.addCheckerOption(/*OptionType*/ "bool",
/*CheckerFullName*/ "example.MyChecker",
/*OptionName*/ "ExampleOption",
/*DefaultValStr*/ "false",
diff --git a/clang/lib/Analysis/plugins/SampleAnalyzer/MainCallChecker.cpp b/clang/lib/Analysis/plugins/SampleAnalyzer/MainCallChecker.cpp
index fd210d733fd0a..5f87670031d7d 100644
--- a/clang/lib/Analysis/plugins/SampleAnalyzer/MainCallChecker.cpp
+++ b/clang/lib/Analysis/plugins/SampleAnalyzer/MainCallChecker.cpp
@@ -3,12 +3,16 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+// This simple plugin is used by clang/test/Analysis/checker-plugins.c
+// to test the use of a checker that is defined in a plugin.
+
using namespace clang;
using namespace ento;
namespace {
class MainCallChecker : public Checker<check::PreStmt<CallExpr>> {
- mutable std::unique_ptr<BugType> BT;
+
+ BugType BT{this, "call to main", "example analyzer plugin"};
public:
void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
@@ -33,21 +37,17 @@ void MainCallChecker::checkPreStmt(const CallExpr *CE,
if (!N)
return;
- if (!BT)
- BT.reset(new BugType(this, "call to main", "example analyzer plugin"));
-
auto report =
- std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N);
+ std::make_unique<PathSensitiveBugReport>(BT, BT.getDescription(), N);
report->addRange(Callee->getSourceRange());
C.emitReport(std::move(report));
}
}
// Register plugin!
-extern "C" void clang_registerCheckers(CheckerRegistry ®istry) {
- registry.addChecker<MainCallChecker>(
- "example.MainCallChecker", "Disallows calls to functions called main",
- "");
+extern "C" void clang_registerCheckers(CheckerRegistry &Registry) {
+ Registry.addChecker<MainCallChecker>("example.MainCallChecker",
+ "Example Description");
}
extern "C" const char clang_analyzerAPIVersionString[] =
diff --git a/clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp b/clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp
index 0f05c39df93e0..d15bec02879f2 100644
--- a/clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp
+++ b/clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp
@@ -91,8 +91,7 @@ void addBlockEntranceTester(AnalysisASTConsumer &AnalysisConsumer,
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
Registry.addChecker(®isterChecker<BlockEntranceCallbackTester>,
&shouldAlwaysRegister, "test.BlockEntranceTester",
- "EmptyDescription", "EmptyDocsUri",
- /*IsHidden=*/false);
+ "EmptyDescription");
});
}
@@ -102,8 +101,7 @@ void addBranchConditionTester(AnalysisASTConsumer &AnalysisConsumer,
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
Registry.addChecker(®isterChecker<BranchConditionCallbackTester>,
&shouldAlwaysRegister, "test.BranchConditionTester",
- "EmptyDescription", "EmptyDocsUri",
- /*IsHidden=*/false);
+ "EmptyDescription");
});
}
diff --git a/clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp b/clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp
index 0ef63b049621e..5595363f669f9 100644
--- a/clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp
+++ b/clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp
@@ -119,8 +119,8 @@ class TestAction : public ASTFrontendAction {
std::make_unique<VerifyPathDiagnosticConsumer>(
std::move(ExpectedDiags), Compiler.getSourceManager()));
AnalysisConsumer->AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<InterestingnessTestChecker>("test.Interestingness",
- "Description", "");
+ Registry.addMockChecker<InterestingnessTestChecker>(
+ "test.Interestingness");
});
Compiler.getAnalyzerOpts().CheckersAndPackages = {
{"test.Interestingness", true}};
diff --git a/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp b/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
index 4cb6bd34fa36d..44192408921ec 100644
--- a/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
+++ b/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
@@ -616,8 +616,7 @@ void addCallDescChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"test.CallDescChecker", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<CallDescChecker>("test.CallDescChecker", "Description",
- "");
+ Registry.addMockChecker<CallDescChecker>("test.CallDescChecker");
});
}
diff --git a/clang/unittests/StaticAnalyzer/CallEventTest.cpp b/clang/unittests/StaticAnalyzer/CallEventTest.cpp
index 2843572e5f800..22c50d168af99 100644
--- a/clang/unittests/StaticAnalyzer/CallEventTest.cpp
+++ b/clang/unittests/StaticAnalyzer/CallEventTest.cpp
@@ -55,8 +55,7 @@ void addCXXDeallocatorChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"test.CXXDeallocator", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<CXXDeallocatorChecker>("test.CXXDeallocator",
- "Description", "");
+ Registry.addMockChecker<CXXDeallocatorChecker>("test.CXXDeallocator");
});
}
diff --git a/clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp b/clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp
index e410cca076637..b11eeb1d290b6 100644
--- a/clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp
+++ b/clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp
@@ -33,10 +33,8 @@ void addEvalFooCheckers(AnalysisASTConsumer &AnalysisConsumer,
AnOpts.CheckersAndPackages = {{"test.EvalFoo1", true},
{"test.EvalFoo2", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<EvalCallFoo1>("test.EvalFoo1", "EmptyDescription",
- "EmptyDocsUri");
- Registry.addChecker<EvalCallFoo2>("test.EvalFoo2", "EmptyDescription",
- "EmptyDocsUri");
+ Registry.addMockChecker<EvalCallFoo1>("test.EvalFoo1");
+ Registry.addMockChecker<EvalCallFoo2>("test.EvalFoo2");
});
}
} // namespace
diff --git a/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp b/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp
index b6eeb9ce37386..94ebbf878b92f 100644
--- a/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp
+++ b/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp
@@ -77,8 +77,8 @@ void addExprEngineVisitPreChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"ExprEngineVisitPreChecker", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<ExprEngineVisitPreChecker>("ExprEngineVisitPreChecker",
- "Desc", "DocsURI");
+ Registry.addMockChecker<ExprEngineVisitPreChecker>(
+ "ExprEngineVisitPreChecker");
});
}
@@ -86,8 +86,8 @@ void addExprEngineVisitPostChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"ExprEngineVisitPostChecker", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<ExprEngineVisitPostChecker>(
- "ExprEngineVisitPostChecker", "Desc", "DocsURI");
+ Registry.addMockChecker<ExprEngineVisitPostChecker>(
+ "ExprEngineVisitPostChecker");
});
}
@@ -95,8 +95,7 @@ void addMemAccessChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"MemAccessChecker", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<MemAccessChecker>("MemAccessChecker", "Desc",
- "DocsURI");
+ Registry.addMockChecker<MemAccessChecker>("MemAccessChecker");
});
}
diff --git a/clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp b/clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
index 8f0a96d41e752..44bd9f2821608 100644
--- a/clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
+++ b/clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
@@ -92,8 +92,8 @@ void addFalsePositiveGenerator(AnalysisASTConsumer &AnalysisConsumer,
AnOpts.CheckersAndPackages = {{"test.FalsePositiveGenerator", true},
{"debug.ViewExplodedGraph", false}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<FalsePositiveGenerator>(
- "test.FalsePositiveGenerator", "EmptyDescription", "EmptyDocsUri");
+ Registry.addMockChecker<FalsePositiveGenerator>(
+ "test.FalsePositiveGenerator");
});
}
diff --git a/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp b/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp
index 0f6e49bf42f4a..cfd0c8cf971c6 100644
--- a/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp
+++ b/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp
@@ -45,8 +45,7 @@ void addDescriptiveNameChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"DescriptiveNameChecker", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<DescriptiveNameChecker>("DescriptiveNameChecker",
- "Desc", "DocsURI");
+ Registry.addMockChecker<DescriptiveNameChecker>("DescriptiveNameChecker");
});
}
diff --git a/clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp b/clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp
index a9033425dfb51..42f91af551aba 100644
--- a/clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp
+++ b/clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp
@@ -138,9 +138,9 @@ void addNonThoroughStatefulChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"test.StatefulChecker", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry
- .addChecker<StatefulChecker<NonThoroughErrorNotPreventedFuncVisitor>>(
- "test.StatefulChecker", "Description", "");
+ Registry.addMockChecker<
+ StatefulChecker<NonThoroughErrorNotPreventedFuncVisitor>>(
+ "test.StatefulChecker");
});
}
@@ -232,8 +232,9 @@ void addThoroughStatefulChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"test.StatefulChecker", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
- Registry.addChecker<StatefulChecker<ThoroughErrorNotPreventedFuncVisitor>>(
- "test.StatefulChecker", "Description", "");
+ Registry
+ .addMockChecker<StatefulChecker<ThoroughErrorNotPreventedFuncVisitor>>(
+ "test.StatefulChecker");
});
}
diff --git a/clang/unittests/StaticAnalyzer/ObjcBug-124477.cpp b/clang/unittests/StaticAnalyzer/ObjcBug-124477.cpp
index 51bd33210032c..7a6d74faec9aa 100644
--- a/clang/unittests/StaticAnalyzer/ObjcBug-124477.cpp
+++ b/clang/unittests/StaticAnalyzer/ObjcBug-124477.cpp
@@ -36,8 +36,8 @@ void addFlagFlipperChecker(AnalysisASTConsumer &AnalysisConsumer,...
[truncated]
|
This commit tweaks the interface of
CheckerRegistry::addChecker
to make it more practical for plugins and tests:DocsUri
appears in the parameter list) is turned into a private method which is only used for loading the checkers described inCheckers.td
. Note that currently nothing queries the documentation URI from the checker registry (it's only used by the logic that generates the clang-tidy documentation, but that loads it directly fromCheckers.td
without involving theCheckerRegistry
), so there is no reason to require anyone to provide this value. (In fact, it may be a good idea to remove it completely from theCheckerRegistry
.)DocsUri
argument and making theIsHidden
parameter optional (by setting it tofalse
by default). This will cover the needs of plugins that want maximal customization.DocsUri
parameter.addMockChecker<T>
is added for use in unit tests.In addition to propagating these changes (e.g. using
addMockChecker
in unittests), this commit clarifies, corrects and extends lots of comments and performs various minor code quality improvements in the code of unittests and example plugins.I originally wrote the bulk of this commit when I was planning to add an extra parameter to
addChecker
in order to implement some technical details of the CheckerFamily framework. At the end I decided against adding that extra parameter, so this cleanup was left out of the PR #139256 and I'm merging it now as a separate commit (after minor tweaks).This commit is mostly NFC: the only functional change is that the public checker registration functions no longer require (or accept)
DocsUri
as an argument, and theIsHidden
argument is optional for both public overloads ofaddChecker
(and not just the "basic" templated one).