[clang] 9ca929f - [NFC][analyzer] Eliminate BranchNodeBuilder (#192744)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 20 01:25:26 PDT 2026
Author: Donát Nagy
Date: 2026-04-20T10:25:22+02:00
New Revision: 9ca929f53508f93e282f37cb3e1295b53d7e58fb
URL: https://github.com/llvm/llvm-project/commit/9ca929f53508f93e282f37cb3e1295b53d7e58fb
DIFF: https://github.com/llvm/llvm-project/commit/9ca929f53508f93e282f37cb3e1295b53d7e58fb.diff
LOG: [NFC][analyzer] Eliminate BranchNodeBuilder (#192744)
This commit removes the class `BranchNodeBuilder` because it didn't
provide enough advantages to justify its existence. (This class wasn't
as bad as `SwitchNodeBuilder` and `IndirectGotoNodeBuilder`, but its
single method was very simple so I still think that it is better to
eliminate it.)
To be able to unify the use of `LocationContext`s in `processBranch`,
this commit also replaces the overcomplicated helper function
`getInlinedLocationContext` with use of `LocationContext::inTopFrame()`.
The code of this helper function was written in March 2012, before the
introduction of `inTopFrame` (November 2012). It was originally in the
body of the method `ExprEngine::processCFGBlockEntrance` and when I
extracted it in 2025 by commit 9600a12f0de233324b559f60997b9c2db153fede
(because I also needed to call it from `processBranch`) I didn't notice
that I can achieve the same goal with simpler logic.
Additionally, a last stray reference to `friend class SwitchNodeBuilder`
is removed (it was overlooked I removed that class in
c80443cd37b2e2788cba67ffa180a6331e5f0791).
Added:
Modified:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
index 7dfdca0d54a67..57b0947093b78 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
@@ -316,21 +316,6 @@ class NodeBuilder {
void addNodes(ExplodedNode *N) { Frontier.insert(N); }
};
-/// BranchNodeBuilder is responsible for constructing the nodes
-/// corresponding to the two branches of the if statement - true and false.
-class BranchNodeBuilder : public NodeBuilder {
- const CFGBlock *DstT;
- const CFGBlock *DstF;
-
-public:
- BranchNodeBuilder(ExplodedNodeSet &DstSet, const NodeBuilderContext &C,
- const CFGBlock *DT, const CFGBlock *DF)
- : NodeBuilder(DstSet, C), DstT(DT), DstF(DF) {}
-
- ExplodedNode *generateNode(ProgramStateRef State, bool branch,
- ExplodedNode *Pred);
-};
-
} // namespace ento
} // namespace clang
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
index bb2297a6889c9..2988b1bbad68f 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
@@ -64,11 +64,9 @@ class ExplodedGraph;
// successors to it at any time after creating it.
class ExplodedNode : public llvm::FoldingSetNode {
- friend class BranchNodeBuilder;
friend class CoreEngine;
friend class ExplodedGraph;
friend class NodeBuilder;
- friend class SwitchNodeBuilder;
/// Efficiently stores a list of ExplodedNodes, or an optional flag.
///
diff --git a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
index 17bee557fe029..89d6bc7a6df3e 100644
--- a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -693,17 +693,3 @@ ExplodedNode *NodeBuilder::generateNode(const ProgramPoint &Loc,
return N;
}
-
-ExplodedNode *BranchNodeBuilder::generateNode(ProgramStateRef State,
- bool Branch,
- ExplodedNode *NodePred) {
- const CFGBlock *Dst = Branch ? DstT : DstF;
-
- if (!Dst)
- return nullptr;
-
- ProgramPoint Loc =
- BlockEdge(C.getBlock(), Dst, NodePred->getLocationContext());
- ExplodedNode *Succ = NodeBuilder::generateNode(Loc, State, NodePred);
- return Succ;
-}
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 35abd792a5ad0..b69b45acb7989 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1641,13 +1641,13 @@ void ExprEngine::processCleanupTemporaryBranch(const CXXBindTemporaryExpr *BTE,
ExplodedNodeSet &Dst,
const CFGBlock *DstT,
const CFGBlock *DstF) {
- BranchNodeBuilder TempDtorBuilder(Dst, *currBldrCtx, DstT, DstF);
ProgramStateRef State = Pred->getState();
const LocationContext *LC = Pred->getLocationContext();
- if (getObjectUnderConstruction(State, BTE, LC)) {
- TempDtorBuilder.generateNode(State, true, Pred);
- } else {
- TempDtorBuilder.generateNode(State, false, Pred);
+
+ std::optional<SVal> Obj = getObjectUnderConstruction(State, BTE, LC);
+ if (const CFGBlock *DstBlock = Obj ? DstT : DstF) {
+ BlockEdge BE(getCurrBlock(), DstBlock, LC);
+ Dst.insert(Engine.makeNode(BE, State, Pred));
}
}
@@ -2547,20 +2547,6 @@ bool ExprEngine::replayWithoutInlining(ExplodedNode *N,
return true;
}
-/// Return the innermost location context which is inlined at `Node`, unless
-/// it's the top-level (entry point) location context.
-static const LocationContext *getInlinedLocationContext(ExplodedNode *Node,
- ExplodedGraph &G) {
- const LocationContext *CalleeLC = Node->getLocation().getLocationContext();
- const LocationContext *RootLC =
- G.getRoot()->getLocation().getLocationContext();
-
- if (CalleeLC->getStackFrame() == RootLC->getStackFrame())
- return nullptr;
-
- return CalleeLC;
-}
-
/// Block entrance. (Update counters).
/// FIXME: `BlockEdge &L` is only used for debug statistics, consider removing
/// it and using `BlockEntrance &BE` (where `BlockEntrance` is a subtype of
@@ -2620,7 +2606,8 @@ void ExprEngine::processCFGBlockEntrance(const BlockEdge &L,
const ExplodedNode *Sink =
Builder.generateSink(TaggedLoc, Pred->getState(), Pred);
- if (const LocationContext *LC = getInlinedLocationContext(Pred, G)) {
+ const LocationContext *LC = Pred->getLocationContext();
+ if (!LC->inTopFrame()) {
// FIXME: This will unconditionally prevent inlining this function (even
// from other entry points), which is not a reasonable heuristic: even if
// we reached max block count on this particular execution path, there
@@ -2846,10 +2833,17 @@ void ExprEngine::processBranch(
assert((!Condition || !isa<CXXBindTemporaryExpr>(Condition)) &&
"CXXBindTemporaryExprs are handled by processBindTemporary.");
+ const LocationContext *LC = Pred->getLocationContext();
+
// Check for NULL conditions; e.g. "for(;;)"
if (!Condition) {
- BranchNodeBuilder NullCondBldr(Dst, *currBldrCtx, DstT, DstF);
- NullCondBldr.generateNode(Pred->getState(), true, Pred);
+ if (!DstT) {
+ // I _hope_ that this "null condition + null transition to loop body"
+ // case is impossible, but I cannot prove this, so let's cover it.
+ return;
+ }
+ BlockEdge BE(getCurrBlock(), DstT, LC);
+ Dst.insert(Engine.makeNode(BE, Pred->getState(), Pred));
return;
}
@@ -2868,7 +2862,6 @@ void ExprEngine::processBranch(
if (CheckersOutSet.empty())
return;
- BranchNodeBuilder Builder(Dst, *currBldrCtx, DstT, DstF);
for (ExplodedNode *PredN : CheckersOutSet) {
ProgramStateRef PrevState = PredN->getState();
@@ -2914,7 +2907,10 @@ void ExprEngine::processBranch(
// default). If we intend to support and stabilize the loop widening,
// we must ensure that it 'plays nicely' with this logic.
if (!SkipTrueBranch || AMgr.options.ShouldWidenLoops) {
- Builder.generateNode(StTrue, true, PredN);
+ if (DstT) {
+ BlockEdge BE(getCurrBlock(), DstT, LC);
+ Dst.insert(Engine.makeNode(BE, StTrue, PredN));
+ }
} else if (!AMgr.options.InlineFunctionsWithAmbiguousLoops) {
// FIXME: There is an ancient and arbitrary heuristic in
// `ExprEngine::processCFGBlockEntrance` which prevents all further
@@ -2931,7 +2927,7 @@ void ExprEngine::processBranch(
// (activates if the third iteration can be entered, and will not
// recognize cases where the fourth iteration would't be completed), but
// should be good enough for practical purposes.
- if (const LocationContext *LC = getInlinedLocationContext(Pred, G)) {
+ if (!LC->inTopFrame()) {
Engine.FunctionSummaries->markShouldNotInline(
LC->getStackFrame()->getDecl());
}
@@ -2954,8 +2950,10 @@ void ExprEngine::processBranch(
bool BeforeFirstIteration = IterationsCompletedInLoop == std::optional{0};
bool SkipFalseBranch = BothFeasible && BeforeFirstIteration &&
AMgr.options.ShouldAssumeAtLeastOneIteration;
- if (!SkipFalseBranch)
- Builder.generateNode(StFalse, false, PredN);
+ if (!SkipFalseBranch && DstF) {
+ BlockEdge BE(getCurrBlock(), DstF, LC);
+ Dst.insert(Engine.makeNode(BE, StFalse, PredN));
+ }
}
}
}
@@ -2971,15 +2969,15 @@ void ExprEngine::processStaticInitializer(const DeclStmt *DS,
const CFGBlock *DstT,
const CFGBlock *DstF) {
const auto *VD = cast<VarDecl>(DS->getSingleDecl());
- ProgramStateRef state = Pred->getState();
- bool initHasRun = state->contains<InitializedGlobalsSet>(VD);
- BranchNodeBuilder Builder(Dst, *currBldrCtx, DstT, DstF);
+ ProgramStateRef State = Pred->getState();
+ bool InitHasRun = State->contains<InitializedGlobalsSet>(VD);
+ if (!InitHasRun)
+ State = State->add<InitializedGlobalsSet>(VD);
- if (!initHasRun) {
- state = state->add<InitializedGlobalsSet>(VD);
+ if (const CFGBlock *DstBlock = InitHasRun ? DstT : DstF) {
+ BlockEdge BE(getCurrBlock(), DstBlock, Pred->getLocationContext());
+ Dst.insert(Engine.makeNode(BE, State, Pred));
}
-
- Builder.generateNode(state, initHasRun, Pred);
}
/// processIndirectGoto - Called by CoreEngine. Used to generate successor
More information about the cfe-commits
mailing list