[clang] 236f634 - [NFC][analyzer] Remove NodeBuilders around defaultEvalCall (#203923)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 26 05:32:43 PDT 2026
Author: Donát Nagy
Date: 2026-06-26T14:32:39+02:00
New Revision: 236f63474d40850a33fd4bcc6d8d63a31cebb8f1
URL: https://github.com/llvm/llvm-project/commit/236f63474d40850a33fd4bcc6d8d63a31cebb8f1
DIFF: https://github.com/llvm/llvm-project/commit/236f63474d40850a33fd4bcc6d8d63a31cebb8f1.diff
LOG: [NFC][analyzer] Remove NodeBuilders around defaultEvalCall (#203923)
This change removes `NodeBuilder`s from the functions connected to
`defaultEvalCall` that were previously passing around `NodeBuilder`
arguments instead of the more usual `ExplodedNodeSet &Dst`
out-parameters.
Although these `NodeBuilder`s "travelled through" many functions, their
usage pattern was relatively simple and their back-and-forth set
manipulation didn't provide any advantage over a plain exploded node
set.
In addition to the removal of the `NodeBuilder`s, this commit performs
minor simplifications in the affected code and renames the old method
`BifurcateCall` to the more specific `dynDispatchBifurcate` (because the
old name was too vague now that we also have `ctuBifurcate`).
Added:
Modified:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index bb2de45cec92a..ce9b20185444e 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -772,7 +772,7 @@ class ExprEngine {
const CallEvent &Call);
/// Default implementation of call evaluation.
- void defaultEvalCall(NodeBuilder &B, ExplodedNode *Pred,
+ void defaultEvalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred,
const CallEvent &Call,
const EvalCallOptions &CallOpts = {});
@@ -908,9 +908,9 @@ class ExprEngine {
const StackFrame *SF);
void inlineCall(WorkList *WList, const CallEvent &Call, const Decl *D,
- NodeBuilder &Bldr, ExplodedNode *Pred, ProgramStateRef State);
+ ExplodedNode *Pred, ProgramStateRef State);
- void ctuBifurcate(const CallEvent &Call, const Decl *D, NodeBuilder &Bldr,
+ void ctuBifurcate(const CallEvent &Call, const Decl *D, ExplodedNodeSet &Dst,
ExplodedNode *Pred, ProgramStateRef State);
/// Returns true if the CTU analysis is running its second phase.
@@ -918,20 +918,20 @@ class ExprEngine {
/// Conservatively evaluate call by invalidating regions and binding
/// a conjured return value.
- void conservativeEvalCall(const CallEvent &Call, NodeBuilder &Bldr,
- ExplodedNode *Pred, ProgramStateRef State);
+ ExplodedNode *conservativeEvalCall(const CallEvent &Call, ExplodedNode *Pred,
+ ProgramStateRef State);
/// Either inline or process the call conservatively (or both), based
/// on DynamicDispatchBifurcation data.
- void BifurcateCall(const MemRegion *BifurReg,
- const CallEvent &Call, const Decl *D, NodeBuilder &Bldr,
- ExplodedNode *Pred);
+ void dynDispatchBifurcate(const MemRegion *BifurReg, const CallEvent &Call,
+ const Decl *D, ExplodedNodeSet &Dst,
+ ExplodedNode *Pred);
bool replayWithoutInlining(ExplodedNode *P, const StackFrame *CalleeSF);
/// Models a trivial copy or move constructor or trivial assignment operator
/// call with a simple bind.
- void performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,
+ void performTrivialCopy(ExplodedNodeSet &Dst, ExplodedNode *Pred,
const CallEvent &Call);
/// If the value of the given expression \p InitWithAdjustments is a NonLoc,
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
index 80c03899d1e39..4db6b6ecaa9f7 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -808,10 +808,8 @@ void CheckerManager::runCheckersForEvalCall(ExplodedNodeSet &Dst,
}
// If none of the checkers evaluated the call, ask ExprEngine to handle it.
- if (!evaluatorChecker) {
- NodeBuilder B(Pred, Dst, Eng.getBuilderContext());
- Eng.defaultEvalCall(B, Pred, *UpdatedCall, CallOpts);
- }
+ if (!evaluatorChecker)
+ Eng.defaultEvalCall(Dst, Pred, *UpdatedCall, CallOpts);
}
}
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 291533d0c3289..e048fc210e608 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -42,9 +42,7 @@ void ExprEngine::CreateCXXTemporaryObject(const MaterializeTemporaryExpr *ME,
Bldr.generateNode(ME, Pred, state);
}
-// FIXME: This is the sort of code that should eventually live in a Core
-// checker rather than as a special case in ExprEngine.
-void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,
+void ExprEngine::performTrivialCopy(ExplodedNodeSet &Dst, ExplodedNode *Pred,
const CallEvent &Call) {
SVal ThisVal;
bool AlwaysReturnsLValue;
@@ -67,8 +65,7 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,
const StackFrame *SF = Pred->getStackFrame();
const Expr *CallExpr = Call.getOriginExpr();
- ExplodedNodeSet Dst;
- Bldr.takeNodes(Pred);
+ ExplodedNodeSet DstEval;
assert(ThisRD);
@@ -87,23 +84,22 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,
evalLocation(Tmp, CallExpr, VExpr, Pred, Pred->getState(), V,
/*isLoad=*/true);
for (ExplodedNode *N : Tmp)
- evalBind(Dst, CallExpr, N, ThisVal, V, !AlwaysReturnsLValue);
+ evalBind(DstEval, CallExpr, N, ThisVal, V, !AlwaysReturnsLValue);
} else {
// We can't copy empty classes because of empty base class optimization.
// 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.insert(Pred);
+ DstEval.insert(Pred);
}
- PostStmt PS(CallExpr, SF);
- for (ExplodedNode *N : Dst) {
+ for (ExplodedNode *N : DstEval) {
ProgramStateRef State = N->getState();
if (AlwaysReturnsLValue)
State = State->BindExpr(CallExpr, SF, ThisVal);
else
State = bindReturnValue(Call, SF, State);
- Bldr.generateNode(PS, State, N);
+ Dst.insert(Engine.makePostStmtNode(CallExpr, State, N));
}
}
@@ -729,10 +725,9 @@ void ExprEngine::handleConstructor(const Expr *E,
if (CE && CE->getConstructor()->isTrivial() &&
CE->getConstructor()->isCopyOrMoveConstructor() &&
!CallOpts.IsArrayCtorOrDtor) {
- NodeBuilder Bldr(DstPreCall, DstEvaluated, *currBldrCtx);
// FIXME: Handle other kinds of trivial constructors as well.
for (ExplodedNode *N : DstPreCall)
- performTrivialCopy(Bldr, N, *Call);
+ performTrivialCopy(DstEvaluated, N, *Call);
} else {
for (ExplodedNode *N : DstPreCall)
@@ -862,9 +857,8 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType,
*Call, *this);
ExplodedNodeSet DstInvalidated;
- NodeBuilder Bldr(DstPreCall, DstInvalidated, *currBldrCtx);
for (ExplodedNode *N : DstPreCall)
- defaultEvalCall(Bldr, N, *Call, CallOpts);
+ defaultEvalCall(DstInvalidated, N, *Call, CallOpts);
getCheckerManager().runCheckersForPostCall(Dst, DstInvalidated,
*Call, *this);
@@ -887,7 +881,6 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE,
*Call, *this);
ExplodedNodeSet DstPostCall;
- NodeBuilder CallBldr(DstPreCall, DstPostCall, *currBldrCtx);
for (ExplodedNode *I : DstPreCall) {
// Operator new calls (CXXNewExpr) are intentionally not eval-called,
// because it does not make sense to eval-call user-provided functions.
@@ -897,7 +890,7 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE,
// is what we want anyway.
// So the best is to not allow eval-calling CXXNewExprs from checkers.
// Checkers can provide their pre/post-call callbacks if needed.
- defaultEvalCall(CallBldr, I, *Call);
+ defaultEvalCall(DstPostCall, I, *Call);
}
// If the call is inlined, DstPostCall will be empty and we bail out now.
@@ -1095,13 +1088,12 @@ void ExprEngine::VisitCXXDeleteExpr(const CXXDeleteExpr *CDE,
ExplodedNodeSet DstPostCall;
if (AMgr.getAnalyzerOptions().MayInlineCXXAllocator) {
- NodeBuilder Bldr(DstPreCall, DstPostCall, *currBldrCtx);
for (ExplodedNode *I : DstPreCall) {
// Intentionally either inline or conservative eval-call the operator
// delete, but avoid triggering an eval-call event for checkers.
// As detailed at handling CXXNewExprs, in short, because it does not
// really make sense to eval-call user-provided functions.
- defaultEvalCall(Bldr, I, *Call);
+ defaultEvalCall(DstPostCall, I, *Call);
}
} else {
DstPostCall = std::move(DstPreCall);
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 97c655c103b0a..b66b5c5e358e4 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -493,36 +493,33 @@ REGISTER_MAP_WITH_PROGRAMSTATE(DynamicDispatchBifurcationMap,
REGISTER_TRAIT_WITH_PROGRAMSTATE(CTUDispatchBifurcation, bool)
void ExprEngine::ctuBifurcate(const CallEvent &Call, const Decl *D,
- NodeBuilder &Bldr, ExplodedNode *Pred,
+ ExplodedNodeSet &Dst, ExplodedNode *Pred,
ProgramStateRef State) {
- ProgramStateRef ConservativeEvalState = nullptr;
if (Call.isForeign() && !isSecondPhaseCTU()) {
const auto IK = AMgr.options.getCTUPhase1Inlining();
const bool DoInline = IK == CTUPhase1InliningKind::All ||
(IK == CTUPhase1InliningKind::Small &&
isSmall(AMgr.getAnalysisDeclContext(D)));
if (DoInline) {
- inlineCall(Engine.getWorkList(), Call, D, Bldr, Pred, State);
+ inlineCall(Engine.getWorkList(), Call, D, Pred, State);
return;
}
const bool BState = State->get<CTUDispatchBifurcation>();
if (!BState) { // This is the first time we see this foreign function.
// Enqueue it to be analyzed in the second (ctu) phase.
- inlineCall(Engine.getCTUWorkList(), Call, D, Bldr, Pred, State);
+ inlineCall(Engine.getCTUWorkList(), Call, D, Pred, State);
// Conservatively evaluate in the first phase.
- ConservativeEvalState = State->set<CTUDispatchBifurcation>(true);
- conservativeEvalCall(Call, Bldr, Pred, ConservativeEvalState);
- } else {
- conservativeEvalCall(Call, Bldr, Pred, State);
+ State = State->set<CTUDispatchBifurcation>(true);
}
+ Dst.insert(conservativeEvalCall(Call, Pred, State));
return;
}
- inlineCall(Engine.getWorkList(), Call, D, Bldr, Pred, State);
+ inlineCall(Engine.getWorkList(), Call, D, Pred, State);
}
void ExprEngine::inlineCall(WorkList *WList, const CallEvent &Call,
- const Decl *D, NodeBuilder &Bldr,
- ExplodedNode *Pred, ProgramStateRef State) {
+ const Decl *D, ExplodedNode *Pred,
+ ProgramStateRef State) {
assert(D);
const StackFrame *CallerSF = Pred->getStackFrame();
@@ -556,10 +553,6 @@ void ExprEngine::inlineCall(WorkList *WList, const CallEvent &Call,
WList->enqueue(N);
}
- // If we decided to inline the call, the successor has been manually
- // added onto the work list so remove it from the node builder.
- Bldr.takeNodes(Pred);
-
NumInlinedCalls++;
Engine.FunctionSummaries->bumpNumTimesInlined(D);
@@ -837,14 +830,15 @@ ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call,
// Conservatively evaluate call by invalidating regions and binding
// a conjured return value.
-void ExprEngine::conservativeEvalCall(const CallEvent &Call, NodeBuilder &Bldr,
- ExplodedNode *Pred, ProgramStateRef State) {
+ExplodedNode *ExprEngine::conservativeEvalCall(const CallEvent &Call,
+ ExplodedNode *Pred,
+ ProgramStateRef State) {
State = Call.invalidateRegions(getNumVisitedCurrent(), State);
State = bindReturnValue(Call, Pred->getStackFrame(), State);
// And make the result node.
static SimpleProgramPointTag PT("ExprEngine", "Conservative eval call");
- Bldr.generateNode(Call.getProgramPoint(false, &PT), State, Pred);
+ return Engine.makeNode(Call.getProgramPoint(false, &PT), State, Pred);
}
ExprEngine::CallInlinePolicy
@@ -1219,7 +1213,7 @@ static bool isTrivialObjectAssignment(const CallEvent &Call) {
return MD->isTrivial();
}
-void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred,
+void ExprEngine::defaultEvalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred,
const CallEvent &Call,
const EvalCallOptions &CallOpts) {
// Make sure we have the most recent state attached to the call.
@@ -1227,7 +1221,7 @@ void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred,
// Special-case trivial assignment operators.
if (isTrivialObjectAssignment(Call)) {
- performTrivialCopy(Bldr, Pred, Call);
+ performTrivialCopy(Dst, Pred, Call);
return;
}
@@ -1247,17 +1241,17 @@ void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred,
// Explore with and without inlining the call.
if (Options.getIPAMode() == IPAK_DynamicDispatchBifurcate) {
- BifurcateCall(RD.getDispatchRegion(), Call, D, Bldr, Pred);
+ dynDispatchBifurcate(RD.getDispatchRegion(), Call, D, Dst, Pred);
return;
}
// Don't inline if we're not in any dynamic dispatch mode.
if (Options.getIPAMode() != IPAK_DynamicDispatch) {
- conservativeEvalCall(Call, Bldr, Pred, State);
+ Dst.insert(conservativeEvalCall(Call, Pred, State));
return;
}
}
- ctuBifurcate(Call, D, Bldr, Pred, State);
+ ctuBifurcate(Call, D, Dst, Pred, State);
return;
}
}
@@ -1268,12 +1262,13 @@ void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred,
State, dyn_cast_or_null<CXXConstructExpr>(E), Call.getStackFrame());
// Also handle the return value and invalidate the regions.
- conservativeEvalCall(Call, Bldr, Pred, State);
+ Dst.insert(conservativeEvalCall(Call, Pred, State));
}
-void ExprEngine::BifurcateCall(const MemRegion *BifurReg,
- const CallEvent &Call, const Decl *D,
- NodeBuilder &Bldr, ExplodedNode *Pred) {
+void ExprEngine::dynDispatchBifurcate(const MemRegion *BifurReg,
+ const CallEvent &Call, const Decl *D,
+ ExplodedNodeSet &Dst,
+ ExplodedNode *Pred) {
assert(BifurReg);
BifurReg = BifurReg->StripCasts();
@@ -1285,11 +1280,11 @@ void ExprEngine::BifurcateCall(const MemRegion *BifurReg,
if (BState) {
// If we are on "inline path", keep inlining if possible.
if (*BState == DynamicDispatchModeInlined)
- ctuBifurcate(Call, D, Bldr, Pred, State);
+ ctuBifurcate(Call, D, Dst, Pred, State);
// If inline failed, or we are on the path where we assume we
// don't have enough info about the receiver to inline, conjure the
// return value and invalidate the regions.
- conservativeEvalCall(Call, Bldr, Pred, State);
+ Dst.insert(conservativeEvalCall(Call, Pred, State));
return;
}
@@ -1298,12 +1293,12 @@ void ExprEngine::BifurcateCall(const MemRegion *BifurReg,
ProgramStateRef IState =
State->set<DynamicDispatchBifurcationMap>(BifurReg,
DynamicDispatchModeInlined);
- ctuBifurcate(Call, D, Bldr, Pred, IState);
+ ctuBifurcate(Call, D, Dst, Pred, IState);
ProgramStateRef NoIState =
State->set<DynamicDispatchBifurcationMap>(BifurReg,
DynamicDispatchModeConservative);
- conservativeEvalCall(Call, Bldr, Pred, NoIState);
+ Dst.insert(conservativeEvalCall(Call, Pred, NoIState));
NumOfDynamicDispatchPathSplits++;
}
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
index 15f0275691e92..26b8ca3e1f2bc 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
@@ -256,36 +256,21 @@ void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME,
// Proceed with evaluate the message expression.
ExplodedNodeSet dstEval;
- NodeBuilder Bldr(dstGenericPrevisit, dstEval, *currBldrCtx);
- for (ExplodedNodeSet::iterator DI = dstGenericPrevisit.begin(),
- DE = dstGenericPrevisit.end(); DI != DE; ++DI) {
- ExplodedNode *Pred = *DI;
+ for (ExplodedNode *Pred : dstGenericPrevisit) {
ProgramStateRef State = Pred->getState();
CallEventRef<ObjCMethodCall> UpdatedMsg = Msg.cloneWithState(State);
- if (UpdatedMsg->isInstanceMessage()) {
- SVal recVal = UpdatedMsg->getReceiverSVal();
- if (!recVal.isUndef()) {
- if (ObjCNoRet.isImplicitNoReturn(ME)) {
- // If we raise an exception, for now treat it as a sink.
- // Eventually we will want to handle exceptions properly.
- Bldr.generateSink(ME, Pred, State);
- continue;
- }
- }
- } else {
- // Check for special class methods that are known to not return
- // and that we should treat as a sink.
- if (ObjCNoRet.isImplicitNoReturn(ME)) {
- // If we raise an exception, for now treat it as a sink.
- // Eventually we will want to handle exceptions properly.
- Bldr.generateSink(ME, Pred, Pred->getState());
- continue;
- }
+ if (ObjCNoRet.isImplicitNoReturn(ME) &&
+ !(UpdatedMsg->isInstanceMessage() &&
+ UpdatedMsg->getReceiverSVal().isUndef())) {
+ // If we raise an exception, for now treat it as a sink.
+ // Eventually we will want to handle exceptions properly.
+ Engine.makePostStmtNode(ME, State, Pred, /*MarkAsSink=*/true);
+ continue;
}
- defaultEvalCall(Bldr, Pred, *UpdatedMsg);
+ defaultEvalCall(dstEval, Pred, *UpdatedMsg);
}
// If there were constructors called for object-type arguments, clean them up.
More information about the cfe-commits
mailing list