Skip to content

[NFC][analyzer] Multipart checker refactor 2: NullabilityChecker #132250

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

Closed
NagyDonat wants to merge 4 commits into from
Closed

[NFC][analyzer] Multipart checker refactor 2: NullabilityChecker #132250

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d4878a6
[NFC][analyzer] Multipart checker refactor 2: NullabilityChecker
NagyDonat Mar 20, 2025
2708bed
Remove hidden dummy checker part NullabilityBase
NagyDonat Mar 20, 2025
3ca23ce
Use `BugType` parameters for `reportBug{,IfInvariantHolds}`
NagyDonat Mar 20, 2025
d490c74
Special case NoDiagnoseCallsToSystemHeaders as group-level option
NagyDonat Mar 20, 2025
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
13 changes: 11 additions & 2 deletions clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1403,6 +1403,16 @@ void NullabilityChecker::printState(raw_ostream &Out, ProgramStateRef State,
}
}

// The checker group "nullability" consists of the checkers that are
// implemented as the parts of the checker class `NullabilityChecker`. These
// checkers share a checker option "nullability:NoDiagnoseCallsToSystemHeaders"
// which semantically belongs to the whole group and not just one checker from
// it. As this is a unique situation (I don't know about any other similar
// group-level option) there is no mechanism to inject this group name from
// e.g. Checkers.td, so I'm just hardcoding it here. (These are old stable
// checkers, I don't think that their name will change.)
Comment on lines +1406 to +1413
Copy link
Contributor Author

@NagyDonat NagyDonat Mar 20, 2025

Choose a reason for hiding this comment

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

Before this PR, the registration functions of the (real) checker parts actually tested the presence of the checker option nullability.NullabilityBase:NoDiagnoseCallsToSystemHeaders, where nullability.NullabilityBase was the hidden dummy checker part that was added as a prerequisite of all the "real" checker parts. (Note that -- ironically -- registerNullabilityBase() did not check for the presence of this option which was nominally attached to it.)

This was working because in the checker option handling code specifying an option on the group level (e.g. saying nullability:NoDiagnoseCallsToSystemHeaders=true) is roughly equivalent to passing it to every checker within the group.

In commit d490c74 I'm turning this option into a "real" group-level option by passing the group name to getCheckerBooleanOption.

By the way the name of this option is really ugly -- it should be DiagnoseCallsToSystemHeaders with negated meaning -- but that's a problem for another commit.

#define CHECKER_GROUP_NAME "nullability"
Copy link
Contributor

@steakhal steakhal Mar 21, 2025

Choose a reason for hiding this comment

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

You could use a static constexpr StringLiteral instead.

Copy link
Contributor Author

@NagyDonat NagyDonat Mar 21, 2025

Choose a reason for hiding this comment

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

Yes, that's probably a better choice.


#define REGISTER_CHECKER(Part, TrackingRequired) \
void ento::register##Part##Checker(CheckerManager &Mgr) { \
auto *Checker = Mgr.registerChecker<NullabilityChecker, \
Expand All @@ -1411,8 +1421,7 @@ void NullabilityChecker::printState(raw_ostream &Out, ProgramStateRef State,
Checker->NoDiagnoseCallsToSystemHeaders = \
Checker->NoDiagnoseCallsToSystemHeaders || \
Mgr.getAnalyzerOptions().getCheckerBooleanOption( \
Mgr.getCurrentCheckerName(), "NoDiagnoseCallsToSystemHeaders", \
true); \
CHECKER_GROUP_NAME, "NoDiagnoseCallsToSystemHeaders", true); \
} \
\
bool ento::shouldRegister##Part##Checker(const CheckerManager &) { \
Expand Down
Loading