[clang] [NFC][analyzer] Use `CheckerBase::getName` in checker option handling (PR #131612)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 17 06:24:26 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Donát Nagy (NagyDonat)
<details>
<summary>Changes</summary>
The virtual method `ProgramPointTag::getTagDescription` had two very distinct use cases:
- It is printed in the DOT graph visualization of the exploded graph (that is, a debug printout).
- The checker option handling code used it to query the name of a checker, which relied on the coincidence that in `CheckerBase` this method is defined to be equivalent with `getName()`.
This commit switches to using `getName` in the second use case, because this way we will be able to properly support checkers that have multiple (separately named) parts.
The method `reportInvalidCheckerOptionName` is extended with an additional overload that allows specifying the `CheckerPartIdx`. The methods `getChecker*Option` could be extended analogously in the future, but they are just convenience wrappers around the variants that directly take `StringRef CheckerName`, so I'll only do this extension if it's needed.
---
Full diff: https://github.com/llvm/llvm-project/pull/131612.diff
6 Files Affected:
- (modified) clang/include/clang/Analysis/ProgramPoint.h (+3)
- (modified) clang/include/clang/StaticAnalyzer/Core/Checker.h (+5-12)
- (modified) clang/include/clang/StaticAnalyzer/Core/CheckerManager.h (+7)
- (modified) clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp (+3-6)
- (modified) clang/lib/StaticAnalyzer/Core/CheckerManager.cpp (+2-2)
- (modified) clang/unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp (+6-31)
``````````diff
diff --git a/clang/include/clang/Analysis/ProgramPoint.h b/clang/include/clang/Analysis/ProgramPoint.h
index 1df1f1cb892e4..c40aa3d8ffb72 100644
--- a/clang/include/clang/Analysis/ProgramPoint.h
+++ b/clang/include/clang/Analysis/ProgramPoint.h
@@ -39,6 +39,9 @@ class ProgramPointTag {
public:
ProgramPointTag(void *tagKind = nullptr) : TagKind(tagKind) {}
virtual ~ProgramPointTag();
+
+ /// The description of this program point which will be displayed when the
+ /// ExplodedGraph is dumped in DOT format for debugging.
virtual StringRef getTagDescription() const = 0;
/// Used to implement 'isKind' in subclasses.
diff --git a/clang/include/clang/StaticAnalyzer/Core/Checker.h b/clang/include/clang/StaticAnalyzer/Core/Checker.h
index df29be8ba3f62..a54c5bee612f6 100644
--- a/clang/include/clang/StaticAnalyzer/Core/Checker.h
+++ b/clang/include/clang/StaticAnalyzer/Core/Checker.h
@@ -516,18 +516,11 @@ class CheckerBase : public ProgramPointTag {
}
StringRef getTagDescription() const override {
- // This method inherited from `ProgramPointTag` has two unrelated roles:
- // (1) The analyzer option handling logic uses this method to query the
- // name of a checker.
- // (2) When the `ExplodedGraph` is dumped in DOT format for debugging,
- // this is called to attach a description to the nodes. (This happens
- // for all subclasses of `ProgramPointTag`, not just checkers.)
- // FIXME: Application (1) should be aware of multiple parts within the same
- // checker class instance, so it should directly use `getName` instead of
- // this inherited interface which cannot support a `CheckerPartIdx`.
- // FIXME: Ideally application (2) should return a string that describes the
- // whole checker class, not just one of it parts. However, this is only for
- // debugging, so returning the name of one part is probably good enough.
+ // When the ExplodedGraph is dumped for debugging (in DOT format), this
+ // method is called to attach a description to nodes created by this
+ // checker _class_. Ideally this should be recognizable identifier of the
+ // whole class, but for this debugging purpose it's sufficient to use the
+ // name of the first registered checker part.
for (const auto &OptName : RegisteredNames)
if (OptName)
return *OptName;
diff --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 44ae3ebe3d09d..03ffadd346d0b 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -194,6 +194,13 @@ class CheckerManager {
/// Emits an error through a DiagnosticsEngine about an invalid user supplied
/// checker option value.
void reportInvalidCheckerOptionValue(const CheckerBase *C,
+ StringRef OptionName,
+ StringRef ExpectedValueDesc) const {
+ reportInvalidCheckerOptionValue(C, DefaultPart, OptionName,
+ ExpectedValueDesc);
+ }
+
+ void reportInvalidCheckerOptionValue(const CheckerBase *C, CheckerPartIdx Idx,
StringRef OptionName,
StringRef ExpectedValueDesc) const;
diff --git a/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
index 86ef4a5686650..3e0414d00f551 100644
--- a/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ b/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -154,8 +154,7 @@ StringRef AnalyzerOptions::getCheckerStringOption(StringRef CheckerName,
StringRef AnalyzerOptions::getCheckerStringOption(const ento::CheckerBase *C,
StringRef OptionName,
bool SearchInParents) const {
- return getCheckerStringOption(
- C->getTagDescription(), OptionName, SearchInParents);
+ return getCheckerStringOption(C->getName(), OptionName, SearchInParents);
}
bool AnalyzerOptions::getCheckerBooleanOption(StringRef CheckerName,
@@ -178,8 +177,7 @@ bool AnalyzerOptions::getCheckerBooleanOption(StringRef CheckerName,
bool AnalyzerOptions::getCheckerBooleanOption(const ento::CheckerBase *C,
StringRef OptionName,
bool SearchInParents) const {
- return getCheckerBooleanOption(
- C->getTagDescription(), OptionName, SearchInParents);
+ return getCheckerBooleanOption(C->getName(), OptionName, SearchInParents);
}
int AnalyzerOptions::getCheckerIntegerOption(StringRef CheckerName,
@@ -199,6 +197,5 @@ int AnalyzerOptions::getCheckerIntegerOption(StringRef CheckerName,
int AnalyzerOptions::getCheckerIntegerOption(const ento::CheckerBase *C,
StringRef OptionName,
bool SearchInParents) const {
- return getCheckerIntegerOption(
- C->getTagDescription(), OptionName, SearchInParents);
+ return getCheckerIntegerOption(C->getName(), OptionName, SearchInParents);
}
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
index 56b1e44acc1fd..7ae86f133904b 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -50,11 +50,11 @@ bool CheckerManager::hasPathSensitiveCheckers() const {
}
void CheckerManager::reportInvalidCheckerOptionValue(
- const CheckerBase *C, StringRef OptionName,
+ const CheckerBase *C, CheckerPartIdx Idx, StringRef OptionName,
StringRef ExpectedValueDesc) const {
getDiagnostics().Report(diag::err_analyzer_checker_option_invalid_input)
- << (llvm::Twine() + C->getTagDescription() + ":" + OptionName).str()
+ << (llvm::Twine(C->getName(Idx)) + ":" + OptionName).str()
<< ExpectedValueDesc;
}
diff --git a/clang/unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp b/clang/unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
index aace4b991b5ec..f4871b8560522 100644
--- a/clang/unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
+++ b/clang/unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
@@ -37,26 +37,17 @@ TEST(StaticAnalyzerOptions, SearchInParentPackageTests) {
Opts.Config["Outer.Inner:Option2"] = "true";
Opts.Config["Outer:Option2"] = "false";
- struct CheckerOneMock : CheckerBase {
- StringRef getTagDescription() const override {
- return "Outer.Inner.CheckerOne";
- }
- };
- struct CheckerTwoMock : CheckerBase {
- StringRef getTagDescription() const override {
- return "Outer.Inner.CheckerTwo";
- }
- };
+ StringRef CheckerOneName = "Outer.Inner.CheckerOne";
+ StringRef CheckerTwoName = "Outer.Inner.CheckerTwo";
// CheckerTwo one has Option specified as true. It should read true regardless
// of search mode.
- CheckerOneMock CheckerOne;
- EXPECT_TRUE(Opts.getCheckerBooleanOption(&CheckerOne, "Option"));
+ EXPECT_TRUE(Opts.getCheckerBooleanOption(CheckerOneName, "Option"));
// The package option is overridden with a checker option.
- EXPECT_TRUE(Opts.getCheckerBooleanOption(&CheckerOne, "Option", true));
+ EXPECT_TRUE(Opts.getCheckerBooleanOption(CheckerOneName, "Option", true));
// The Outer package option is overridden by the Inner package option. No
// package option is specified.
- EXPECT_TRUE(Opts.getCheckerBooleanOption(&CheckerOne, "Option2", true));
+ EXPECT_TRUE(Opts.getCheckerBooleanOption(CheckerOneName, "Option2", true));
// No package option is specified and search in packages is turned off. We
// should assert here, but we can't test that.
//Opts.getCheckerBooleanOption(&CheckerOne, "Option2");
@@ -64,8 +55,7 @@ TEST(StaticAnalyzerOptions, SearchInParentPackageTests) {
// Checker true has no option specified. It should get false when search in
// parents turned on.
- CheckerTwoMock CheckerTwo;
- EXPECT_FALSE(Opts.getCheckerBooleanOption(&CheckerTwo, "Option", true));
+ EXPECT_FALSE(Opts.getCheckerBooleanOption(CheckerTwoName, "Option", true));
// In any other case, we should assert, that we cannot test unfortunately.
//Opts.getCheckerBooleanOption(&CheckerTwo, "Option");
//Opts.getCheckerBooleanOption(&CheckerTwo, "Option");
@@ -74,21 +64,6 @@ TEST(StaticAnalyzerOptions, SearchInParentPackageTests) {
TEST(StaticAnalyzerOptions, StringOptions) {
AnalyzerOptions Opts;
Opts.Config["Outer.Inner.CheckerOne:Option"] = "StringValue";
-
- struct CheckerOneMock : CheckerBase {
- StringRef getTagDescription() const override {
- return "Outer.Inner.CheckerOne";
- }
- };
-
- CheckerOneMock CheckerOne;
- EXPECT_TRUE("StringValue" ==
- Opts.getCheckerStringOption(&CheckerOne, "Option"));
-}
-
-TEST(StaticAnalyzerOptions, SubCheckerOptions) {
- AnalyzerOptions Opts;
- Opts.Config["Outer.Inner.CheckerOne:Option"] = "StringValue";
EXPECT_TRUE("StringValue" == Opts.getCheckerStringOption(
"Outer.Inner.CheckerOne", "Option"));
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/131612
More information about the cfe-commits
mailing list