[clang] [NFC][analyzer] Improve computeObjectUnderConstruction (PR #186186)
Donát Nagy via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 12 10:09:31 PDT 2026
https://github.com/NagyDonat created https://github.com/llvm/llvm-project/pull/186186
Previously the method `ExprEngine::computeObjectUnderConstruction` took a `NodeBuilderContext` parameter which was only used to call its `blockCount()` method; this commit replaces this with directly taking `NumVisitedCaller` (= number of times the caller was visited, the `blockCount`) as an `unsigned` value.
Also leave a FIXME comment on some logic that seems to be fishy (I _suspect_ that sometimes it uses the wrong `LocationContext` and `Block`, but I'm not certain.)
>From 22f9ce383d8468bf28c8e29579123db534c6e00a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Tue, 10 Mar 2026 11:39:16 +0100
Subject: [PATCH 1/2] [NFC][analyzer] Simplify computeObjectUnderConstruction
part 1
Previously the method `ExprEngine::computeObjectUnderConstruction` took
a `NodeBuilderContext` parameter which was only used to call its
`blockCount()` method; this commit replaces this with directly taking
`NumVisitedCaller` (= number of times the caller was visited, the
`blockCount`) as an `unsigned` value.
This moves toward the elimination of the type `NodeBuilderContext`.
Note that I'm using the name `NumVisitedCaller` instead of e.g.
`CallerBlockCount` because I think this is more self-documenting.
This commit leaves behind some non-idiomatic constructs which will be
cleaned up in an immediate follow-up.
---
.../StaticAnalyzer/Core/PathSensitive/ExprEngine.h | 6 +++---
clang/lib/StaticAnalyzer/Core/CallEvent.cpp | 2 +-
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 11 ++++++-----
3 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 2023a7a5b1ac8..360a260d4d803 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -796,7 +796,7 @@ class ExprEngine {
/// A multi-dimensional array is also a continuous memory location in a
/// row major order, so for arr[0][0] Idx is 0 and for arr[3][3] Idx is 8.
SVal computeObjectUnderConstruction(const Expr *E, ProgramStateRef State,
- const NodeBuilderContext *BldrCtx,
+ unsigned NumVisitedCaller,
const LocationContext *LCtx,
const ConstructionContext *CC,
EvalCallOptions &CallOpts,
@@ -818,8 +818,8 @@ class ExprEngine {
const LocationContext *LCtx, const ConstructionContext *CC,
EvalCallOptions &CallOpts, unsigned Idx = 0) {
- SVal V = computeObjectUnderConstruction(E, State, BldrCtx, LCtx, CC,
- CallOpts, Idx);
+ SVal V = computeObjectUnderConstruction(E, State, BldrCtx->blockCount(),
+ LCtx, CC, CallOpts, Idx);
State = updateObjectsUnderConstruction(V, E, State, LCtx, CC, CallOpts);
return std::make_pair(State, V);
diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
index f6c3a8e3266da..cff3116caf6e6 100644
--- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -563,7 +563,7 @@ std::optional<SVal> CallEvent::getReturnValueUnderConstruction() const {
EvalCallOptions CallOpts;
ExprEngine &Engine = getState()->getStateManager().getOwningEngine();
SVal RetVal = Engine.computeObjectUnderConstruction(
- getOriginExpr(), getState(), &Engine.getBuilderContext(),
+ getOriginExpr(), getState(), Engine.getBuilderContext().blockCount(),
getLocationContext(), CC, CallOpts);
return RetVal;
}
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 0866dda766667..c91b5ddd0d46e 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -129,7 +129,7 @@ SVal ExprEngine::makeElementRegion(ProgramStateRef State, SVal LValue,
// it's materialization happens in context of the caller.
// We pass BldrCtx explicitly, as currBldrCtx always refers to callee's context.
SVal ExprEngine::computeObjectUnderConstruction(
- const Expr *E, ProgramStateRef State, const NodeBuilderContext *BldrCtx,
+ const Expr *E, ProgramStateRef State, unsigned NumVisitedCaller,
const LocationContext *LCtx, const ConstructionContext *CC,
EvalCallOptions &CallOpts, unsigned Idx) {
@@ -232,8 +232,9 @@ SVal ExprEngine::computeObjectUnderConstruction(
NodeBuilderContext CallerBldrCtx(getCoreEngine(),
SFC->getCallSiteBlock(), CallerLCtx);
+ unsigned NumVisitedCaller = CallerBldrCtx.blockCount();
return computeObjectUnderConstruction(
- cast<Expr>(SFC->getCallSite()), State, &CallerBldrCtx, CallerLCtx,
+ cast<Expr>(SFC->getCallSite()), State, NumVisitedCaller, CallerLCtx,
RTC->getConstructionContext(), CallOpts);
} else {
// We are on the top frame of the analysis. We do not know where is the
@@ -273,7 +274,7 @@ SVal ExprEngine::computeObjectUnderConstruction(
EvalCallOptions PreElideCallOpts = CallOpts;
SVal V = computeObjectUnderConstruction(
- TCC->getConstructorAfterElision(), State, BldrCtx, LCtx,
+ TCC->getConstructorAfterElision(), State, NumVisitedCaller, LCtx,
TCC->getConstructionContextAfterElision(), CallOpts);
// FIXME: This definition of "copy elision has not failed" is unreliable.
@@ -346,7 +347,7 @@ SVal ExprEngine::computeObjectUnderConstruction(
CallEventManager &CEMgr = getStateManager().getCallEventManager();
auto getArgLoc = [&](CallEventRef<> Caller) -> std::optional<SVal> {
const LocationContext *FutureSFC =
- Caller->getCalleeStackFrame(BldrCtx->blockCount());
+ Caller->getCalleeStackFrame(NumVisitedCaller);
// Return early if we are unable to reliably foresee
// the future stack frame.
if (!FutureSFC)
@@ -365,7 +366,7 @@ SVal ExprEngine::computeObjectUnderConstruction(
// because this-argument is implemented as a normal argument in
// operator call expressions but not in operator declarations.
const TypedValueRegion *TVR = Caller->getParameterLocation(
- *Caller->getAdjustedParameterIndex(Idx), BldrCtx->blockCount());
+ *Caller->getAdjustedParameterIndex(Idx), NumVisitedCaller);
if (!TVR)
return std::nullopt;
>From a80473647ab61fb3d4a1a60c91441652c72df4d5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Tue, 10 Mar 2026 14:22:43 +0100
Subject: [PATCH 2/2] [NFC][analyzer] Simplify computeObjectUnderConstruction
part 2
Clean-up non-idiomatic code left behind by the previous change.
Also leave a FIXME comment on some logic that seems to be fishy (I
_suspect_ that sometimes it uses the wrong LocationContext and Block,
but I'm not certain.)
---
clang/lib/StaticAnalyzer/Core/CallEvent.cpp | 6 +++++-
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 7 ++-----
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
index cff3116caf6e6..86ffd92cdf6f5 100644
--- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -562,8 +562,12 @@ std::optional<SVal> CallEvent::getReturnValueUnderConstruction() const {
EvalCallOptions CallOpts;
ExprEngine &Engine = getState()->getStateManager().getOwningEngine();
+ // FIXME: This code assumes that the _current_ location context and block is
+ // the location and block where this `CallExpr` is called. For a more stable
+ // solution `Engine.getNumVisitedCurrent()` should be replaced with a call to
+ // `Engine.getNumVisited(<CallerLCtx>, <CallerBlock>)`.
SVal RetVal = Engine.computeObjectUnderConstruction(
- getOriginExpr(), getState(), Engine.getBuilderContext().blockCount(),
+ getOriginExpr(), getState(), Engine.getNumVisitedCurrent(),
getLocationContext(), CC, CallOpts);
return RetVal;
}
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index c91b5ddd0d46e..cf22b62225f2f 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -127,7 +127,6 @@ SVal ExprEngine::makeElementRegion(ProgramStateRef State, SVal LValue,
// In case when the prvalue is returned from the function (kind is one of
// SimpleReturnedValueKind, CXX17ElidedCopyReturnedValueKind), then
// it's materialization happens in context of the caller.
-// We pass BldrCtx explicitly, as currBldrCtx always refers to callee's context.
SVal ExprEngine::computeObjectUnderConstruction(
const Expr *E, ProgramStateRef State, unsigned NumVisitedCaller,
const LocationContext *LCtx, const ConstructionContext *CC,
@@ -230,11 +229,9 @@ SVal ExprEngine::computeObjectUnderConstruction(
assert(!isa<BlockInvocationContext>(CallerLCtx));
}
- NodeBuilderContext CallerBldrCtx(getCoreEngine(),
- SFC->getCallSiteBlock(), CallerLCtx);
- unsigned NumVisitedCaller = CallerBldrCtx.blockCount();
+ unsigned NVCaller = getNumVisited(CallerLCtx, SFC->getCallSiteBlock());
return computeObjectUnderConstruction(
- cast<Expr>(SFC->getCallSite()), State, NumVisitedCaller, CallerLCtx,
+ cast<Expr>(SFC->getCallSite()), State, NVCaller, CallerLCtx,
RTC->getConstructionContext(), CallOpts);
} else {
// We are on the top frame of the analysis. We do not know where is the
More information about the cfe-commits
mailing list