Skip to content

[NFC][analyzer] Rename CheckerBase::getCheckerName to getName #130953

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 12, 2025
Merged

[NFC][analyzer] Rename CheckerBase::getCheckerName to getName #130953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Checker ever null? Do we still need the CheckerName field?

Copy link
Contributor Author

@NagyDonat NagyDonat Mar 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 currently cannot query the name of the relevant checker part from a CheckerBase * (because each multipart checker implements its own custom array of checker names), so we need to initialize the BugType with a CheckerNameRef (and set Checker to null), and this requires lazy initialization (the ugly mutable std::unique_ptr<BugType>) 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 BugTypes 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 <CheckerBase*, unsigned> pair (multipart checkers), so we can get rid of the BugType constructor that takes a raw CheckerName (and needs the awkward lazy initialization).

StringRef Ret = Checker ? Checker->getName() : CheckerName;
assert(!Ret.empty() && "Checker name is not set properly.");
return Ret;
}
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/StaticAnalyzer/Core/Checker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 2 additions & 3 deletions clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
6 changes: 3 additions & 3 deletions clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class ReturnValueChecker : public Checker<check::PostCall> {
};
} // namespace

static std::string getName(const CallEvent &Call) {
static std::string getFunctionName(const CallEvent &Call) {
std::string Name;
if (const auto *MD = dyn_cast<CXXMethodDecl>(Call.getDecl()))
if (const CXXRecordDecl *RD = MD->getParent())
Expand All @@ -84,7 +84,7 @@ void ReturnValueChecker::checkPostCall(const CallEvent &Call,
if (ProgramStateRef StTrue = State->assume(*ReturnV, true)) {
// The return value can be true, so transition to a state where it's true.
std::string Msg =
formatv("'{0}' returns true (by convention)", getName(Call));
formatv("'{0}' returns true (by convention)", getFunctionName(Call));
C.addTransition(StTrue, C.getNoteTag(Msg, /*IsPrunable=*/true));
return;
}
Expand All @@ -94,7 +94,7 @@ void ReturnValueChecker::checkPostCall(const CallEvent &Call,
// Note that this checker is 'hidden' so it cannot produce a bug report.
std::string Msg = formatv("'{0}' returned false, breaking the convention "
"that it always returns true",
getName(Call));
getFunctionName(Call));
C.addTransition(State, C.getNoteTag(Msg, /*IsPrunable=*/true));
}

Expand Down
4 changes: 2 additions & 2 deletions clang/lib/StaticAnalyzer/Core/BugReporter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3418,8 +3418,8 @@ void BugReporter::EmitBasicReport(const Decl *DeclWithIssue,
PathDiagnosticLocation Loc,
ArrayRef<SourceRange> Ranges,
ArrayRef<FixItHint> Fixits) {
EmitBasicReport(DeclWithIssue, Checker->getCheckerName(), Name, Category, Str,
Loc, Ranges, Fixits);
EmitBasicReport(DeclWithIssue, Checker->getName(), Name, Category, Str, Loc,
Ranges, Fixits);
}

void BugReporter::EmitBasicReport(const Decl *DeclWithIssue,
Expand Down
8 changes: 4 additions & 4 deletions clang/lib/StaticAnalyzer/Core/Checker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,20 @@ using namespace ento;

int ImplicitNullDerefEvent::Tag;

StringRef CheckerBase::getTagDescription() const { return getCheckerName(); }
StringRef CheckerBase::getTagDescription() const { return getName(); }

CheckerNameRef CheckerBase::getCheckerName() const { return Name; }
CheckerNameRef CheckerBase::getName() const { return Name; }

CheckerProgramPointTag::CheckerProgramPointTag(StringRef CheckerName,
StringRef Msg)
: SimpleProgramPointTag(CheckerName, Msg) {}

CheckerProgramPointTag::CheckerProgramPointTag(const CheckerBase *Checker,
StringRef Msg)
: SimpleProgramPointTag(Checker->getCheckerName(), Msg) {}
: SimpleProgramPointTag(Checker->getName(), Msg) {}

raw_ostream& clang::ento::operator<<(raw_ostream &Out,
const CheckerBase &Checker) {
Out << Checker.getCheckerName();
Out << Checker.getName();
return Out;
}
11 changes: 5 additions & 6 deletions clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ std::string checkerScopeName(StringRef Name, const CheckerBase *Checker) {
if (!llvm::timeTraceProfilerEnabled())
return "";
StringRef CheckerName =
Checker ? static_cast<StringRef>(Checker->getCheckerName()) : "<unknown>";
Checker ? static_cast<StringRef>(Checker->getName()) : "<unknown>";
return (Name + ":" + CheckerName).str();
}

Expand Down Expand Up @@ -722,12 +722,12 @@ void CheckerManager::runCheckersForEvalCall(ExplodedNodeSet &Dst,
"while the {2} checker also tried to evaluate the same call. At "
"most one checker supposed to evaluate a call.",
toString(Call), evaluatorChecker,
EvalCallChecker.Checker->getCheckerName());
EvalCallChecker.Checker->getName());
llvm_unreachable(AssertionMessage.c_str());
}
#endif
if (evaluated) {
evaluatorChecker = EvalCallChecker.Checker->getCheckerName();
evaluatorChecker = EvalCallChecker.Checker->getName();
Dst.insert(checkDst);
#ifdef NDEBUG
break; // on release don't check that no other checker also evals.
Expand Down Expand Up @@ -798,9 +798,8 @@ void CheckerManager::runCheckersForPrintStateJson(raw_ostream &Out,
if (TempBuf.empty())
continue;

Indent(Out, Space, IsDot)
<< "{ \"checker\": \"" << CT.second->getCheckerName()
<< "\", \"messages\": [" << NL;
Indent(Out, Space, IsDot) << "{ \"checker\": \"" << CT.second->getName()
<< "\", \"messages\": [" << NL;
Indent(Out, InnerSpace, IsDot)
<< '\"' << TempBuf.str().trim() << '\"' << NL;
Indent(Out, Space, IsDot) << "]}";
Expand Down
Loading