Timm =?utf-8?q?Bäder?= <tbae...@redhat.com> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/149...@github.com>
https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/149963 >From 7fa404e88e104eea07f26f4959cf4d6e1768dc14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Mon, 21 Jul 2025 16:56:27 +0200 Subject: [PATCH 1/2] [clang][bytecode] Unions again --- clang/lib/AST/ByteCode/Compiler.cpp | 9 ++++- clang/test/AST/ByteCode/unions.cpp | 55 +++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index 07efd6f852fc2..e760055a8d235 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -25,11 +25,18 @@ using APSInt = llvm::APSInt; namespace clang { namespace interp { +static bool hasTrivialDefaultCtorParent(const FieldDecl *FD) { + assert(FD); + assert(FD->getParent()->isUnion()); + const auto *CXXRD = dyn_cast<CXXRecordDecl>(FD->getParent()); + return !CXXRD || CXXRD->hasTrivialDefaultConstructor(); +} + static bool refersToUnion(const Expr *E) { for (;;) { if (const auto *ME = dyn_cast<MemberExpr>(E)) { if (const auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl()); - FD && FD->getParent()->isUnion()) + FD && FD->getParent()->isUnion() && hasTrivialDefaultCtorParent(FD)) return true; E = ME->getBase(); continue; diff --git a/clang/test/AST/ByteCode/unions.cpp b/clang/test/AST/ByteCode/unions.cpp index 7cfd0d677a7b3..ae71776353cf0 100644 --- a/clang/test/AST/ByteCode/unions.cpp +++ b/clang/test/AST/ByteCode/unions.cpp @@ -861,6 +861,61 @@ namespace CopyCtorMutable { // both-note {{in call}} } + +namespace NonTrivialCtor { + struct A { int x = 1; constexpr int f() { return 1; } }; + struct B : A { int y = 1; constexpr int g() { return 2; } }; + struct C { + int x; + constexpr virtual int f() = 0; + }; + struct D : C { + int y; + constexpr virtual int f() override { return 3; } + }; + + union U { + int n; + B b; + D d; + }; + + consteval int test(int which) { + if (which == 0) {} + + U u{.n = 5}; + assert_active(u); + assert_active(u.n); + assert_inactive(u.b); + + switch (which) { + case 0: + u.b.x = 10; // both-note {{assignment to member 'b' of union with active member 'n'}} + return u.b.f(); + case 1: + u.b.y = 10; // both-note {{assignment to member 'b' of union with active member 'n'}} + return u.b.g(); + case 2: + u.d.x = 10; // both-note {{assignment to member 'd' of union with active member 'n'}} + return u.d.f(); + case 3: + u.d.y = 10; // both-note {{assignment to member 'd' of union with active member 'n'}} + return u.d.f(); + } + + return 1; + } + static_assert(test(0)); // both-error {{not an integral constant expression}} \ + // both-note {{in call}} + static_assert(test(1)); // both-error {{not an integral constant expression}} \ + // both-note {{in call}} + static_assert(test(2)); // both-error {{not an integral constant expression}} \ + // both-note {{in call}} + static_assert(test(3)); // both-error {{not an integral constant expression}} \ + // both-note {{in call}} + +} + #endif namespace AddressComparison { >From 67dd9f569a80ae2d3d2bb1673c86e000c9c9b900 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Tue, 22 Jul 2025 06:59:02 +0200 Subject: [PATCH 2/2] [clang][bytecode] Activate primitive fields before initializing them The initializer itself might need the field to be active. --- clang/lib/AST/ByteCode/Compiler.cpp | 89 ++++++++++++++++------------- clang/lib/AST/ByteCode/Compiler.h | 2 + clang/lib/AST/ByteCode/Interp.h | 10 ++++ clang/lib/AST/ByteCode/Opcodes.td | 1 + clang/test/AST/ByteCode/unions.cpp | 17 +++++- 5 files changed, 77 insertions(+), 42 deletions(-) diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index e760055a8d235..63ac536c2b445 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -25,41 +25,6 @@ using APSInt = llvm::APSInt; namespace clang { namespace interp { -static bool hasTrivialDefaultCtorParent(const FieldDecl *FD) { - assert(FD); - assert(FD->getParent()->isUnion()); - const auto *CXXRD = dyn_cast<CXXRecordDecl>(FD->getParent()); - return !CXXRD || CXXRD->hasTrivialDefaultConstructor(); -} - -static bool refersToUnion(const Expr *E) { - for (;;) { - if (const auto *ME = dyn_cast<MemberExpr>(E)) { - if (const auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl()); - FD && FD->getParent()->isUnion() && hasTrivialDefaultCtorParent(FD)) - return true; - E = ME->getBase(); - continue; - } - - if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(E)) { - E = ASE->getBase()->IgnoreImplicit(); - continue; - } - - if (const auto *ICE = dyn_cast<ImplicitCastExpr>(E); - ICE && (ICE->getCastKind() == CK_NoOp || - ICE->getCastKind() == CK_DerivedToBase || - ICE->getCastKind() == CK_UncheckedDerivedToBase)) { - E = ICE->getSubExpr(); - continue; - } - - break; - } - return false; -} - static std::optional<bool> getBoolValue(const Expr *E) { if (const auto *CE = dyn_cast_if_present<ConstantExpr>(E); CE && CE->hasAPValueResult() && @@ -5408,6 +5373,53 @@ bool Compiler<Emitter>::maybeEmitDeferredVarInit(const VarDecl *VD) { return true; } +static bool hasTrivialDefaultCtorParent(const FieldDecl *FD) { + assert(FD); + assert(FD->getParent()->isUnion()); + const auto *CXXRD = dyn_cast<CXXRecordDecl>(FD->getParent()); + return !CXXRD || CXXRD->hasTrivialDefaultConstructor(); +} + +template <class Emitter> bool Compiler<Emitter>::refersToUnion(const Expr *E) { + for (;;) { + if (const auto *ME = dyn_cast<MemberExpr>(E)) { + if (const auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl()); + FD && FD->getParent()->isUnion() && hasTrivialDefaultCtorParent(FD)) + return true; + E = ME->getBase(); + continue; + } + + if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(E)) { + E = ASE->getBase()->IgnoreImplicit(); + continue; + } + + if (const auto *ICE = dyn_cast<ImplicitCastExpr>(E); + ICE && (ICE->getCastKind() == CK_NoOp || + ICE->getCastKind() == CK_DerivedToBase || + ICE->getCastKind() == CK_UncheckedDerivedToBase)) { + E = ICE->getSubExpr(); + continue; + } + + if (const auto *This = dyn_cast<CXXThisExpr>(E)) { + const auto *ThisRecord = + This->getType()->getPointeeType()->getAsRecordDecl(); + if (!ThisRecord->isUnion()) + return false; + // Otherwise, always activate if we're in the ctor. + if (const auto *Ctor = + dyn_cast_if_present<CXXConstructorDecl>(CompilingFunction)) + return Ctor->getParent() == ThisRecord; + return false; + } + + break; + } + return false; +} + template <class Emitter> bool Compiler<Emitter>::visitDeclStmt(const DeclStmt *DS, bool EvaluateConditionDecl) { @@ -5940,16 +5952,15 @@ bool Compiler<Emitter>::compileConstructor(const CXXConstructorDecl *Ctor) { return false; if (OptPrimType T = this->classify(InitExpr)) { + if (Activate && !this->emitActivateThisField(FieldOffset, InitExpr)) + return false; + if (!this->visit(InitExpr)) return false; bool BitField = F->isBitField(); - if (BitField && Activate) - return this->emitInitThisBitFieldActivate(*T, F, FieldOffset, InitExpr); if (BitField) return this->emitInitThisBitField(*T, F, FieldOffset, InitExpr); - if (Activate) - return this->emitInitThisFieldActivate(*T, FieldOffset, InitExpr); return this->emitInitThisField(*T, FieldOffset, InitExpr); } // Non-primitive case. Get a pointer to the field-to-initialize diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h index 503269399c757..3a263429a0e15 100644 --- a/clang/lib/AST/ByteCode/Compiler.h +++ b/clang/lib/AST/ByteCode/Compiler.h @@ -401,6 +401,8 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>, bool checkLiteralType(const Expr *E); bool maybeEmitDeferredVarInit(const VarDecl *VD); + bool refersToUnion(const Expr *E); + protected: /// Variable to storage mapping. llvm::DenseMap<const ValueDecl *, Scope::Local> Locals; diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index 7f29200f8617f..a234f8ebf0c82 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -1983,6 +1983,16 @@ static inline bool Activate(InterpState &S, CodePtr OpPC) { return true; } +static inline bool ActivateThisField(InterpState &S, CodePtr OpPC, uint32_t I) { + if (S.checkingPotentialConstantExpression()) + return false; + + const Pointer &Ptr = S.Current->getThis(); + assert(Ptr.atField(I).canBeInitialized()); + Ptr.atField(I).activate(); + return true; +} + template <PrimType Name, class T = typename PrimConv<Name>::T> bool StoreActivate(InterpState &S, CodePtr OpPC) { const T &Value = S.Stk.pop<T>(); diff --git a/clang/lib/AST/ByteCode/Opcodes.td b/clang/lib/AST/ByteCode/Opcodes.td index 80703ad72d954..abfed77750f87 100644 --- a/clang/lib/AST/ByteCode/Opcodes.td +++ b/clang/lib/AST/ByteCode/Opcodes.td @@ -510,6 +510,7 @@ def StoreBitFieldActivate : StoreBitFieldOpcode {} def StoreBitFieldActivatePop : StoreBitFieldOpcode {} def Activate : Opcode {} +def ActivateThisField : Opcode { let Args = [ArgUint32]; } // [Pointer, Value] -> [] def Init : StoreOpcode {} diff --git a/clang/test/AST/ByteCode/unions.cpp b/clang/test/AST/ByteCode/unions.cpp index ae71776353cf0..139e318e1ba68 100644 --- a/clang/test/AST/ByteCode/unions.cpp +++ b/clang/test/AST/ByteCode/unions.cpp @@ -79,10 +79,9 @@ namespace DefaultInit { constexpr U1 u1; /// OK. - constexpr int foo() { // expected-error {{never produces a constant expression}} + constexpr int foo() { U1 u; - return u.a; // both-note {{read of member 'a' of union with active member 'b'}} \ - // expected-note {{read of member 'a' of union with active member 'b'}} + return u.a; // both-note {{read of member 'a' of union with active member 'b'}} } static_assert(foo() == 42); // both-error {{not an integral constant expression}} \ // both-note {{in call to}} @@ -916,6 +915,18 @@ namespace NonTrivialCtor { } +namespace PrimitiveFieldInitActivates { + /// The initializer of a needs the field to be active _before_ it's visited. + template<int> struct X {}; + union V { + int a, b; + constexpr V(X<0>) : a(a = 1) {} // ok + constexpr V(X<2>) : a() { b = 1; } // ok + }; + constinit V v0 = X<0>(); + constinit V v2 = X<2>(); +} + #endif namespace AddressComparison { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits