[clang] [clang][bytecode] Lazily run ctor func of local blocks (PR #233250)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Oct 18 08:50:53 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Timm Baeder (tbaederr)
<details>
<summary>Changes</summary>
The problem with the attached test case is that a conditional operator will force initialize the current variable scope, so we know that the scope is initialized in both arms of the conditional operator. However, this causes problems if the conditional operator (and thus its force initialization of the scope) is itself not always executed, e.g. if preceded by `0 &&`. In that case , the binary operator will skip the RHS.
Remove the `InitScope` op altogether and simply call `Block::invokeCtor()` when accessing a local variable for the first time. This way, we automatically run the ctor, even if the `InitScope` wasn't reached (if it still existed anyway).
This also allows us to remove a similar workaround from `Pointer.cpp`.
Fixes https://github.com/llvm/llvm-project/issues/232854
---
Full diff: https://github.com/llvm/llvm-project/pull/233250.diff
6 Files Affected:
- (modified) clang/lib/AST/ByteCode/Compiler.cpp (-20)
- (modified) clang/lib/AST/ByteCode/Interp.h (-5)
- (modified) clang/lib/AST/ByteCode/InterpFrame.cpp (+11-27)
- (modified) clang/lib/AST/ByteCode/InterpFrame.h (+22-9)
- (modified) clang/lib/AST/ByteCode/Opcodes.td (-3)
- (modified) clang/lib/AST/ByteCode/Pointer.cpp (-12)
``````````diff
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 40b36b4e715a10..66234608666de5 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -148,29 +148,12 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> {
if (!Idx) {
Idx = static_cast<unsigned>(this->Ctx->Descriptors.size());
this->Ctx->Descriptors.emplace_back();
-
- if constexpr (!std::is_same_v<Emitter, EvalEmitter>)
- this->Ctx->emitInitScope(*Idx, {});
}
Local.EnabledByDefault = this->LocalsAlwaysEnabled;
this->Ctx->Descriptors[*Idx].emplace_back(Local);
}
- /// Force-initialize this scope. Usually, scopes are lazily initialized when
- /// the first local variable is created, but in scenarios with conditonal
- /// operators, we need to ensure scope is initialized just in case one of the
- /// arms will create a local and the other won't. In such a case, the
- /// InitScope() op would be part of the arm that created the local.
- void forceInit() override {
- if (!Idx) {
- Idx = static_cast<unsigned>(this->Ctx->Descriptors.size());
- this->Ctx->Descriptors.emplace_back();
- if constexpr (!std::is_same_v<Emitter, EvalEmitter>)
- this->Ctx->emitInitScope(*Idx, {});
- }
- }
-
bool emitDestructors(const Expr *E = nullptr) override {
if (!Idx)
return true;
@@ -3183,9 +3166,6 @@ bool Compiler<Emitter>::VisitAbstractConditionalOperator(
return false;
}
- // Force-init the scope, which creates a InitScope op. This is necessary so
- // the scope is not only initialized in one arm of the conditional operator.
- this->VarScope->forceInit();
// The TrueExpr and FalseExpr of a conditional operator do _not_ create a
// scope, which means the local variables created within them unconditionally
// always exist. However, we need to later differentiate which branch was
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 73f2d415cf56e6..f72063a3101144 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -2854,11 +2854,6 @@ inline bool SubPtr(InterpState &S, CodePtr OpPC, uint32_t ElemSize) {
return true;
}
-inline bool InitScope(InterpState &S, uint32_t I) {
- S.Current->initScope(I);
- return true;
-}
-
inline bool EnableLocal(InterpState &S, uint32_t I) {
assert(!S.Current->isLocalEnabled(I));
S.Current->enableLocal(I);
diff --git a/clang/lib/AST/ByteCode/InterpFrame.cpp b/clang/lib/AST/ByteCode/InterpFrame.cpp
index c04a70cc49f7c7..e1a3bc96efdffb 100644
--- a/clang/lib/AST/ByteCode/InterpFrame.cpp
+++ b/clang/lib/AST/ByteCode/InterpFrame.cpp
@@ -48,11 +48,10 @@ InterpFrame::InterpFrame(InterpState &S, const Function *Func,
for (auto &Scope : Func->scopes()) {
for (auto &Local : Scope.locals()) {
- new (localBlock(Local.Offset))
- Block(S.EvalID, Local.Desc, Block::InlineDescMD);
+ Block *B = localBlock(Local.Offset);
+ new (B) Block(S.EvalID, Local.Desc, Block::InlineDescMD);
// Note that we are NOT calling invokeCtor() here, since that is done
- // via the InitScope op.
- new (localInlineDesc(Local.Offset)) InlineDescriptor(Local.Desc);
+ // lazily when accessing the local.
}
}
}
@@ -81,22 +80,14 @@ InterpFrame::~InterpFrame() {
// This happens when the execution is interruped midway-through.
for (auto &Scope : Func->scopes()) {
for (auto &Local : Scope.locals()) {
- S.deallocate(localBlock(Local.Offset));
+ Block *B = localBlock(Local.Offset);
+ if (B->isInitialized())
+ S.deallocate(B);
}
}
}
-void InterpFrame::initScope(unsigned Idx) {
- if (!Func)
- return;
-
- for (auto &Local : Func->getScope(Idx).locals()) {
- assert(!localBlock(Local.Offset)->isInitialized());
- localBlock(Local.Offset)->invokeCtor();
- }
-}
-
-void InterpFrame::enableLocal(unsigned Idx) {
+void InterpFrame::enableLocal(unsigned Offset) {
assert(Func);
// FIXME: This is a little dirty, but to avoid adding a flag to
@@ -104,12 +95,14 @@ void InterpFrame::enableLocal(unsigned Idx) {
// variables, we reuse the IsActive flag for the enabled state. We should
// probably use a different struct than InlineDescriptor for the block-level
// inline descriptor of local varaibles.
- localInlineDesc(Idx)->IsActive = true;
+ getLocalBlock(Offset)->getBlockDesc<InlineDescriptor>().IsActive = true;
}
void InterpFrame::destroy(unsigned Idx) {
for (auto &Local : Func->getScope(Idx).locals_reverse()) {
- S.deallocate(localBlock(Local.Offset));
+ Block *B = localBlock(Local.Offset);
+ if (B->isInitialized())
+ S.deallocate(B);
}
}
@@ -234,15 +227,6 @@ const FunctionDecl *InterpFrame::getCallee() const {
return Func->getDecl();
}
-Pointer InterpFrame::getLocalPointer(unsigned Offset) const {
- assert(Offset < Func->getFrameSize() && "Invalid local offset.");
- return Pointer(localBlock(Offset));
-}
-
-Block *InterpFrame::getLocalBlock(unsigned Offset) const {
- return localBlock(Offset);
-}
-
Pointer InterpFrame::getParamPointer(unsigned Index) {
assert(!isBottomFrame());
diff --git a/clang/lib/AST/ByteCode/InterpFrame.h b/clang/lib/AST/ByteCode/InterpFrame.h
index acc9c337bfb1b2..5e03d58790ed73 100644
--- a/clang/lib/AST/ByteCode/InterpFrame.h
+++ b/clang/lib/AST/ByteCode/InterpFrame.h
@@ -58,10 +58,9 @@ class InterpFrame final : public Frame {
/// Invokes the destructors for a scope.
void destroy(unsigned Idx);
- void initScope(unsigned Idx);
- void enableLocal(unsigned Idx);
- bool isLocalEnabled(unsigned Idx) const {
- return localInlineDesc(Idx)->IsActive;
+ void enableLocal(unsigned Offset);
+ bool isLocalEnabled(unsigned Offset) const {
+ return getLocalBlock(Offset)->getBlockDesc<InlineDescriptor>().IsActive;
}
/// Describes the frame with arguments for diagnostic purposes.
@@ -91,14 +90,27 @@ class InterpFrame final : public Frame {
/// Mutates a local variable.
template <typename T> void setLocal(unsigned Offset, const T &Value) {
- localRef<T>(Offset) = Value;
- localInlineDesc(Offset)->IsInitialized = true;
- localInlineDesc(Offset)->LifeState = Lifetime::Started;
+ Block *B = getLocalBlock(Offset);
+
+ B->deref<T>() = Value;
+ auto &LID = B->getBlockDesc<InlineDescriptor>();
+ LID.IsInitialized = true;
+ LID.LifeState = Lifetime::Started;
}
/// Returns a pointer to a local variables.
- Pointer getLocalPointer(unsigned Offset) const;
- Block *getLocalBlock(unsigned Offset) const;
+ Pointer getLocalPointer(unsigned Offset) const {
+ assert(Offset < Func->getFrameSize() && "Invalid local offset.");
+ return Pointer(getLocalBlock(Offset));
+ }
+ Block *getLocalBlock(unsigned Offset) const {
+ Block *B = reinterpret_cast<Block *>(locals() + Offset - sizeof(Block));
+ if (LLVM_UNLIKELY(!B->isInitialized())) {
+ B->invokeCtor();
+ new (localInlineDesc(Offset)) InlineDescriptor(B->getDescriptor());
+ }
+ return B;
+ };
/// Returns the value of an argument.
template <typename T> const T &getParam(unsigned Index) const {
@@ -188,6 +200,7 @@ class InterpFrame final : public Frame {
}
/// Returns a pointer to a local's block.
+ /// Will NOT initialize the block.
Block *localBlock(unsigned Offset) const {
return reinterpret_cast<Block *>(locals() + Offset - sizeof(Block));
}
diff --git a/clang/lib/AST/ByteCode/Opcodes.td b/clang/lib/AST/ByteCode/Opcodes.td
index 7cd20985141668..9e2e5e8552e8e9 100644
--- a/clang/lib/AST/ByteCode/Opcodes.td
+++ b/clang/lib/AST/ByteCode/Opcodes.td
@@ -267,9 +267,6 @@ def Destroy : Opcode {
let Args = [ArgUint32];
let HasCustomEval = 1;
}
-def InitScope : SuccessOpcode {
- let Args = [ArgUint32];
-}
def GetLocalEnabled : SuccessOpcode {
let Args = [ArgUint32];
diff --git a/clang/lib/AST/ByteCode/Pointer.cpp b/clang/lib/AST/ByteCode/Pointer.cpp
index 839a0929d18525..cc7199236e8e65 100644
--- a/clang/lib/AST/ByteCode/Pointer.cpp
+++ b/clang/lib/AST/ByteCode/Pointer.cpp
@@ -807,12 +807,6 @@ void PtrView::setLifeState(Lifetime L) const {
}
void PtrView::initialize() const {
- // FIXME: This happens when the control flow jumps right into a scope, e.g. in
- // switch_into_init_stmt in constant-expression-cxx2a.cpp. I.e. we have never
- // initialized the scope via an InitScope op.
- if (LLVM_UNLIKELY(!Pointee->isInitialized()))
- Pointee->invokeCtor();
-
if (isRoot() && Base == sizeof(GlobalInlineDescriptor) && Offset == Base) {
auto &GD = Pointee->getBlockDesc<GlobalInlineDescriptor>();
GD.InitState = GlobalInitState::Initialized;
@@ -840,12 +834,6 @@ void PtrView::initializeElement(unsigned Index) const {
assert(Index < getFieldDesc()->getNumElems());
- // FIXME: This happens when the control flow jumps right into a scope, e.g. in
- // switch_into_init_stmt in constant-expression-cxx2a.cpp. I.e. we have never
- // initialized the scope via an InitScope op.
- if (LLVM_UNLIKELY(!Pointee->isInitialized()))
- Pointee->invokeCtor();
-
InitMapPtr &IM = getInitMap();
if (IM.allInitialized())
return;
``````````
</details>
https://github.com/llvm/llvm-project/pull/233250
More information about the cfe-commits
mailing list