[clang] [clang][Interp] Fix activating via indirect field initializers (PR #102753)
Timm Baeder via cfe-commits
cfe-commits at lists.llvm.org
Sat Aug 10 06:57:09 PDT 2024
https://github.com/tbaederr created https://github.com/llvm/llvm-project/pull/102753
Pointer::activate() propagates up anyway, so that is handled. But we need to call activate() in any case since the parent might not be a union, but the activate() is still needed. Always call it and hope that the InUnion flag takes care of the potential performance problems.
>From 082892fff50e3e971c2b1a9394a3a088f697ed0b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Sat, 10 Aug 2024 15:45:14 +0200
Subject: [PATCH] [clang][Interp] Fix activating via indirect field
initializers
Pointer::activate() propagates up anyway, so that is handled. But
we need to call activate() in any case since the parent might not
be a union, but the activate() is still needed. Always call it and
hope that the InUnion flag takes care of the potential performance
problems.
---
clang/lib/AST/Interp/Compiler.cpp | 14 ++++-------
clang/lib/AST/Interp/Interp.h | 26 +-------------------
clang/lib/AST/Interp/Opcodes.td | 4 ---
clang/test/AST/Interp/unions.cpp | 41 +++++++++++++++++++++++++++++++
4 files changed, 47 insertions(+), 38 deletions(-)
diff --git a/clang/lib/AST/Interp/Compiler.cpp b/clang/lib/AST/Interp/Compiler.cpp
index 1fa7a944448fb3..5bed71392740eb 100644
--- a/clang/lib/AST/Interp/Compiler.cpp
+++ b/clang/lib/AST/Interp/Compiler.cpp
@@ -4737,8 +4737,7 @@ bool Compiler<Emitter>::visitFunc(const FunctionDecl *F) {
// Classify the return type.
ReturnType = this->classify(F->getReturnType());
- auto emitFieldInitializer = [&](const Record *R, const Record::Field *F,
- unsigned FieldOffset,
+ auto emitFieldInitializer = [&](const Record::Field *F, unsigned FieldOffset,
const Expr *InitExpr) -> bool {
// We don't know what to do with these, so just return false.
if (InitExpr->getType().isNull())
@@ -4750,8 +4749,6 @@ bool Compiler<Emitter>::visitFunc(const FunctionDecl *F) {
if (F->isBitField())
return this->emitInitThisBitField(*T, F, FieldOffset, InitExpr);
- if (R->isUnion())
- return this->emitInitThisFieldActive(*T, FieldOffset, InitExpr);
return this->emitInitThisField(*T, FieldOffset, InitExpr);
}
// Non-primitive case. Get a pointer to the field-to-initialize
@@ -4787,7 +4784,7 @@ bool Compiler<Emitter>::visitFunc(const FunctionDecl *F) {
if (const FieldDecl *Member = Init->getMember()) {
const Record::Field *F = R->getField(Member);
- if (!emitFieldInitializer(R, F, F->Offset, InitExpr))
+ if (!emitFieldInitializer(F, F->Offset, InitExpr))
return false;
} else if (const Type *Base = Init->getBaseClass()) {
const auto *BaseDecl = Base->getAsCXXRecordDecl();
@@ -4815,11 +4812,11 @@ bool Compiler<Emitter>::visitFunc(const FunctionDecl *F) {
assert(IFD->getChainingSize() >= 2);
unsigned NestedFieldOffset = 0;
- const Record *FieldRecord = nullptr;
const Record::Field *NestedField = nullptr;
for (const NamedDecl *ND : IFD->chain()) {
const auto *FD = cast<FieldDecl>(ND);
- FieldRecord = this->P.getOrCreateRecord(FD->getParent());
+ const Record *FieldRecord =
+ this->P.getOrCreateRecord(FD->getParent());
assert(FieldRecord);
NestedField = FieldRecord->getField(FD);
@@ -4829,8 +4826,7 @@ bool Compiler<Emitter>::visitFunc(const FunctionDecl *F) {
}
assert(NestedField);
- if (!emitFieldInitializer(FieldRecord, NestedField, NestedFieldOffset,
- InitExpr))
+ if (!emitFieldInitializer(NestedField, NestedFieldOffset, InitExpr))
return false;
} else {
assert(Init->isDelegatingInitializer());
diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index 196fc15a77519b..af33d507ef8d73 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -1391,6 +1391,7 @@ bool InitThisField(InterpState &S, CodePtr OpPC, uint32_t I) {
return false;
const Pointer &Field = This.atField(I);
Field.deref<T>() = S.Stk.pop<T>();
+ Field.activate();
Field.initialize();
return true;
}
@@ -1413,20 +1414,6 @@ bool InitThisBitField(InterpState &S, CodePtr OpPC, const Record::Field *F,
return true;
}
-template <PrimType Name, class T = typename PrimConv<Name>::T>
-bool InitThisFieldActive(InterpState &S, CodePtr OpPC, uint32_t I) {
- if (S.checkingPotentialConstantExpression())
- return false;
- const Pointer &This = S.Current->getThis();
- if (!CheckThis(S, OpPC, This))
- return false;
- const Pointer &Field = This.atField(I);
- Field.deref<T>() = S.Stk.pop<T>();
- Field.activate();
- Field.initialize();
- return true;
-}
-
/// 1) Pops the value from the stack
/// 2) Peeks a pointer from the stack
/// 3) Pushes the value to field I of the pointer on the stack
@@ -1451,17 +1438,6 @@ bool InitBitField(InterpState &S, CodePtr OpPC, const Record::Field *F) {
return true;
}
-template <PrimType Name, class T = typename PrimConv<Name>::T>
-bool InitFieldActive(InterpState &S, CodePtr OpPC, uint32_t I) {
- const T &Value = S.Stk.pop<T>();
- const Pointer &Ptr = S.Stk.pop<Pointer>();
- const Pointer &Field = Ptr.atField(I);
- Field.deref<T>() = Value;
- Field.activate();
- Field.initialize();
- return true;
-}
-
//===----------------------------------------------------------------------===//
// GetPtr Local/Param/Global/Field/This
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/AST/Interp/Opcodes.td b/clang/lib/AST/Interp/Opcodes.td
index 220dff0c556b18..3478eb174e518e 100644
--- a/clang/lib/AST/Interp/Opcodes.td
+++ b/clang/lib/AST/Interp/Opcodes.td
@@ -440,8 +440,6 @@ def SetThisField : AccessOpcode;
// [Value] -> []
def InitThisField : AccessOpcode;
// [Value] -> []
-def InitThisFieldActive : AccessOpcode;
-// [Value] -> []
def InitThisBitField : Opcode {
let Types = [AluTypeClass];
let Args = [ArgRecordField, ArgUint32];
@@ -451,8 +449,6 @@ def InitThisBitField : Opcode {
def InitField : AccessOpcode;
// [Pointer, Value] -> []
def InitBitField : BitFieldOpcode;
-// [Pointer, Value] -> []
-def InitFieldActive : AccessOpcode;
//===----------------------------------------------------------------------===//
// Pointer access
diff --git a/clang/test/AST/Interp/unions.cpp b/clang/test/AST/Interp/unions.cpp
index 1f52950b1e09b6..4607df6c1d6444 100644
--- a/clang/test/AST/Interp/unions.cpp
+++ b/clang/test/AST/Interp/unions.cpp
@@ -306,4 +306,45 @@ namespace Zeroing {
static_assert(UnionWithUnnamedBitfield{}.n == 0, "");
static_assert(UnionWithUnnamedBitfield{1}.n == 1, "");
}
+
+namespace IndirectField {
+ struct S {
+ struct {
+ union {
+ struct {
+ int a;
+ int b;
+ };
+ int c;
+ };
+ int d;
+ };
+ union {
+ int e;
+ int f;
+ };
+ constexpr S(int a, int b, int d, int e) : a(a), b(b), d(d), e(e) {}
+ constexpr S(int c, int d, int f) : c(c), d(d), f(f) {}
+ };
+
+ constexpr S s1(1,2,3,4);
+ constexpr S s2(5, 6, 7);
+
+ static_assert(s1.a == 1, "");
+ static_assert(s1.b == 2, "");
+
+ static_assert(s1.c == 0, ""); // both-error {{constant expression}} both-note {{union with active member}}
+ static_assert(s1.d == 3, "");
+ static_assert(s1.e == 4, "");
+ static_assert(s1.f == 0, ""); // both-error {{constant expression}} both-note {{union with active member}}
+
+ static_assert(s2.a == 0, ""); // both-error {{constant expression}} both-note {{union with active member}}
+ static_assert(s2.b == 0, ""); // both-error {{constant expression}} both-note {{union with active member}}
+ static_assert(s2.c == 5, "");
+ static_assert(s2.d == 6, "");
+ static_assert(s2.e == 0, ""); // both-error {{constant expression}} both-note {{union with active member}}
+ static_assert(s2.f == 7, "");
+}
+
+
#endif
More information about the cfe-commits
mailing list