[clang] [NFC][analyzer] Eliminate some simple node builders (PR #204187)
Donát Nagy via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 17 09:07:53 PDT 2026
https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/204187
>From 651de90d902cac7227a9cecaa3c845c72cca535c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Tue, 16 Jun 2026 13:22:16 +0200
Subject: [PATCH 1/8] Eliminate NodeBuilder from ProcessLifetimeEnd
The NodeBuilder in this recently published method was used in a simple
and straightforward way, so I was able to replace it with a `makeNode`
call. Note that the new code relies on passing a single `ExplodedNode *`
to an `ExplodedNodeSet` parameter of `runCheckersForLifetimeEnd` -- this
works because there is a converting constructor which is frequently used
to streamline situations like this.
---
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 32da5e097c76e..0d2fa88ec0ec1 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1148,10 +1148,8 @@ void ExprEngine::ProcessLifetimeEnd(const Stmt *S, const VarDecl *D,
PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
S->getBeginLoc(),
"Error evaluating end of a lifetime");
- ExplodedNodeSet Src;
- NodeBuilder Bldr(Pred, Src, *currBldrCtx);
LifetimeEnd PP(S, D, Pred->getStackFrame());
- Bldr.generateNode(PP, Pred->getState(), Pred);
+ ExplodedNode *Src = Engine.makeNode(PP, Pred->getState(), Pred);
ExplodedNodeSet Dst;
getCheckerManager().runCheckersForLifetimeEnd(Dst, Src, D, *this);
>From 13005af677d06ca221aed88e4e2711d1a13451cf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Tue, 16 Jun 2026 13:27:57 +0200
Subject: [PATCH 2/8] Eliminate NodeBuilder from VisitCXXBindTemporaryExpr
The constructor of this `NodeBuilder` was inserting all elements of
`PreVisit` to `Dst` (the `Frontier` of the `NodeBuilder`), but then the
`for` loop removed each element when it unconditionally called
`generateNode` from each `Node`. (Note that `Dst` was empty previously,
this method is only called from `ExprEngine::Visit`.)
The new code elides this back-and-forth logic by only inserting the
freshly created nodes into `Dst`.
---
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 0d2fa88ec0ec1..a1c600cfc1980 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1637,7 +1637,6 @@ void ExprEngine::VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *BTE,
Dst = PreVisit;
return;
}
- NodeBuilder Builder(PreVisit, Dst, *currBldrCtx);
for (ExplodedNode *Node : PreVisit) {
ProgramStateRef State = Node->getState();
const StackFrame *SF = Node->getStackFrame();
@@ -1648,7 +1647,7 @@ void ExprEngine::VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *BTE,
// temporary destructor nodes.
State = addObjectUnderConstruction(State, BTE, SF, UnknownVal());
}
- Builder.generateNode(BTE, Node, State);
+ Dst.insert(Engine.makePostStmtNode(BTE, State, Node));
}
}
>From e29599ee7bd999e47049ef9edcb2e0c366d79d39 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Tue, 16 Jun 2026 13:50:12 +0200
Subject: [PATCH 3/8] Eliminate NodeBuilder in VisitMemberExpr
This was a slightly more complex case: the constructor of the
`NodeBuilder` inserts `CheckedSet` into `EvalSet` (which was empty
previously), then the loop iterating over `CheckedSet` has a complex
body, but still each iteration reaches either a `takeNodes` or a
`generateNode` call that removes the current node `I` from `EvalSet`.
Therefore I was able to "cancel out" the insertion side effect of the
constructor and these node removal operations. The new code just creates
the new nodes with the higher-level `makeNodeWithBinding` (that
simplifies the code) and directly inserts them into `EvalSet`.
I could probably already remove `ExplodedNodeSet &Tmp` in this commit,
but right now I'm just adding a FIXME because understanding the
potential effect of this change will be simpler when there are no
`NodeBuilder`s in `evalLoad`.
---
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index a1c600cfc1980..7a8ccfc02889d 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3349,7 +3349,6 @@ void ExprEngine::VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred,
for (const auto I : CheckedSet)
VisitCommonDeclRefExpr(M, Member, I, EvalSet);
} else {
- NodeBuilder Bldr(CheckedSet, EvalSet, *currBldrCtx);
ExplodedNodeSet Tmp;
for (const auto I : CheckedSet) {
@@ -3363,9 +3362,8 @@ void ExprEngine::VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred,
state = createTemporaryRegionIfNeeded(state, SF, BaseExpr);
SVal MDVal = svalBuilder.getFunctionPointer(MD);
- state = state->BindExpr(M, SF, MDVal);
- Bldr.generateNode(M, I, state);
+ EvalSet.insert(Engine.makeNodeWithBinding(I, M, MDVal, state));
continue;
}
@@ -3409,12 +3407,13 @@ void ExprEngine::VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred,
L = UnknownVal();
}
- Bldr.generateNode(M, I, state->BindExpr(M, SF, L), nullptr,
- ProgramPoint::PostLValueKind);
+ EvalSet.insert(Engine.makeNodeWithBinding(
+ I, M, L, state, ProgramPoint::PostLValueKind));
} else {
- Bldr.takeNodes(I);
+ // FIXME: When evalLoad no longer uses NodeBuilders, eliminate Tmp and
+ // pass EvalSet as the first argument of evalLoad.
evalLoad(Tmp, M, M, I, state, L);
- Bldr.addNodes(Tmp);
+ EvalSet.insert(Tmp);
}
}
}
>From b18b0144e34b88f1ac392ca893dbb86d4cde1f88 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Tue, 16 Jun 2026 14:04:55 +0200
Subject: [PATCH 4/8] Eliminate NodeBuilder from VisitAtomicExpr
The constructor of this `NodeBuilder` was inserting all elements of
`AfterPreSet` to the previously empty local `AfterInvalidateSet` (the
`Frontier` of the `NodeBuilder`), but then the `for` loop removed each
element when it unconditionally called `generateNode` from each `I`.
The new code elides this back-and-forth logic by only inserting the
freshly created nodes into `AfterInvalidateSet`. (This node creation
matches the standard pattern of `makeNodeWithBinding`, so I was able to
simplify the code.)
---
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 7a8ccfc02889d..7c9490e6b8646 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3430,7 +3430,6 @@ void ExprEngine::VisitAtomicExpr(const AtomicExpr *AE, ExplodedNode *Pred,
// FIXME: Ideally we should model the behavior of the atomics precisely here.
ExplodedNodeSet AfterInvalidateSet;
- NodeBuilder Bldr(AfterPreSet, AfterInvalidateSet, *currBldrCtx);
for (const auto I : AfterPreSet) {
ProgramStateRef State = I->getState();
@@ -3448,10 +3447,8 @@ void ExprEngine::VisitAtomicExpr(const AtomicExpr *AE, ExplodedNode *Pred,
/*CausedByPointerEscape*/ true,
/*Symbols=*/nullptr);
- SVal ResultVal = UnknownVal();
- State = State->BindExpr(AE, SF, ResultVal);
- Bldr.generateNode(AE, I, State, nullptr,
- ProgramPoint::PostStmtKind);
+ AfterInvalidateSet.insert(
+ Engine.makeNodeWithBinding(I, AE, UnknownVal(), State));
}
getCheckerManager().runCheckersForPostStmt(Dst, AfterInvalidateSet, AE, *this);
>From a0d26d9ff05ca5aeb354b50208a07e2c7ef736f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Tue, 16 Jun 2026 14:13:22 +0200
Subject: [PATCH 5/8] [side] Use range-based loop over children of AtomicExpr
We do have `AtomicExpr::children` that returns a range -- unfortunately
this range contains the subexpressions as `Stmt*` objects so I needed to
use a `cast` to get a proper `Expr*`.
---
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 7c9490e6b8646..0ecef1c77f4cb 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3436,9 +3436,8 @@ void ExprEngine::VisitAtomicExpr(const AtomicExpr *AE, ExplodedNode *Pred,
const StackFrame *SF = I->getStackFrame();
SmallVector<SVal, 8> ValuesToInvalidate;
- for (unsigned SI = 0, Count = AE->getNumSubExprs(); SI != Count; SI++) {
- const Expr *SubExpr = AE->getSubExprs()[SI];
- SVal SubExprVal = State->getSVal(SubExpr, SF);
+ for (const Stmt *SubExpr : AE->children()) {
+ SVal SubExprVal = State->getSVal(cast<Expr>(SubExpr), SF);
ValuesToInvalidate.push_back(SubExprVal);
}
>From 186878b4db278c1bdd885d219fdfceee25caedca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Wed, 17 Jun 2026 17:53:06 +0200
Subject: [PATCH 6/8] Eliminate NodeBuilders for ASM statements
These were extremely trivial.
---
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 0ecef1c77f4cb..baa0576db73e1 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3760,7 +3760,6 @@ bool ExprEngine::didEagerlyAssumeBifurcateAt(ProgramStateRef State,
void ExprEngine::VisitGCCAsmStmt(const GCCAsmStmt *A, ExplodedNode *Pred,
ExplodedNodeSet &Dst) {
- NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
// We have processed both the inputs and the outputs. All of the outputs
// should evaluate to Locs. Nuke all of their values.
@@ -3792,13 +3791,12 @@ void ExprEngine::VisitGCCAsmStmt(const GCCAsmStmt *A, ExplodedNode *Pred,
/*CausedByPointerEscape=*/true);
}
- Bldr.generateNode(A, Pred, state);
+ Dst.insert(Engine.makePostStmtNode(A, state, Pred));
}
void ExprEngine::VisitMSAsmStmt(const MSAsmStmt *A, ExplodedNode *Pred,
ExplodedNodeSet &Dst) {
- NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
- Bldr.generateNode(A, Pred, Pred->getState());
+ Dst.insert(Engine.makePostStmtNode(A, Pred->getState(), Pred));
}
//===----------------------------------------------------------------------===//
>From ee2a592b2e84b94cc0cf344704611a4f57d36dae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Wed, 17 Jun 2026 18:03:00 +0200
Subject: [PATCH 7/8] Simplify logic in ConstructInitList
Don't duplicate binding the value and generating the node.
---
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index baa0576db73e1..151b957b21f06 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3969,18 +3969,17 @@ void ExprEngine::ConstructInitList(const Expr *E, ArrayRef<Expr *> Args,
bool IsCompound = T->isArrayType() || T->isRecordType() ||
T->isAnyComplexType() || T->isVectorType();
+ SVal Val;
if (Args.size() > 1 || (E->isPRValue() && IsCompound && !IsTransparent)) {
llvm::ImmutableList<SVal> ArgList = getBasicVals().getEmptySValList();
for (Expr *E : llvm::reverse(Args))
ArgList = getBasicVals().prependSVal(S->getSVal(E, SF), ArgList);
- B.generateNode(E, Pred,
- S->BindExpr(E, SF, svalBuilder.makeCompoundVal(T, ArgList)));
+ Val = getSValBuilder().makeCompoundVal(T, ArgList);
+ } else if (Args.size() == 0) {
+ Val = getSValBuilder().makeZeroVal(T);
} else {
- B.generateNode(E, Pred,
- S->BindExpr(E, SF,
- Args.size() == 0
- ? getSValBuilder().makeZeroVal(T)
- : S->getSVal(Args.front(), SF)));
+ Val = S->getSVal(Args.front(), SF);
}
+ B.generateNode(E, Pred, S->BindExpr(E, SF, Val));
}
>From 703f6342c2fbf1470d0f2da629b9170944fcf2dd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Wed, 17 Jun 2026 18:06:53 +0200
Subject: [PATCH 8/8] Remove NodeBuilder from ConstructInitList
Another very straightforward case that showcases the usefulness of
`makeNodeWithBinding`.
---
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 151b957b21f06..77faa675b90b8 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3962,7 +3962,6 @@ void ExprEngine::ConstructInitList(const Expr *E, ArrayRef<Expr *> Args,
const StackFrame *SF = Pred->getStackFrame();
- NodeBuilder B(Pred, Dst, *currBldrCtx);
ProgramStateRef S = Pred->getState();
QualType T = E->getType().getCanonicalType();
@@ -3981,5 +3980,5 @@ void ExprEngine::ConstructInitList(const Expr *E, ArrayRef<Expr *> Args,
} else {
Val = S->getSVal(Args.front(), SF);
}
- B.generateNode(E, Pred, S->BindExpr(E, SF, Val));
+ Dst.insert(Engine.makeNodeWithBinding(Pred, E, Val));
}
More information about the cfe-commits
mailing list