[clang] 65028f9 - [clang][bytecode] Lazily run ctor func of local blocks (#233250)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 21 05:01:06 PDT 2026
Author: Timm Baeder
Date: 2026-10-21T14:01:00+02:00
New Revision: 65028f9a57128cf0f0233601f310dc7791dd5cd6
URL: https://github.com/llvm/llvm-project/commit/65028f9a57128cf0f0233601f310dc7791dd5cd6
DIFF: https://github.com/llvm/llvm-project/commit/65028f9a57128cf0f0233601f310dc7791dd5cd6.diff
LOG: [clang][bytecode] Lazily run ctor func of local blocks (#233250)
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
Added:
Modified:
clang/lib/AST/ByteCode/Compiler.cpp
clang/lib/AST/ByteCode/Interp.h
clang/lib/AST/ByteCode/InterpFrame.cpp
clang/lib/AST/ByteCode/InterpFrame.h
clang/lib/AST/ByteCode/Opcodes.td
clang/lib/AST/ByteCode/Pointer.cpp
clang/test/AST/ByteCode/cxx20.cpp
Removed:
################################################################################
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 40b36b4e715a1..e20e54837e926 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -101,7 +101,6 @@ template <class Emitter> class VariableScope {
virtual bool emitDestructors(const Expr *E = nullptr) { return true; }
virtual bool destroyLocals(const Expr *E = nullptr) { return true; }
- virtual void forceInit() {}
VariableScope *getParent() const { return Parent; }
ScopeKind getKind() const { return Kind; }
@@ -148,29 +147,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 +3165,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
diff erentiate which branch was
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 73f2d415cf56e..f72063a310114 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 b5f621eee9b0a..5aafa0f2ceeca 100644
--- a/clang/lib/AST/ByteCode/InterpFrame.cpp
+++ b/clang/lib/AST/ByteCode/InterpFrame.cpp
@@ -47,11 +47,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.
}
}
}
@@ -80,22 +79,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
@@ -103,12 +94,14 @@ void InterpFrame::enableLocal(unsigned Idx) {
// variables, we reuse the IsActive flag for the enabled state. We should
// probably use a
diff erent 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);
}
}
@@ -233,15 +226,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 3bce6b098d764..6a093ca148005 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 = localBlock(Offset);
+ 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 {
@@ -187,6 +199,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 7cd2098514166..9e2e5e8552e8e 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 cd2d7df17a532..3890f2ed2f91d 100644
--- a/clang/lib/AST/ByteCode/Pointer.cpp
+++ b/clang/lib/AST/ByteCode/Pointer.cpp
@@ -806,12 +806,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;
@@ -839,12 +833,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;
diff --git a/clang/test/AST/ByteCode/cxx20.cpp b/clang/test/AST/ByteCode/cxx20.cpp
index d9200d18898d1..5ef748e162a23 100644
--- a/clang/test/AST/ByteCode/cxx20.cpp
+++ b/clang/test/AST/ByteCode/cxx20.cpp
@@ -1577,3 +1577,24 @@ namespace DiscardedConstevalStruct {
return (make(), 0); // both-warning {{left operand of comma operator has no effect}}
}
}
+
+namespace ConditionalOperatorInBORHS {
+ struct S { int x; };
+
+ struct a {
+ int c = 12;
+
+ constexpr int e() {
+
+ 0 && (0 ? 0 : 0); // both-warning {{use of logical '&&' with constant operand}} \
+ // both-note {{use '&' for a bitwise operation}} \
+ // both-note {{remove constant to silence this warning}} \
+ // both-warning {{expression result unused}}
+
+ int f = c;
+
+ return (f);
+ }
+ };
+ static_assert(a{}.e() == 12);
+}
More information about the cfe-commits
mailing list