[clang] [NFC][analyzer] Remove class 'NodeBuilderContext' (PR #218442)
Donát Nagy via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 24 10:51:52 PDT 2026
https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/218442
>From c46a5fa899851f1ae083e41a619ebe602b123b52 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 24 Aug 2026 17:04:45 +0200
Subject: [PATCH 1/5] Remove NodeBuilderContext in handleConstructionContext
The value passed to the NodeBuilderContext parameter was always
'currBldrCtx', so we can use 'getNumVisitedCurrent()' to get the current
block count.
---
.../clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h | 5 ++---
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 4 ++--
clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 2 +-
3 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 68d4362aca941..9b3709b2e3c41 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -815,12 +815,11 @@ class ExprEngine {
/// and updateObjectsUnderConstruction.
std::pair<ProgramStateRef, SVal>
handleConstructionContext(const Expr *E, ProgramStateRef State,
- const NodeBuilderContext *BldrCtx,
const StackFrame *SF, const ConstructionContext *CC,
EvalCallOptions &CallOpts, unsigned Idx = 0) {
- SVal V = computeObjectUnderConstruction(E, State, BldrCtx->blockCount(), SF,
- CC, CallOpts, Idx);
+ SVal V = computeObjectUnderConstruction(E, State, getNumVisitedCurrent(),
+ SF, CC, CallOpts, Idx);
State = updateObjectsUnderConstruction(V, E, State, SF, CC, CallOpts);
return std::make_pair(State, V);
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 3df2d3d9e3674..9fb167ee2ea4a 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -602,8 +602,8 @@ void ExprEngine::handleConstructor(const Expr *E, ExplodedNode *Pred,
}
// The target region is found from construction context.
- std::tie(State, Target) = handleConstructionContext(CE, State, currBldrCtx,
- SF, CC, CallOpts, Idx);
+ std::tie(State, Target) =
+ handleConstructionContext(CE, State, SF, CC, CallOpts, Idx);
break;
}
case CXXConstructionKind::VirtualBase: {
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index e0139484b2b48..66fdb6d117a96 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -773,7 +773,7 @@ ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call,
assert(RTC->getStmt() == Call.getOriginExpr());
EvalCallOptions CallOpts; // FIXME: We won't really need those.
std::tie(State, Target) =
- handleConstructionContext(Call.getOriginExpr(), State, currBldrCtx, SF,
+ handleConstructionContext(Call.getOriginExpr(), State, SF,
RTC->getConstructionContext(), CallOpts);
const MemRegion *TargetR = Target.getAsRegion();
assert(TargetR);
>From 18743597ac1cc5c8bd922a11fee0900ffac282d6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 24 Aug 2026 17:11:41 +0200
Subject: [PATCH 2/5] Remove currBldrCtx
---
.../Core/PathSensitive/ExprEngine.h | 54 ++++---------------
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 2 +-
2 files changed, 10 insertions(+), 46 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 9b3709b2e3c41..a76098aba19b5 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -156,29 +156,8 @@ class ExprEngine {
SValBuilder &svalBuilder;
unsigned int currStmtIdx = 0;
-
- /// Pointer to a (so-called, somewhat misnamed) NodeBuilderContext object
- /// which has three independent roles:
- /// - It holds a pointer to the CFGBlock that is currently under analysis.
- /// (This is the primary way to get the current block.)
- /// - It holds a pointer to the current StackFrame. (This is rarely
- /// used, the stack frame is usually queried from a recent
- /// ExplodedNode. Unfortunately it seems that these two sources of truth
- /// are not always consistent.)
- /// - It can be used for constructing `NodeBuilder`s. Practically all
- /// `NodeBuilder` objects are useless complications in the code, so I
- /// intend to replace them with direct use of `CoreEngine::makeNode`.
- /// TODO: Eventually `currBldrCtx` should be replaced by two separate fields:
- /// `const CFGBlock *CurrBlock` & `const StackFrame *CurrStackFrame`
- /// that are kept up-to-date and are almost always non-null during the
- /// analysis. I will switch to this more natural representation when
- /// `NodeBuilder`s are eliminated from the code.
- const NodeBuilderContext *currBldrCtx = nullptr;
- /// Historically `currBldrCtx` pointed to a local variable in some stack
- /// frame. This field is introduced as a temporary measure to allow a gradual
- /// transition. Only use this in {re,}setCurrStackFrameAndBlock!
- /// TODO: Remove this temporary hack.
- std::optional<NodeBuilderContext> OwnedCurrBldrCtx;
+ const StackFrame *CurrStackFrame = nullptr;
+ const CFGBlock *CurrBlock = nullptr;
/// Helper object to determine if an Objective-C message expression
/// implicitly never returns.
@@ -235,12 +214,6 @@ class ExprEngine {
return &CTU;
}
- // FIXME: Ideally the body of this method should look like
- // CurrStackFrame = SF;
- // CurrBlock = B;
- // where CurrStackFrame and CurrBlock are new member variables that
- // fulfill the roles of `currBldrCtx` in a more natural way.
- // This implementation is a temporary measure to allow a gradual transition.
void setCurrStackFrameAndBlock(const StackFrame *SF, const CFGBlock *B) {
// The current StackFrame and Block is reset at the beginning of
// dispatchWorkItem. Ideally, this method should be called only once per
@@ -249,20 +222,15 @@ class ExprEngine {
// StackFrame and Block needs to change in the middle of a single step
// (which currently happens only once, in processCallExit), use an explicit
// call to resetCurrStackFrameAndBlock.
- assert(!currBldrCtx && !OwnedCurrBldrCtx &&
+ assert(!CurrBlock && !CurrStackFrame &&
"The current StackFrame and Block is already set");
- OwnedCurrBldrCtx.emplace(Engine, B, SF);
- currBldrCtx = &*OwnedCurrBldrCtx;
+ CurrStackFrame = SF;
+ CurrBlock = B;
}
void resetCurrStackFrameAndBlock() {
- currBldrCtx = nullptr;
- OwnedCurrBldrCtx = std::nullopt;
- }
-
- const NodeBuilderContext &getBuilderContext() const {
- assert(currBldrCtx);
- return *currBldrCtx;
+ CurrStackFrame = nullptr;
+ CurrBlock = nullptr;
}
const StackFrame *getRootStackFrame() const {
@@ -277,15 +245,11 @@ class ExprEngine {
/// (e.g. a recent `ExplodedNode`). Traditionally this stack frame is
/// only used for block count calculations (`getNumVisited`); it is probably
/// wise to follow this tradition until the discrepancies are resolved.
- const StackFrame *getCurrStackFrame() const {
- return currBldrCtx ? currBldrCtx->getStackFrame() : nullptr;
- }
+ const StackFrame *getCurrStackFrame() const { return CurrStackFrame; }
/// Get the 'current' CFGBlock corresponding to the current work item
/// (elementary analysis step handled by `dispatchWorkItem`).
- const CFGBlock *getCurrBlock() const {
- return currBldrCtx ? currBldrCtx->getBlock() : nullptr;
- }
+ const CFGBlock *getCurrBlock() const { return CurrBlock; }
ConstCFGElementRef getCFGElementRef() const {
return {getCurrBlock(), currStmtIdx};
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index e6349eb4eba2a..fea5536bf3244 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1153,7 +1153,7 @@ void ExprEngine::ProcessLifetimeEnd(const Stmt *S, const VarDecl *D,
ExplodedNodeSet Dst;
getCheckerManager().runCheckersForLifetimeEnd(Dst, Src, D, *this);
- Engine.enqueueStmtNodes(Dst, currBldrCtx->getBlock(), currStmtIdx);
+ Engine.enqueueStmtNodes(Dst, CurrBlock, currStmtIdx);
}
void ExprEngine::ProcessInitializer(const CFGInitializer CFGInit,
>From 182596181b25a313c61f82ed0ca72c2cbfbbacd3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 24 Aug 2026 17:12:51 +0200
Subject: [PATCH 3/5] Remove class NodeBuilderContext
---
.../StaticAnalyzer/Core/CheckerManager.h | 1 -
.../Core/PathSensitive/CoreEngine.h | 32 -------------------
.../Core/PathSensitive/ExprEngine.h | 1 -
3 files changed, 34 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 3311774f069ea..46e648c6b7d90 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -49,7 +49,6 @@ class ExplodedNodeSet;
class ExprEngine;
struct EvalCallOptions;
class MemRegion;
-class NodeBuilderContext;
class ObjCMethodCall;
class RegionAndSymbolInvalidationTraits;
class SVal;
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
index 71443e4434462..d26c0d9257b0f 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
@@ -49,7 +49,6 @@ class ExprEngine;
/// It traverses the CFG and generates the ExplodedGraph.
class CoreEngine {
friend class ExprEngine;
- friend class NodeBuilderContext;
public:
using BlocksExhausted =
@@ -210,37 +209,6 @@ class CoreEngine {
DataTag::Factory &getDataTags() { return DataTags; }
};
-class NodeBuilderContext {
- const CoreEngine &Eng;
- const CFGBlock *Block;
- const StackFrame *SF;
-
-public:
- NodeBuilderContext(const CoreEngine &E, const CFGBlock *B,
- const StackFrame *S)
- : Eng(E), Block(B), SF(S) {
- assert(B);
- }
-
- NodeBuilderContext(const CoreEngine &E, const CFGBlock *B, ExplodedNode *N)
- : NodeBuilderContext(E, B, N->getStackFrame()) {}
-
- /// Return the CoreEngine associated with this builder.
- const CoreEngine &getEngine() const { return Eng; }
-
- /// Return the CFGBlock associated with this builder.
- const CFGBlock *getBlock() const { return Block; }
-
- /// Return the stack frame associated with this builder.
- const StackFrame *getStackFrame() const { return SF; }
-
- /// Returns the number of times the current basic block has been
- /// visited on the exploded graph path.
- unsigned blockCount() const {
- return Eng.WList->getBlockCounter().getNumVisited(SF, Block->getBlockID());
- }
-};
-
} // namespace ento
} // namespace clang
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index a76098aba19b5..478012c3744ca 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -83,7 +83,6 @@ class ConstraintManager;
class ExplodedNodeSet;
class ExplodedNode;
class MemRegion;
-class NodeBuilderContext;
class ProgramState;
class ProgramStateManager;
class RegionAndSymbolInvalidationTraits;
>From faf0ec6435bc5258dce4f9264c4d7bc5e9afd911 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 24 Aug 2026 17:17:20 +0200
Subject: [PATCH 4/5] Remove mention of NodeBuilderContext in comment
---
clang/test/Analysis/stack-frame-context-revision.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang/test/Analysis/stack-frame-context-revision.cpp b/clang/test/Analysis/stack-frame-context-revision.cpp
index 3ed85e4cb20b8..5137b60289e38 100644
--- a/clang/test/Analysis/stack-frame-context-revision.cpp
+++ b/clang/test/Analysis/stack-frame-context-revision.cpp
@@ -1,10 +1,10 @@
// RUN: %clang_analyze_cc1 -std=c++14 -analyzer-checker=core,cplusplus.NewDelete -verify %s
// expected-no-diagnostics:
-// From now the profile of the 'StackFrame' also contains the
-// 'NodeBuilderContext::blockCount()'. With this addition we can distinguish
-// between the 'StackArgumentsSpaceRegion' of the 'P' arguments being different
-// on every iteration.
+// From now the profile of the 'StackFrame' also contains the 'BlockCount'
+// ('ExprEngine::getNumVisitedCurrent()') value at the call site. With this
+// addition we can distinguish between the 'StackArgumentsSpaceRegion' of the
+// 'P' arguments being different on every iteration.
typedef __INTPTR_TYPE__ intptr_t;
>From 4a2705506b13416424f39ab3b6044544f79e14a8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 24 Aug 2026 19:51:14 +0200
Subject: [PATCH 5/5] Follow AI nitpicks
---
.../clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h | 1 +
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 478012c3744ca..64a2ebe5149e4 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -223,6 +223,7 @@ class ExprEngine {
// call to resetCurrStackFrameAndBlock.
assert(!CurrBlock && !CurrStackFrame &&
"The current StackFrame and Block is already set");
+ assert(SF && B && "The StackFrame and Block must be non-null");
CurrStackFrame = SF;
CurrBlock = B;
}
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index fea5536bf3244..a8fa11b078993 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1153,7 +1153,7 @@ void ExprEngine::ProcessLifetimeEnd(const Stmt *S, const VarDecl *D,
ExplodedNodeSet Dst;
getCheckerManager().runCheckersForLifetimeEnd(Dst, Src, D, *this);
- Engine.enqueueStmtNodes(Dst, CurrBlock, currStmtIdx);
+ Engine.enqueueStmtNodes(Dst, getCurrBlock(), currStmtIdx);
}
void ExprEngine::ProcessInitializer(const CFGInitializer CFGInit,
More information about the cfe-commits
mailing list