[clang] a6caf52 - [NFC][analyzer] Clean up and document `ExplodedNodeSet` (#187742)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 23 07:09:38 PDT 2026
Author: Donát Nagy
Date: 2026-03-23T15:09:32+01:00
New Revision: a6caf52908bf499fd7fc3ae96dcd3ef8c41d5ce1
URL: https://github.com/llvm/llvm-project/commit/a6caf52908bf499fd7fc3ae96dcd3ef8c41d5ce1
DIFF: https://github.com/llvm/llvm-project/commit/a6caf52908bf499fd7fc3ae96dcd3ef8c41d5ce1.diff
LOG: [NFC][analyzer] Clean up and document `ExplodedNodeSet` (#187742)
`ExplodedNodeSet` is a simple and useful utility type in the analyzer,
but its insertion methods were a bit confusing, so this commit clarifies
them (and adds doc-comments for this class).
Previously this class had `void Add(ExplodedNode*)` for inserting single
nodes and `void insert(const ExplodedNodeSet &)` for inserting all nodes
from another set; but `ExplodedNode*` is implicitly convertible to
`ExplodedNodeSet`, so it was also possible to insert single nodes with
`insert`. There was also a subtle difference between `Set.Add(Node)` and
`Set.insert(Node)`: `Add` accepted and ignored nullpointers and sink
nodes (which is often useful) while the constructor
`ExplodedNodeSet(ExplodedNode*)` enforced the same invariant in a less
helpful way, with an assertion.
This commit eliminates the name `Add` (because `insert` is more
customary for set types), but standardizes its "null or sink nodes are
silently ignored" behavior which is very useful in practice.
After this commit `insert` has two overloads:
`ExplodedNodeSet::insert(ExplodedNode*)` which inserts a single node (or
does nothing if the argument is null or sink) and
`ExplodedNodeSet::insert(const ExplodedNodeSet&)` which inserts all
nodes from the other set.
Note that around half of the calls to `Add` were recently introduced by
my `NodeBuilder` cleanup commit series; I expect to introduce dozens of
calls in the foreseeable future -- this commit can be considered as
preparation for those.
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
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
index 8d5dcc1ca8b13..76bd8346f269e 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
@@ -259,7 +259,7 @@ class NodeBuilder {
NodeBuilder(ExplodedNode *SrcNode, ExplodedNodeSet &DstSet,
const NodeBuilderContext &Ctx)
: NodeBuilder(DstSet, Ctx) {
- Frontier.Add(SrcNode);
+ Frontier.insert(SrcNode);
}
NodeBuilder(const ExplodedNodeSet &SrcSet, ExplodedNodeSet &DstSet,
@@ -314,7 +314,7 @@ class NodeBuilder {
void takeNodes(ExplodedNode *N) { Frontier.erase(N); }
void addNodes(const ExplodedNodeSet &S) { Frontier.insert(S); }
- void addNodes(ExplodedNode *N) { Frontier.Add(N); }
+ void addNodes(ExplodedNode *N) { Frontier.insert(N); }
};
/// BranchNodeBuilder is responsible for constructing the nodes
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
index a536f5ee046e1..bb2297a6889c9 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
@@ -440,23 +440,23 @@ class ExplodedGraph {
void collectNode(ExplodedNode *node);
};
+/// ExplodedNodeSet is a set of `ExplodedNode *` elements with the invariant
+/// that its elements cannot be nullpointers or sink nodes. Insertion of null
+/// or sink nodes is silently ignored (which is comfortable in many use cases).
+/// Note that `ExplodedNode *` is implicitly convertible to an
+/// `ExplodedNodeSet` containing 0 or 1 elements (where null pointers and sink
+/// nodes converted to the empty set).
+/// This type has set semantics (repeated insertions are ignored), but the
+/// iteration order is always the order of (first) insertion.
class ExplodedNodeSet {
using ImplTy = llvm::SmallSetVector<ExplodedNode *, 4>;
ImplTy Impl;
public:
- ExplodedNodeSet(ExplodedNode *N) {
- assert(N && !N->isSink());
- Impl.insert(N);
- }
+ ExplodedNodeSet(ExplodedNode *N) { insert(N); }
ExplodedNodeSet() = default;
- void Add(ExplodedNode *N) {
- if (N && !N->isSink())
- Impl.insert(N);
- }
-
using iterator = ImplTy::iterator;
using const_iterator = ImplTy::const_iterator;
@@ -466,8 +466,14 @@ class ExplodedNodeSet {
void clear() { Impl.clear(); }
+ void insert(ExplodedNode *N) {
+ if (N && !N->isSink())
+ Impl.insert(N);
+ }
+
void insert(const ExplodedNodeSet &S) {
- assert(&S != this);
+ if (&S == this)
+ return;
if (empty())
Impl = S.Impl;
else
diff --git a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
index 26672ff75996c..a348f7947ad98 100644
--- a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -689,7 +689,7 @@ ExplodedNode *NodeBuilder::generateNode(const ProgramPoint &Loc,
Frontier.erase(FromN);
ExplodedNode *N = C.getEngine().makeNode(Loc, State, FromN, MarkAsSink);
- Frontier.Add(N);
+ Frontier.insert(N);
return N;
}
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 0fb8f1d00b618..e9522a7975515 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1131,7 +1131,7 @@ void ExprEngine::ProcessStmt(const Stmt *currStmt, ExplodedNode *Pred) {
removeDead(Pred, CleanedStates, currStmt,
Pred->getLocationContext());
} else
- CleanedStates.Add(Pred);
+ CleanedStates.insert(Pred);
// Visit the statement.
ExplodedNodeSet Dst;
@@ -1190,7 +1190,7 @@ void ExprEngine::ProcessInitializer(const CFGInitializer CFGInit,
// But we still need to stop tracking the object under construction.
State = finishObjectConstruction(State, BMI, LC);
PostStore PS(Init, LC, /*Loc*/ nullptr, /*tag*/ nullptr);
- Tmp.Add(Engine.makeNode(PS, State, Pred));
+ Tmp.insert(Engine.makeNode(PS, State, Pred));
} else {
const ValueDecl *Field;
if (BMI->isIndirectMemberInitializer()) {
@@ -1245,7 +1245,7 @@ void ExprEngine::ProcessInitializer(const CFGInitializer CFGInit,
ExplodedNodeSet Dst;
for (ExplodedNode *Pred : Tmp)
- Dst.Add(Engine.makeNode(PP, Pred->getState(), Pred));
+ Dst.insert(Engine.makeNode(PP, Pred->getState(), Pred));
// Enqueue the new nodes onto the work list.
Engine.enqueueStmtNodes(Dst, getCurrBlock(), currStmtIdx);
}
@@ -1328,7 +1328,7 @@ void ExprEngine::ProcessNewAllocator(const CXXNewExpr *NE,
const LocationContext *LCtx = Pred->getLocationContext();
PostImplicitCall PP(NE->getOperatorNew(), NE->getBeginLoc(), LCtx,
getCFGElementRef());
- Dst.Add(Engine.makeNode(PP, Pred->getState(), Pred));
+ Dst.insert(Engine.makeNode(PP, Pred->getState(), Pred));
}
Engine.enqueueStmtNodes(Dst, getCurrBlock(), currStmtIdx);
}
@@ -2999,7 +2999,7 @@ void ExprEngine::processIndirectGoto(ExplodedNodeSet &Dst, const Expr *Tgt,
// FIXME: If 'V' was a symbolic value, then record that on this execution
// path it is equal to the address of the label leading to 'Succ'.
BlockEdge BE(getCurrBlock(), Succ, Pred->getLocationContext());
- Dst.Add(Engine.makeNode(BE, State, Pred));
+ Dst.insert(Engine.makeNode(BE, State, Pred));
}
}
}
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index cf22b62225f2f..701c7fdc88497 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -93,7 +93,7 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,
// In that case, copying the empty base class subobject would overwrite the
// object that it overlaps with - so let's not do that.
// See issue-157467.cpp for an example.
- Dst.Add(Pred);
+ Dst.insert(Pred);
}
PostStmt PS(CallExpr, LCtx);
@@ -1130,7 +1130,7 @@ void ExprEngine::VisitCXXCatchStmt(const CXXCatchStmt *CS, ExplodedNode *Pred,
ExplodedNodeSet &Dst) {
const VarDecl *VD = CS->getExceptionDecl();
if (!VD) {
- Dst.Add(Pred);
+ Dst.insert(Pred);
return;
}
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index f6ba3699312ec..b4dc4fa8a077d 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -169,7 +169,7 @@ void ExprEngine::removeDeadOnEndOfFunction(ExplodedNode *Pred,
const CFGBlock *Blk = nullptr;
std::tie(LastSt, Blk) = getLastStmt(Pred);
if (!Blk || !LastSt) {
- Dst.Add(Pred);
+ Dst.insert(Pred);
return;
}
@@ -369,7 +369,7 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) {
/*DiagnosticStmt=*/CalleeCtx->getAnalysisDeclContext()->getBody(),
ProgramPoint::PostStmtPurgeDeadSymbolsKind);
} else {
- CleanedNodes.Add(CEBNode);
+ CleanedNodes.insert(CEBNode);
}
// The second half of this process happens in the caller context. This is an
More information about the cfe-commits
mailing list