[clang] [NFC][analyzer] Introduce specialized variants of makeNode (PR #194459)
Donát Nagy via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 27 13:47:57 PDT 2026
https://github.com/NagyDonat created https://github.com/llvm/llvm-project/pull/194459
This commit introduces new methods `makePostStmtNode` and `makeNodeWithBinding` of `CoreEngine`, which will be used instead of the 5-parameter overloads of `NodeBuilder::generateNode` and `NodeBuilder::generateSink` (which were originally methods of the class `StmtNodeBuilder` that was deleted in commit fb46677a858697afa116c4252e84050a07bc6a70).
This commit applies the newly introduced methods in a few places (as examples), but there are 80+ call sites that use the 5-parameter `NodeBuilder::generateNode` or `generateSink`, so this transition will be completed in multiple follow-up commits.
The parametrization of these methods was designed to provide the features that will be used frequently. E.g. `makeNodeWithBinding` was introduced because there are 30+ call sites that use `BindExpr` just before the `generateNode` call -- but there is no support for specifying a `tag` because only a few call sites would use it.
----
For the reviewer's convenience, the [5-parameter `generateNode`](https://github.com/llvm/llvm-project/blob/5e625826d6d394e65a767bf94d21739b18dade30/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h#L285) is defined as:
```c++
ExplodedNode *generateNode(const Stmt *S,
ExplodedNode *Pred,
ProgramStateRef St,
const ProgramPointTag *tag = nullptr,
ProgramPoint::Kind K = ProgramPoint::PostStmtKind){
const ProgramPoint &L = ProgramPoint::getProgramPoint(S, K,
Pred->getLocationContext(), tag);
return generateNode(L, St, Pred);
}
```
>From 65e22c64e6dfe4e8c1650b6772bf241ce8ad3c7e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Fri, 24 Apr 2026 13:36:24 +0200
Subject: [PATCH 1/6] [NFC][analyzer] Introduce new variants of makeNode
These will be used to replace the use of the 5-parameter overloads of
`NodeBuilder::generateNode` and `NodeBuilder::generateSink` (which were
originally methods of the class `StmtNodeBuilder` that was deleted in
commit fb46677a858697afa116c4252e84050a07bc6a70).
The parametrization of these methods was designed to provide features
that are helpful in 5+ call sites. E.g. `makeNodeWithBinding` was
introduced because 30+ call sites (of 80) use `BindExpr` just before the
`generateNode` call -- but there is no support for specifying a `tag`
because only a few call sites would use it.
---
.../Core/PathSensitive/CoreEngine.h | 23 +++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
index 57b0947093b78..1068cb2353f90 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
@@ -170,6 +170,29 @@ class CoreEngine {
ExplodedNode *makeNode(const ProgramPoint &Loc, ProgramStateRef State,
ExplodedNode *Pred, bool MarkAsSink = false) const;
+ ExplodedNode *makePostStmtNode(const Stmt *S, ProgramStateRef State,
+ ExplodedNode *Pred,
+ bool MarkAsSink = false) const {
+ PostStmt Loc(S, Pred->getLocationContext(), /*tag=*/nullptr);
+ return makeNode(Loc, State, Pred, MarkAsSink);
+ }
+
+ ExplodedNode *
+ makeNodeWithBinding(ExplodedNode *Pred, const Expr *E, SVal V,
+ ProgramStateRef State,
+ ProgramPoint::Kind K = ProgramPoint::PostStmtKind) const {
+ const LocationContext *LC = Pred->getLocationContext();
+ State = State->BindExpr(E, LC, V);
+ const auto &L = ProgramPoint::getProgramPoint(E, K, LC, /*tag=*/nullptr);
+ return makeNode(L, State, Pred);
+ }
+
+ ExplodedNode *
+ makeNodeWithBinding(ExplodedNode *Pred, const Expr *E, SVal V,
+ ProgramPoint::Kind K = ProgramPoint::PostStmtKind) const {
+ return makeNodeWithBinding(Pred, E, V, Pred->getState(), K);
+ }
+
/// Enqueue the given set of nodes onto the work list.
void enqueue(ExplodedNodeSet &Set);
>From 9a497975dc2bfcb9b2c4a81839d5c19b1cf16e75 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Fri, 24 Apr 2026 15:17:13 +0200
Subject: [PATCH 2/6] [NFC] Use makePostStmtNode in ProcessTemporaryDtor
The old code was overcomplicated with the `ExplodedNodeSet` that
held at most one element.
Note that the "is null or sink" check appears because it was present in
`ExplodedNodeSet::insert` which is called in `generateNode`.
---
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index b69b45acb7989..05fe416fd17dd 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1590,16 +1590,16 @@ void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D,
return;
}
- ExplodedNodeSet CleanDtorState;
- NodeBuilder Builder(Pred, CleanDtorState, *currBldrCtx);
- Builder.generateNode(D.getBindTemporaryExpr(), Pred, State);
+ ExplodedNode *CleanPred =
+ Engine.makePostStmtNode(D.getBindTemporaryExpr(), State, Pred);
+ if (!CleanPred || CleanPred->isSink()) {
+ // FIXME: We can get a null node here due to temporaries being
+ // bound to default parameters.
+ // Sink check is just PosteriorlyOverconstrained paranoia.
+ CleanPred = Pred;
+ }
QualType T = D.getBindTemporaryExpr()->getSubExpr()->getType();
- // FIXME: Currently CleanDtorState can be empty here due to temporaries being
- // bound to default parameters.
- assert(CleanDtorState.size() <= 1);
- ExplodedNode *CleanPred =
- CleanDtorState.empty() ? Pred : *CleanDtorState.begin();
EvalCallOptions CallOpts;
CallOpts.IsTemporaryCtorOrDtor = true;
>From 1c18a3e3ca9739dd098029d2052542d75fd7dd00 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Fri, 24 Apr 2026 15:28:55 +0200
Subject: [PATCH 3/6] [NFC][side] Simplify ProcessTemporaryDtor
Opportunistic cleanup in `ExprEngine::processTemporaryDtor`, which is
unrelated to the core goal of this commit series.
This replaces some getter calls with references to local variables
(which were already conveniently present as shorter aliases of those
cumbersome calls), and eliminates the _other_ `NodeBuilder` from this
method.
This node builder removal is an NFC change because `Dst` is always empty
at the beginning, so the "remove from the `Frontier`" step does nothing.
---
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 21 ++++++++------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 05fe416fd17dd..ac591f2d1ce1e 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1568,13 +1568,11 @@ void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D,
const LocationContext *LC = Pred->getLocationContext();
const MemRegion *MR = nullptr;
- if (std::optional<SVal> V = getObjectUnderConstruction(
- State, D.getBindTemporaryExpr(), Pred->getLocationContext())) {
+ if (std::optional<SVal> V = getObjectUnderConstruction(State, BTE, LC)) {
// FIXME: Currently we insert temporary destructors for default parameters,
// but we don't insert the constructors, so the entry in
// ObjectsUnderConstruction may be missing.
- State = finishObjectConstruction(State, D.getBindTemporaryExpr(),
- Pred->getLocationContext());
+ State = finishObjectConstruction(State, BTE, LC);
MR = V->getAsRegion();
}
@@ -1582,16 +1580,13 @@ void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D,
// destructor was elided, we need to skip the destructor as well.
if (isDestructorElided(State, BTE, LC)) {
State = cleanupElidedDestructor(State, BTE, LC);
- NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
- PostImplicitCall PP(D.getDestructorDecl(getContext()),
- D.getBindTemporaryExpr()->getBeginLoc(),
- Pred->getLocationContext(), getCFGElementRef());
- Bldr.generateNode(PP, State, Pred);
+ PostImplicitCall PP(D.getDestructorDecl(getContext()), BTE->getBeginLoc(),
+ LC, getCFGElementRef());
+ Dst.insert(Engine.makeNode(PP, State, Pred));
return;
}
- ExplodedNode *CleanPred =
- Engine.makePostStmtNode(D.getBindTemporaryExpr(), State, Pred);
+ ExplodedNode *CleanPred = Engine.makePostStmtNode(BTE, State, Pred);
if (!CleanPred || CleanPred->isSink()) {
// FIXME: We can get a null node here due to temporaries being
// bound to default parameters.
@@ -1599,7 +1594,7 @@ void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D,
CleanPred = Pred;
}
- QualType T = D.getBindTemporaryExpr()->getSubExpr()->getType();
+ QualType T = BTE->getSubExpr()->getType();
EvalCallOptions CallOpts;
CallOpts.IsTemporaryCtorOrDtor = true;
@@ -1632,7 +1627,7 @@ void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D,
// but for now we don't have the respective construction contexts,
// so MR would always be null in this case. Do nothing for now.
}
- VisitCXXDestructor(T, MR, D.getBindTemporaryExpr(),
+ VisitCXXDestructor(T, MR, BTE,
/*IsBase=*/false, CleanPred, Dst, CallOpts);
}
>From 975227dfad4b1f8849de6b7f152408245a56b221 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Fri, 24 Apr 2026 16:11:29 +0200
Subject: [PATCH 4/6] [NFC] Remove builder in VisitCommonDeclRefExpr
This is a good example of the code pattern that motivated the
introduction of `makeNodeWithBinding`:
- The same expression is passed to both `BindExpr` and the creation of
the `ProgramPoint`.
- The `LocationContext` passed to `BindExpr` is always the
`LocationContext` of the `Pred` node (there is no stack frame change
around these parts).
- The state that is updated by `BindExpr` is usually the state of `Pred`
(this holds here: the `state` is inspected many times, but the only
state changes are these `BindExpr` calls that are incorporated into
`makeNodeWithBinding`).
- Elsewhere the `ProgramPoint` kind is almost always `PostStmt`, but
here we need to create several `PostLValue` program points, therefore
`makeNodeWithBinding` takes the kind as an optional parameter.
Note that `Dst` was empty before entering this function. There were
two branches where the `Dst.insert(Pred)` call in the constructor of
`NodeBuilder` wasn't "cancelled" by a `generateNode` call -- on those
branches this commit needs to add the `Dst.insert(Pred)` call.
In the future it would be nice to eventually replace the customary
out-parameter `ExplodedNodeSet &Dst` with directly returning the created
node or nodes.
---
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 23 ++++++++++----------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index ac591f2d1ce1e..76ccb9ef4a103 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3189,8 +3189,6 @@ void ExprEngine::processSwitch(const SwitchStmt *Switch, ExplodedNode *Pred,
void ExprEngine::VisitCommonDeclRefExpr(const Expr *Ex, const NamedDecl *D,
ExplodedNode *Pred,
ExplodedNodeSet &Dst) {
- NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
-
ProgramStateRef state = Pred->getState();
const LocationContext *LCtx = Pred->getLocationContext();
@@ -3242,25 +3240,26 @@ void ExprEngine::VisitCommonDeclRefExpr(const Expr *Ex, const NamedDecl *D,
V = UnknownVal();
}
- Bldr.generateNode(Ex, Pred, state->BindExpr(Ex, LCtx, V), nullptr,
- ProgramPoint::PostLValueKind);
+ Dst.insert(
+ Engine.makeNodeWithBinding(Pred, Ex, V, ProgramPoint::PostLValueKind));
return;
}
if (const auto *ED = dyn_cast<EnumConstantDecl>(D)) {
assert(!Ex->isGLValue());
SVal V = svalBuilder.makeIntVal(ED->getInitVal());
- Bldr.generateNode(Ex, Pred, state->BindExpr(Ex, LCtx, V));
+ Dst.insert(Engine.makeNodeWithBinding(Pred, Ex, V));
return;
}
if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
SVal V = svalBuilder.getFunctionPointer(FD);
- Bldr.generateNode(Ex, Pred, state->BindExpr(Ex, LCtx, V), nullptr,
- ProgramPoint::PostLValueKind);
+ Dst.insert(
+ Engine.makeNodeWithBinding(Pred, Ex, V, ProgramPoint::PostLValueKind));
return;
}
if (isa<FieldDecl, IndirectFieldDecl>(D)) {
// Delegate all work related to pointer to members to the surrounding
// operator&.
+ Dst.insert(Pred);
return;
}
if (const auto *BD = dyn_cast<BindingDecl>(D)) {
@@ -3276,8 +3275,8 @@ void ExprEngine::VisitCommonDeclRefExpr(const Expr *Ex, const NamedDecl *D,
V = UnknownVal();
}
- Bldr.generateNode(Ex, Pred, state->BindExpr(Ex, LCtx, V), nullptr,
- ProgramPoint::PostLValueKind);
+ Dst.insert(Engine.makeNodeWithBinding(Pred, Ex, V,
+ ProgramPoint::PostLValueKind));
return;
}
@@ -3331,15 +3330,15 @@ void ExprEngine::VisitCommonDeclRefExpr(const Expr *Ex, const NamedDecl *D,
V = UnknownVal();
}
- Bldr.generateNode(Ex, Pred, state->BindExpr(Ex, LCtx, V), nullptr,
- ProgramPoint::PostLValueKind);
-
+ Dst.insert(
+ Engine.makeNodeWithBinding(Pred, Ex, V, ProgramPoint::PostLValueKind));
return;
}
if (const auto *TPO = dyn_cast<TemplateParamObjectDecl>(D)) {
// FIXME: We should meaningfully implement this.
(void)TPO;
+ Dst.insert(Pred);
return;
}
>From a6416e95087f064dc6dc6151e35bcc4ecd9ef759 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Fri, 24 Apr 2026 16:24:13 +0200
Subject: [PATCH 5/6] [NFC][side] Remove redundant local in
VisitCommonDeclRefExpr
At the beginning of this function we have
```
const LocationContext *LCtx = Pred->getLocationContext();
```
(and `Pred` stays the same) so this alias was redundant.
---
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 76ccb9ef4a103..507ba49c0831d 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3221,12 +3221,11 @@ void ExprEngine::VisitCommonDeclRefExpr(const Expr *Ex, const NamedDecl *D,
// C permits "extern void v", and if you cast the address to a valid type,
// you can even do things with it. We simply pretend
assert(Ex->isGLValue() || VD->getType()->isVoidType());
- const LocationContext *LocCtxt = Pred->getLocationContext();
std::optional<std::pair<SVal, QualType>> VInfo =
resolveAsLambdaCapturedVar(VD);
if (!VInfo)
- VInfo = std::make_pair(state->getLValue(VD, LocCtxt), VD->getType());
+ VInfo = std::make_pair(state->getLValue(VD, LCtx), VD->getType());
SVal V = VInfo->first;
bool IsReference = VInfo->second->isReferenceType();
>From f9a8a5d26d98a95a12b4b848495a639a6484c2f4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Fri, 24 Apr 2026 16:45:43 +0200
Subject: [PATCH 6/6] [NFC] Remove builder in VisitArraySubscriptExpr
Standard situation where `generateNode` is called in a loop and replaces
each node (produced by the checkers) with a successor.
---
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 507ba49c0831d..9796119b05cc3 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3453,7 +3453,6 @@ void ExprEngine::VisitArraySubscriptExpr(const ArraySubscriptExpr *A,
getCheckerManager().runCheckersForPreStmt(CheckerPreStmt, Pred, A, *this);
ExplodedNodeSet EvalSet;
- NodeBuilder Bldr(CheckerPreStmt, EvalSet, *currBldrCtx);
bool IsVectorType = A->getBase()->getType()->isVectorType();
@@ -3479,11 +3478,11 @@ void ExprEngine::VisitArraySubscriptExpr(const ArraySubscriptExpr *A,
SVal V = state->getLValue(T,
state->getSVal(Idx, LCtx),
state->getSVal(Base, LCtx));
- Bldr.generateNode(A, Node, state->BindExpr(A, LCtx, V), nullptr,
- ProgramPoint::PostLValueKind);
+ EvalSet.insert(
+ Engine.makeNodeWithBinding(Node, A, V, ProgramPoint::PostLValueKind));
} else if (IsVectorType) {
// FIXME: non-glvalue vector reads are not modelled.
- Bldr.generateNode(A, Node, state, nullptr);
+ EvalSet.insert(Engine.makePostStmtNode(A, state, Node));
} else {
llvm_unreachable("Array subscript should be an lValue when not \
a vector and not a forbidden lvalue type");
More information about the cfe-commits
mailing list