https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/169030
>From 480df19e28fcd631528d79802d5e23f5c788e05d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <[email protected]> Date: Fri, 21 Nov 2025 11:15:12 +0100 Subject: [PATCH] local activate --- clang/lib/AST/ByteCode/ByteCodeEmitter.h | 2 +- clang/lib/AST/ByteCode/Compiler.cpp | 34 +++++++++++++++------ clang/lib/AST/ByteCode/Compiler.h | 39 +++++++++++++++++++----- clang/lib/AST/ByteCode/EvalEmitter.cpp | 29 +++++++++++++++++- clang/lib/AST/ByteCode/Function.h | 2 ++ clang/lib/AST/ByteCode/Interp.h | 12 ++++++++ clang/lib/AST/ByteCode/InterpFrame.cpp | 12 ++++++++ clang/lib/AST/ByteCode/InterpFrame.h | 4 +++ clang/lib/AST/ByteCode/Opcodes.td | 10 ++++++ clang/test/AST/ByteCode/cxx23.cpp | 23 ++++++++++++++ 10 files changed, 148 insertions(+), 19 deletions(-) diff --git a/clang/lib/AST/ByteCode/ByteCodeEmitter.h b/clang/lib/AST/ByteCode/ByteCodeEmitter.h index ca8dc38e65246..dd18341d52a09 100644 --- a/clang/lib/AST/ByteCode/ByteCodeEmitter.h +++ b/clang/lib/AST/ByteCode/ByteCodeEmitter.h @@ -25,11 +25,11 @@ enum Opcode : uint32_t; /// An emitter which links the program to bytecode for later use. class ByteCodeEmitter { protected: - using LabelTy = uint32_t; using AddrTy = uintptr_t; using Local = Scope::Local; public: + using LabelTy = uint32_t; /// Compiles the function into the module. void compileFunc(const FunctionDecl *FuncDecl, Function *Func = nullptr); diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index 8f8f3d6a05820..e221d71ea9f2f 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -16,6 +16,7 @@ #include "PrimType.h" #include "Program.h" #include "clang/AST/Attr.h" +#include "llvm/Support/SaveAndRestore.h" using namespace clang; using namespace clang::interp; @@ -2500,17 +2501,18 @@ bool Compiler<Emitter>::VisitAbstractConditionalOperator( const Expr *TrueExpr = E->getTrueExpr(); const Expr *FalseExpr = E->getFalseExpr(); - auto visitChildExpr = [&](const Expr *E) -> bool { - LocalScope<Emitter> S(this); - if (!this->delegate(E)) - return false; - return S.destroyLocals(); - }; + // 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 + // taken and only destroy the varibles of the active branch. This is what the + // "enabled" flags on local variables are used for. + llvm::SaveAndRestore LAAA(this->VarScope->LocalsAlwaysEnabled, + /*NewValue=*/false); if (std::optional<bool> BoolValue = getBoolValue(Condition)) { if (*BoolValue) - return visitChildExpr(TrueExpr); - return visitChildExpr(FalseExpr); + return this->delegate(TrueExpr); + return this->delegate(FalseExpr); } bool IsBcpCall = false; @@ -2542,13 +2544,15 @@ bool Compiler<Emitter>::VisitAbstractConditionalOperator( if (!this->jumpFalse(LabelFalse)) return false; - if (!visitChildExpr(TrueExpr)) + if (!this->delegate(TrueExpr)) return false; + if (!this->jump(LabelEnd)) return false; this->emitLabel(LabelFalse); - if (!visitChildExpr(FalseExpr)) + if (!this->delegate(FalseExpr)) return false; + this->fallthrough(LabelEnd); this->emitLabel(LabelEnd); @@ -2955,10 +2959,15 @@ bool Compiler<Emitter>::VisitMaterializeTemporaryExpr( bool IsVolatile = SubExpr->getType().isVolatileQualified(); unsigned LocalIndex = allocateLocalPrimitive( E, *SubExprT, IsConst, IsVolatile, E->getExtendingDecl()); + if (!this->VarScope->LocalsAlwaysEnabled && + !this->emitEnableLocal(LocalIndex, E)) + return false; + if (!this->visit(SubExpr)) return false; if (!this->emitSetLocal(*SubExprT, LocalIndex, E)) return false; + return this->emitGetPtrLocal(LocalIndex, E); } @@ -2968,6 +2977,11 @@ bool Compiler<Emitter>::VisitMaterializeTemporaryExpr( if (UnsignedOrNone LocalIndex = allocateLocal(E, Inner->getType(), E->getExtendingDecl())) { InitLinkScope<Emitter> ILS(this, InitLink::Temp(*LocalIndex)); + + if (!this->VarScope->LocalsAlwaysEnabled && + !this->emitEnableLocal(*LocalIndex, E)) + return false; + if (!this->emitGetPtrLocal(*LocalIndex, E)) return false; return this->visitInitializer(SubExpr) && this->emitFinishInit(E); diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h index 0c6cab9276531..2da06ef7793ed 100644 --- a/clang/lib/AST/ByteCode/Compiler.h +++ b/clang/lib/AST/ByteCode/Compiler.h @@ -477,12 +477,14 @@ template <class Emitter> class VariableScope { VariableScope(Compiler<Emitter> *Ctx, const ValueDecl *VD, ScopeKind Kind = ScopeKind::Block) : Ctx(Ctx), Parent(Ctx->VarScope), ValDecl(VD), Kind(Kind) { + if (Parent) + this->LocalsAlwaysEnabled = Parent->LocalsAlwaysEnabled; Ctx->VarScope = this; } virtual ~VariableScope() { Ctx->VarScope = this->Parent; } - virtual void addLocal(const Scope::Local &Local) { + virtual void addLocal(Scope::Local Local) { llvm_unreachable("Shouldn't be called"); } @@ -519,7 +521,6 @@ template <class Emitter> class VariableScope { if (!P) break; } - // Add to this scope. this->addLocal(Local); } @@ -530,6 +531,11 @@ template <class Emitter> class VariableScope { VariableScope *getParent() const { return Parent; } ScopeKind getKind() const { return Kind; } + /// Whether locals added to this scope are enabled by default. + /// This is almost always true, except for the two branches + /// of a conditional operator. + bool LocalsAlwaysEnabled = true; + protected: /// Compiler instance. Compiler<Emitter> *Ctx; @@ -576,29 +582,48 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> { return Success; } - void addLocal(const Scope::Local &Local) override { + void addLocal(Scope::Local Local) override { if (!Idx) { Idx = static_cast<unsigned>(this->Ctx->Descriptors.size()); this->Ctx->Descriptors.emplace_back(); this->Ctx->emitInitScope(*Idx, {}); } + Local.EnabledByDefault = this->LocalsAlwaysEnabled; this->Ctx->Descriptors[*Idx].emplace_back(Local); } bool emitDestructors(const Expr *E = nullptr) override { if (!Idx) return true; + assert(!this->Ctx->Descriptors[*Idx].empty()); + // Emit destructor calls for local variables of record // type with a destructor. for (Scope::Local &Local : llvm::reverse(this->Ctx->Descriptors[*Idx])) { if (Local.Desc->hasTrivialDtor()) continue; - if (!this->Ctx->emitGetPtrLocal(Local.Offset, E)) - return false; - if (!this->Ctx->emitDestructionPop(Local.Desc, Local.Desc->getLoc())) - return false; + if (!Local.EnabledByDefault) { + typename Emitter::LabelTy EndLabel = this->Ctx->getLabel(); + if (!this->Ctx->emitGetLocalEnabled(Local.Offset, E)) + return false; + if (!this->Ctx->jumpFalse(EndLabel)) + return false; + + if (!this->Ctx->emitGetPtrLocal(Local.Offset, E)) + return false; + + if (!this->Ctx->emitDestructionPop(Local.Desc, Local.Desc->getLoc())) + return false; + + this->Ctx->emitLabel(EndLabel); + } else { + if (!this->Ctx->emitGetPtrLocal(Local.Offset, E)) + return false; + if (!this->Ctx->emitDestructionPop(Local.Desc, Local.Desc->getLoc())) + return false; + } removeIfStoredOpaqueValue(Local); } diff --git a/clang/lib/AST/ByteCode/EvalEmitter.cpp b/clang/lib/AST/ByteCode/EvalEmitter.cpp index 007321791fdd4..a2e01efc8ffd9 100644 --- a/clang/lib/AST/ByteCode/EvalEmitter.cpp +++ b/clang/lib/AST/ByteCode/EvalEmitter.cpp @@ -113,7 +113,7 @@ Scope::Local EvalEmitter::createLocal(Descriptor *D) { InlineDescriptor &Desc = *reinterpret_cast<InlineDescriptor *>(B->rawData()); Desc.Desc = D; Desc.Offset = sizeof(InlineDescriptor); - Desc.IsActive = true; + Desc.IsActive = false; Desc.IsBase = false; Desc.IsFieldMutable = false; Desc.IsConst = false; @@ -322,6 +322,33 @@ bool EvalEmitter::emitDestroy(uint32_t I, SourceInfo Info) { return true; } +bool EvalEmitter::emitGetLocalEnabled(uint32_t I, SourceInfo Info) { + if (!isActive()) + return true; + + Block *B = getLocal(I); + const InlineDescriptor &Desc = + *reinterpret_cast<InlineDescriptor *>(B->rawData()); + + S.Stk.push<bool>(Desc.IsActive); + return true; +} + +bool EvalEmitter::emitEnableLocal(uint32_t I, SourceInfo Info) { + if (!isActive()) + return true; + + // FIXME: This is a little dirty, but to avoid adding a flag to + // InlineDescriptor that's only ever useful on the toplevel of local + // 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. + Block *B = getLocal(I); + InlineDescriptor &Desc = *reinterpret_cast<InlineDescriptor *>(B->rawData()); + Desc.IsActive = true; + return true; +} + /// Global temporaries (LifetimeExtendedTemporary) carry their value /// around as an APValue, which codegen accesses. /// We set their value once when creating them, but we don't update it diff --git a/clang/lib/AST/ByteCode/Function.h b/clang/lib/AST/ByteCode/Function.h index 95add5809afcc..8c309c921afa9 100644 --- a/clang/lib/AST/ByteCode/Function.h +++ b/clang/lib/AST/ByteCode/Function.h @@ -41,6 +41,8 @@ class Scope final { unsigned Offset; /// Descriptor of the local. Descriptor *Desc; + /// If the cleanup for this local should be emitted. + bool EnabledByDefault = true; }; using LocalVectorTy = llvm::SmallVector<Local, 8>; diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index 3e869c1ee5f2c..86b1ba88ca9d4 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -2474,6 +2474,18 @@ inline bool InitScope(InterpState &S, CodePtr OpPC, uint32_t I) { return true; } +inline bool EnableLocal(InterpState &S, CodePtr OpPC, uint32_t I) { + assert(!S.Current->isLocalEnabled(I)); + S.Current->enableLocal(I); + return true; +} + +inline bool GetLocalEnabled(InterpState &S, CodePtr OpPC, uint32_t I) { + assert(S.Current); + S.Stk.push<bool>(S.Current->isLocalEnabled(I)); + return true; +} + //===----------------------------------------------------------------------===// // Cast, CastFP //===----------------------------------------------------------------------===// diff --git a/clang/lib/AST/ByteCode/InterpFrame.cpp b/clang/lib/AST/ByteCode/InterpFrame.cpp index 039acb5d72b2c..3b883761ad001 100644 --- a/clang/lib/AST/ByteCode/InterpFrame.cpp +++ b/clang/lib/AST/ByteCode/InterpFrame.cpp @@ -89,11 +89,23 @@ void InterpFrame::destroyScopes() { void InterpFrame::initScope(unsigned Idx) { if (!Func) return; + for (auto &Local : Func->getScope(Idx).locals()) { localBlock(Local.Offset)->invokeCtor(); } } +void InterpFrame::enableLocal(unsigned Idx) { + assert(Func); + + // FIXME: This is a little dirty, but to avoid adding a flag to + // InlineDescriptor that's only ever useful on the toplevel of local + // 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; +} + void InterpFrame::destroy(unsigned Idx) { for (auto &Local : Func->getScope(Idx).locals_reverse()) { S.deallocate(localBlock(Local.Offset)); diff --git a/clang/lib/AST/ByteCode/InterpFrame.h b/clang/lib/AST/ByteCode/InterpFrame.h index fa9de2e1e7c6d..8f563c8b9348c 100644 --- a/clang/lib/AST/ByteCode/InterpFrame.h +++ b/clang/lib/AST/ByteCode/InterpFrame.h @@ -55,6 +55,10 @@ class InterpFrame final : public Frame { void destroy(unsigned Idx); void initScope(unsigned Idx); void destroyScopes(); + void enableLocal(unsigned Idx); + bool isLocalEnabled(unsigned Idx) const { + return localInlineDesc(Idx)->IsActive; + } /// Describes the frame with arguments for diagnostic purposes. void describe(llvm::raw_ostream &OS) const override; diff --git a/clang/lib/AST/ByteCode/Opcodes.td b/clang/lib/AST/ByteCode/Opcodes.td index a236f89dcf78b..6e768793fcfcf 100644 --- a/clang/lib/AST/ByteCode/Opcodes.td +++ b/clang/lib/AST/ByteCode/Opcodes.td @@ -251,6 +251,16 @@ def InitScope : Opcode { let Args = [ArgUint32]; } +def GetLocalEnabled : Opcode { + let Args = [ArgUint32]; + let HasCustomEval = 1; +} + +def EnableLocal : Opcode { + let Args = [ArgUint32]; + let HasCustomEval = 1; +} + //===----------------------------------------------------------------------===// // Constants //===----------------------------------------------------------------------===// diff --git a/clang/test/AST/ByteCode/cxx23.cpp b/clang/test/AST/ByteCode/cxx23.cpp index c5d26925ce11b..819460628c1b7 100644 --- a/clang/test/AST/ByteCode/cxx23.cpp +++ b/clang/test/AST/ByteCode/cxx23.cpp @@ -473,3 +473,26 @@ namespace AIEWithIndex0Narrows { } static_assert(test()); } + +#if __cplusplus >= 202302L +namespace InactiveLocalsInConditionalOp { + struct A { constexpr A(){}; ~A(); constexpr int get() { return 10; } }; // all-note 2{{declared here}} + constexpr int get(bool b) { + return b ? A().get() : 1; // all-note {{non-constexpr function '~A' cannot be used in a constant expression}} + } + static_assert(get(false) == 1, ""); + static_assert(get(true) == 10, ""); // all-error {{not an integral constant expression}} \ + // all-note {{in call to}} + + static_assert( (false ? A().get() : 1) == 1); + static_assert( (true ? A().get() : 1) == 1); // all-error {{not an integral constant expression}} \ + // all-note {{non-constexpr function '~A' cannot be used in a constant expression}} + + constexpr bool test2(bool b) { + unsigned long __ms = b ? (const unsigned long &)0 : __ms; + return true; + } + static_assert(test2(true)); + +} +#endif _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
