[clang] [NFC][analyzer] Rename `CheckerBase::getCheckerName` to `getName` (PR #130953)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/130953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][analyzer] Rename `CheckerBase::getCheckerName` to `getName` (PR #130953)
@@ -41,19 +41,19 @@ class BugType {
Checker(nullptr), SuppressOnSink(SuppressOnSink) {}
BugType(const CheckerBase *Checker, StringRef Desc,
StringRef Cat = categories::LogicError, bool SuppressOnSink = false)
- : CheckerName(Checker->getCheckerName()), Description(Desc),
-Category(Cat), Checker(Checker), SuppressOnSink(SuppressOnSink) {}
+ : CheckerName(), Description(Desc), Category(Cat), Checker(Checker),
NagyDonat wrote:
It calls the default constructor of `CheckerNameRef CheckerName` which
initializes this data member to an empty string.
In fact the two data members `CheckerName` and `Checker` should have been
alternatives within a single `variant` (and not separate data members), because
one of them is always an unused empty value. (The first constructor fills
`CheckerName`, the second one fills `Checker`.)
This `variant`ization of these two data members does happen in follow-up PR
https://github.com/llvm/llvm-project/pull/130985 (which introduces my multipart
checker framework).
https://github.com/llvm/llvm-project/pull/130953
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][analyzer] Rename `CheckerBase::getCheckerName` to `getName` (PR #130953)
@@ -41,19 +41,19 @@ class BugType {
Checker(nullptr), SuppressOnSink(SuppressOnSink) {}
BugType(const CheckerBase *Checker, StringRef Desc,
StringRef Cat = categories::LogicError, bool SuppressOnSink = false)
- : CheckerName(Checker->getCheckerName()), Description(Desc),
-Category(Cat), Checker(Checker), SuppressOnSink(SuppressOnSink) {}
+ : CheckerName(), Description(Desc), Category(Cat), Checker(Checker),
steakhal wrote:
Ah nvm.
https://github.com/llvm/llvm-project/pull/130953
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][analyzer] Rename `CheckerBase::getCheckerName` to `getName` (PR #130953)
@@ -41,19 +41,19 @@ class BugType {
Checker(nullptr), SuppressOnSink(SuppressOnSink) {}
BugType(const CheckerBase *Checker, StringRef Desc,
StringRef Cat = categories::LogicError, bool SuppressOnSink = false)
- : CheckerName(Checker->getCheckerName()), Description(Desc),
-Category(Cat), Checker(Checker), SuppressOnSink(SuppressOnSink) {}
+ : CheckerName(), Description(Desc), Category(Cat), Checker(Checker),
steakhal wrote:
What does this `CheckerName` initialize?
https://github.com/llvm/llvm-project/pull/130953
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][analyzer] Rename `CheckerBase::getCheckerName` to `getName` (PR #130953)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/130953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][analyzer] Rename `CheckerBase::getCheckerName` to `getName` (PR #130953)
https://github.com/NagyDonat closed https://github.com/llvm/llvm-project/pull/130953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][analyzer] Rename `CheckerBase::getCheckerName` to `getName` (PR #130953)
@@ -41,19 +41,19 @@ class BugType {
Checker(nullptr), SuppressOnSink(SuppressOnSink) {}
BugType(const CheckerBase *Checker, StringRef Desc,
StringRef Cat = categories::LogicError, bool SuppressOnSink = false)
- : CheckerName(Checker->getCheckerName()), Description(Desc),
-Category(Cat), Checker(Checker), SuppressOnSink(SuppressOnSink) {}
+ : CheckerName(), Description(Desc), Category(Cat), Checker(Checker),
+SuppressOnSink(SuppressOnSink) {}
virtual ~BugType() = default;
StringRef getDescription() const { return Description; }
StringRef getCategory() const { return Category; }
StringRef getCheckerName() const {
-// FIXME: This is a workaround to ensure that the correct checerk name is
+// FIXME: This is a workaround to ensure that the correct checker name is
// used. The checker names are set after the constructors are run.
// In case the BugType object is initialized in the checker's ctor
// the CheckerName field will be empty. To circumvent this problem we use
// CheckerBase whenever it is possible.
-StringRef Ret = Checker ? Checker->getCheckerName() : CheckerName;
Xazax-hun wrote:
Is `Checker` ever null? Do we still need the `CheckerName` field?
https://github.com/llvm/llvm-project/pull/130953
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][analyzer] Rename `CheckerBase::getCheckerName` to `getName` (PR #130953)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/130953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][analyzer] Rename `CheckerBase::getCheckerName` to `getName` (PR #130953)
@@ -41,19 +41,19 @@ class BugType {
Checker(nullptr), SuppressOnSink(SuppressOnSink) {}
BugType(const CheckerBase *Checker, StringRef Desc,
StringRef Cat = categories::LogicError, bool SuppressOnSink = false)
- : CheckerName(Checker->getCheckerName()), Description(Desc),
-Category(Cat), Checker(Checker), SuppressOnSink(SuppressOnSink) {}
+ : CheckerName(), Description(Desc), Category(Cat), Checker(Checker),
+SuppressOnSink(SuppressOnSink) {}
virtual ~BugType() = default;
StringRef getDescription() const { return Description; }
StringRef getCategory() const { return Category; }
StringRef getCheckerName() const {
-// FIXME: This is a workaround to ensure that the correct checerk name is
+// FIXME: This is a workaround to ensure that the correct checker name is
// used. The checker names are set after the constructors are run.
// In case the BugType object is initialized in the checker's ctor
// the CheckerName field will be empty. To circumvent this problem we use
// CheckerBase whenever it is possible.
-StringRef Ret = Checker ? Checker->getCheckerName() : CheckerName;
NagyDonat wrote:
Under the current system we still need `CheckerName`, but we will be able to
remove it when I implement my new multipart checker framework and all multipart
checkers are transferred to it.
Right now both constructors of `BugType` are used and needed:
- In simple straightforward checkers `Checker->getCheckerName()` can return the
(single) name of the checker, so we can directly initialize the bug types as
`BugType BT{this, ...}` with the constructor which takes `CheckerBase *Checker`
and default initializes `CheckerName`.
- In multipart checkers the `BugType` wouldn't be able to query the name of the
relevant checker part from a `CheckerBase *` (because currently each multipart
checker implements its own custom array of checker names), so we need to
initialize the `BugType` with a `CheckerNameRef`, which requires lazy
initialization (the ugly `mutable std::unique_ptr`) because the names
of the sub-checkers are not yet available when the checker object is
constructed.
The main effect of my multipart checker change is (roughly speaking) that I
will provide a `CheckerBase::getName(unsigned CheckerPartIdx)` and then
`BugType`s can exactly reference a checker part with a pair of a `CheckerBase*`
(the singleton instance of the checker class) and a numerical index (which
identifies the part). After this, we can directly initialize each bug type
object with either a `CheckerBase*` (simple checkers) or a `` pair (multipart checkers), so we can get rid of the `BugType`
constructor that takes a raw `CheckerName` (and needs the awkward lazy
initialization).
https://github.com/llvm/llvm-project/pull/130953
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][analyzer] Rename `CheckerBase::getCheckerName` to `getName` (PR #130953)
https://github.com/Xazax-hun approved this pull request. https://github.com/llvm/llvm-project/pull/130953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][analyzer] Rename `CheckerBase::getCheckerName` to `getName` (PR #130953)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/130953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][analyzer] Rename `CheckerBase::getCheckerName` to `getName` (PR #130953)
llvmbot wrote:
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Donát Nagy (NagyDonat)
Changes
The method name `getCheckerName` would imply "get the name of the checker
associated with `this`", so it's suitable for e.g. `BugType::getCheckerName` --
but the proper name for a method that "gets the name of `this`" should be
simply called `getName`.
This change eliminates the redundant and ugly pattern
`Checker->getCheckerName()` and helps to visually distinguish this method
from `BugType::getCheckerName`.
In the constructor of `BugType` the call of this method was completely removed
(instead of just changing the name) because that call was dead code (when the
data member `Checker` is non-null, the string stored in `CheckerName` is
irrelevant) and was often querying the name of the checker before it was
properly initialized.
Moreover, in `ReturnValueChecker.cpp` the static helper function `getName`
(which gets a function name from a `CallEvent`) was renamed to the more
specific `getFunctionName` to avoid the name collision.
This change is yet another cleanup commit before my planned changes that would
add support for multi-part checkers to this method.
---
Full diff: https://github.com/llvm/llvm-project/pull/130953.diff
7 Files Affected:
- (modified) clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
(+4-4)
- (modified) clang/include/clang/StaticAnalyzer/Core/Checker.h (+1-1)
- (modified) clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp (+2-3)
- (modified) clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp (+3-3)
- (modified) clang/lib/StaticAnalyzer/Core/BugReporter.cpp (+2-2)
- (modified) clang/lib/StaticAnalyzer/Core/Checker.cpp (+4-4)
- (modified) clang/lib/StaticAnalyzer/Core/CheckerManager.cpp (+5-6)
``diff
diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
index e50afd6d0da7e..97cf7eda0ae2f 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
@@ -41,19 +41,19 @@ class BugType {
Checker(nullptr), SuppressOnSink(SuppressOnSink) {}
BugType(const CheckerBase *Checker, StringRef Desc,
StringRef Cat = categories::LogicError, bool SuppressOnSink = false)
- : CheckerName(Checker->getCheckerName()), Description(Desc),
-Category(Cat), Checker(Checker), SuppressOnSink(SuppressOnSink) {}
+ : CheckerName(), Description(Desc), Category(Cat), Checker(Checker),
+SuppressOnSink(SuppressOnSink) {}
virtual ~BugType() = default;
StringRef getDescription() const { return Description; }
StringRef getCategory() const { return Category; }
StringRef getCheckerName() const {
-// FIXME: This is a workaround to ensure that the correct checerk name is
+// FIXME: This is a workaround to ensure that the correct checker name is
// used. The checker names are set after the constructors are run.
// In case the BugType object is initialized in the checker's ctor
// the CheckerName field will be empty. To circumvent this problem we use
// CheckerBase whenever it is possible.
-StringRef Ret = Checker ? Checker->getCheckerName() : CheckerName;
+StringRef Ret = Checker ? Checker->getName() : CheckerName;
assert(!Ret.empty() && "Checker name is not set properly.");
return Ret;
}
diff --git a/clang/include/clang/StaticAnalyzer/Core/Checker.h
b/clang/include/clang/StaticAnalyzer/Core/Checker.h
index 2ec54a837c42c..4d9b33cc559b8 100644
--- a/clang/include/clang/StaticAnalyzer/Core/Checker.h
+++ b/clang/include/clang/StaticAnalyzer/Core/Checker.h
@@ -490,7 +490,7 @@ class CheckerBase : public ProgramPointTag {
public:
StringRef getTagDescription() const override;
- CheckerNameRef getCheckerName() const;
+ CheckerNameRef getName() const;
/// See CheckerManager::runCheckersForPrintState.
virtual void printState(raw_ostream &Out, ProgramStateRef State,
diff --git a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
index 1b87a0d8af38d..6035e2d34c2b3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -430,9 +430,8 @@ void ExprInspectionChecker::analyzerHashDump(const CallExpr
*CE,
const LangOptions &Opts = C.getLangOpts();
const SourceManager &SM = C.getSourceManager();
FullSourceLoc FL(CE->getArg(0)->getBeginLoc(), SM);
- std::string HashContent =
- getIssueString(FL, getCheckerName(), "Category",
- C.getLocationContext()->getDecl(), Opts);
+ std::string HashContent = getIssueString(
+ FL, getName(), "Category", C.getLocationContext()->getDecl(), Opts);
reportBug(HashContent, C);
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
b/clang/lib
[clang] [NFC][analyzer] Rename `CheckerBase::getCheckerName` to `getName` (PR #130953)
https://github.com/NagyDonat created
https://github.com/llvm/llvm-project/pull/130953
The method name `getCheckerName` would imply "get the name of the checker
associated with `this`", so it's suitable for e.g. `BugType::getCheckerName` --
but the proper name for a method that "gets the name of `this`" should be
simply called `getName`.
This change eliminates the redundant and ugly pattern
`Checker->getCheckerName()` and helps to visually distinguish this method from
`BugType::getCheckerName`.
In the constructor of `BugType` the call of this method was completely removed
(instead of just changing the name) because that call was dead code (when the
data member `Checker` is non-null, the string stored in `CheckerName` is
irrelevant) and was often querying the name of the checker before it was
properly initialized.
Moreover, in `ReturnValueChecker.cpp` the static helper function `getName`
(which gets a function name from a `CallEvent`) was renamed to the more
specific `getFunctionName` to avoid the name collision.
This change is yet another cleanup commit before my planned changes that would
add support for multi-part checkers to this method.
From 9ff0645b63b390686c5aa2826f4c1ae89c08dfa9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?=
Date: Wed, 12 Mar 2025 13:02:19 +0100
Subject: [PATCH] [NFC][analyzer] Rename `CheckerBase::getCheckerName` to
`getName`
The method name `getCheckerName` would imply "get the name of the
checker associated with `this`", so it's suitable for e.g.
`BugType::getCheckerName` -- but the proper name for a method that "gets
the name of `this`" should be simply called `getName`.
This change eliminates the redundant and ugly pattern
`Checker->getCheckerName()` and helps to visually distinguish this
method from `BugType::getCheckerName`.
In the constructor of `BugType` the call of this method was completely
removed (instead of just changing the name) because that call was dead
code (when the data member `Checker` is non-null, the string stored in
`CheckerName` is irrelevant) and was often querying the name of the
checker before it was properly initialized.
Moreover, in `ReturnValueChecker.cpp` the static helper function
`getName` (which gets a function name from a `CallEvent`) was renamed to
the more specific `getFunctionName` to avoid the name collision.
This change is yet another cleanup commit before my planned changes that
would add support for multi-part checkers to this method.
---
.../clang/StaticAnalyzer/Core/BugReporter/BugType.h | 8
clang/include/clang/StaticAnalyzer/Core/Checker.h | 2 +-
.../StaticAnalyzer/Checkers/ExprInspectionChecker.cpp | 5 ++---
.../StaticAnalyzer/Checkers/ReturnValueChecker.cpp| 6 +++---
clang/lib/StaticAnalyzer/Core/BugReporter.cpp | 4 ++--
clang/lib/StaticAnalyzer/Core/Checker.cpp | 8
clang/lib/StaticAnalyzer/Core/CheckerManager.cpp | 11 +--
7 files changed, 21 insertions(+), 23 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
index e50afd6d0da7e..97cf7eda0ae2f 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
@@ -41,19 +41,19 @@ class BugType {
Checker(nullptr), SuppressOnSink(SuppressOnSink) {}
BugType(const CheckerBase *Checker, StringRef Desc,
StringRef Cat = categories::LogicError, bool SuppressOnSink = false)
- : CheckerName(Checker->getCheckerName()), Description(Desc),
-Category(Cat), Checker(Checker), SuppressOnSink(SuppressOnSink) {}
+ : CheckerName(), Description(Desc), Category(Cat), Checker(Checker),
+SuppressOnSink(SuppressOnSink) {}
virtual ~BugType() = default;
StringRef getDescription() const { return Description; }
StringRef getCategory() const { return Category; }
StringRef getCheckerName() const {
-// FIXME: This is a workaround to ensure that the correct checerk name is
+// FIXME: This is a workaround to ensure that the correct checker name is
// used. The checker names are set after the constructors are run.
// In case the BugType object is initialized in the checker's ctor
// the CheckerName field will be empty. To circumvent this problem we use
// CheckerBase whenever it is possible.
-StringRef Ret = Checker ? Checker->getCheckerName() : CheckerName;
+StringRef Ret = Checker ? Checker->getName() : CheckerName;
assert(!Ret.empty() && "Checker name is not set properly.");
return Ret;
}
diff --git a/clang/include/clang/StaticAnalyzer/Core/Checker.h
b/clang/include/clang/StaticAnalyzer/Core/Checker.h
index 2ec54a837c42c..4d9b33cc559b8 100644
--- a/clang/include/clang/StaticAnalyzer/Core/Checker.h
+++ b/clang/include/clang/StaticAnalyzer/Core/Checker.h
@@ -490,7 +490,7 @@ class CheckerBase : public ProgramPointTag {
