Author: Donát Nagy Date: 2026-02-18T10:37:28+01:00 New Revision: fa31acedc547481feebe6fb1cf920a1e3e45c6b6
URL: https://github.com/llvm/llvm-project/commit/fa31acedc547481feebe6fb1cf920a1e3e45c6b6 DIFF: https://github.com/llvm/llvm-project/commit/fa31acedc547481feebe6fb1cf920a1e3e45c6b6.diff LOG: [NFCI][analyzer] Simplify NodeBuilder constructors (#181875) This commit simplifies the construction of `NodeBuilder` and its subclasses in three ways: - It removes an assertion that only appeared in one of the two constructors of `NodeBuilder`. While the asserted "no sinks in the `Frontier`" invariant apparently holds, this assertion was not the right place for expressing it. (In the future I might re-add a similar assertion in a more reasonable location.) - It adds a new constructor for `NodeBuilder` that doesn't take a "source node(s)" argument, because this argument was often irrelevant. - It removes the "source node(s)" arguments from the subclasses of `NodeBuilder` because it was always completely irrelevant in those situations. Before this commit, constructors of `NodeBuilder` took three arguments: - the source node or nodes, - the destination node set where the freshly built nodes are placed, - the `NodeBuilderContext` that provides access to the exploded graph. Among these, the latter two were saved in data members, but the only use of the source node(s) was that the constructor inserted them into the destination node set (= the data member `Frontier`). However, adding the source node(s) to the `Frontier` is usually irrelevant, because later set operations almost always remove them from the `Frontier`. This is especially clear in the subclasses derived from `NodeBuilder` whose constructor immediately remove the source node(s) from the `Frontier`. (In other situations, the source node(s) are usually removed from the `Frontier` by the calls to `generateNode()`.) This commit introduces a new constructor for `NodeBuilder` that doesn't have a "source node(s)" parameter, then uses this constructor to implement the constructors of the subclasses without pointless insertion and then removal of nodes from the destination set. Note that if a node `N` was already present in the destination set, then its insertion+removal would remove it from the set; but I verified that the destination set passed to constructors of subclasses of `NodeBuilder` is always an empty set (at this point). The new constructor of `NodeBuiler` is public because later it will be also useful for simplifying other code that uses `NodeBuilder` directly. Added: Modified: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.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 a7fe34b425d5d..d4669fd2d1720 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h @@ -245,30 +245,25 @@ class NodeBuilder { /// the builder dies. ExplodedNodeSet &Frontier; - bool hasNoSinksInFrontier() { - for (const auto I : Frontier) - if (I->isSink()) - return false; - return true; - } - ExplodedNode *generateNodeImpl(const ProgramPoint &PP, ProgramStateRef State, ExplodedNode *Pred, bool MarkAsSink = false); public: + NodeBuilder(ExplodedNodeSet &DstSet, const NodeBuilderContext &Ctx) + : C(Ctx), Frontier(DstSet) {} + NodeBuilder(ExplodedNode *SrcNode, ExplodedNodeSet &DstSet, const NodeBuilderContext &Ctx) - : C(Ctx), Frontier(DstSet) { + : NodeBuilder(DstSet, Ctx) { Frontier.Add(SrcNode); } NodeBuilder(const ExplodedNodeSet &SrcSet, ExplodedNodeSet &DstSet, const NodeBuilderContext &Ctx) - : C(Ctx), Frontier(DstSet) { + : NodeBuilder(DstSet, Ctx) { Frontier.insert(SrcSet); - assert(hasNoSinksInFrontier()); } /// Generates a node in the ExplodedGraph. @@ -333,21 +328,9 @@ class BranchNodeBuilder : public NodeBuilder { const CFGBlock *DstF; public: - BranchNodeBuilder(ExplodedNode *SrcNode, ExplodedNodeSet &DstSet, - const NodeBuilderContext &C, const CFGBlock *DT, - const CFGBlock *DF) - : NodeBuilder(SrcNode, DstSet, C), DstT(DT), DstF(DF) { - // The branch node builder does not generate autotransitions. - // If there are no successors it means that both branches are infeasible. - takeNodes(SrcNode); - } - - BranchNodeBuilder(const ExplodedNodeSet &SrcSet, ExplodedNodeSet &DstSet, - const NodeBuilderContext &C, const CFGBlock *DT, - const CFGBlock *DF) - : NodeBuilder(SrcSet, DstSet, C), DstT(DT), DstF(DF) { - takeNodes(SrcSet); - } + 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); @@ -358,14 +341,9 @@ class IndirectGotoNodeBuilder : public NodeBuilder { const Expr *Target; public: - IndirectGotoNodeBuilder(ExplodedNode *SrcNode, ExplodedNodeSet &DstSet, - NodeBuilderContext &Ctx, const Expr *Tgt, - const CFGBlock *Dispatch) - : NodeBuilder(SrcNode, DstSet, Ctx), DispatchBlock(*Dispatch), - Target(Tgt) { - // The indirect goto node builder does not generate autotransitions. - takeNodes(SrcNode); - } + IndirectGotoNodeBuilder(ExplodedNodeSet &DstSet, NodeBuilderContext &Ctx, + const Expr *Tgt, const CFGBlock *Dispatch) + : NodeBuilder(DstSet, Ctx), DispatchBlock(*Dispatch), Target(Tgt) {} using iterator = CFGBlock::const_succ_iterator; @@ -386,12 +364,8 @@ class IndirectGotoNodeBuilder : public NodeBuilder { class SwitchNodeBuilder : public NodeBuilder { public: - SwitchNodeBuilder(ExplodedNode *SrcNode, ExplodedNodeSet &DstSet, - const NodeBuilderContext &Ctx) - : NodeBuilder(SrcNode, DstSet, Ctx) { - // The switch node builder does not generate autotransitions. - takeNodes(SrcNode); - } + SwitchNodeBuilder(ExplodedNodeSet &DstSet, const NodeBuilderContext &Ctx) + : NodeBuilder(DstSet, Ctx) {} using iterator = CFGBlock::const_succ_reverse_iterator; diff --git a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp index 1e5a04b8efbe4..9f0fcc4c3e3a5 100644 --- a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp @@ -428,7 +428,7 @@ void CoreEngine::HandleBlockExit(const CFGBlock * B, ExplodedNode *Pred) { NodeBuilderContext Ctx(*this, B, Pred); ExplodedNodeSet Dst; IndirectGotoNodeBuilder Builder( - Pred, Dst, Ctx, cast<IndirectGotoStmt>(Term)->getTarget(), + Dst, Ctx, cast<IndirectGotoStmt>(Term)->getTarget(), *(B->succ_begin())); ExprEng.processIndirectGoto(Builder, Pred); diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index e0bc083f6b978..8c9265ae55311 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -1650,7 +1650,7 @@ void ExprEngine::processCleanupTemporaryBranch(const CXXBindTemporaryExpr *BTE, ExplodedNodeSet &Dst, const CFGBlock *DstT, const CFGBlock *DstF) { - BranchNodeBuilder TempDtorBuilder(Pred, Dst, BldCtx, DstT, DstF); + BranchNodeBuilder TempDtorBuilder(Dst, BldCtx, DstT, DstF); ProgramStateRef State = Pred->getState(); const LocationContext *LC = Pred->getLocationContext(); if (getObjectUnderConstruction(State, BTE, LC)) { @@ -2850,7 +2850,7 @@ void ExprEngine::processBranch( // Check for NULL conditions; e.g. "for(;;)" if (!Condition) { - BranchNodeBuilder NullCondBldr(Pred, Dst, BldCtx, DstT, DstF); + BranchNodeBuilder NullCondBldr(Dst, BldCtx, DstT, DstF); NullCondBldr.generateNode(Pred->getState(), true, Pred); return; } @@ -2870,7 +2870,7 @@ void ExprEngine::processBranch( if (CheckersOutSet.empty()) return; - BranchNodeBuilder Builder(CheckersOutSet, Dst, BldCtx, DstT, DstF); + BranchNodeBuilder Builder(Dst, BldCtx, DstT, DstF); for (ExplodedNode *PredN : CheckersOutSet) { if (PredN->isSink()) continue; @@ -2979,7 +2979,7 @@ void ExprEngine::processStaticInitializer( const auto *VD = cast<VarDecl>(DS->getSingleDecl()); ProgramStateRef state = Pred->getState(); bool initHasRun = state->contains<InitializedGlobalsSet>(VD); - BranchNodeBuilder Builder(Pred, Dst, BuilderCtx, DstT, DstF); + BranchNodeBuilder Builder(Dst, BuilderCtx, DstT, DstF); if (!initHasRun) { state = state->add<InitializedGlobalsSet>(VD); @@ -3108,7 +3108,7 @@ void ExprEngine::processSwitch(NodeBuilderContext &BC, const SwitchStmt *Switch, ExplodedNode *Pred, ExplodedNodeSet &Dst) { const Expr *Condition = Switch->getCond(); - SwitchNodeBuilder Builder(Pred, Dst, BC); + SwitchNodeBuilder Builder(Dst, BC); ProgramStateRef State = Pred->getState(); SVal CondV = State->getSVal(Condition, Pred->getLocationContext()); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
