[clang][Interp] Fix activating via indirect field initializers

[tbaederr]

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.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/102753.diff

4 Files Affected:

• (modified) clang/lib/AST/Interp/Compiler.cpp (+5-9)
• (modified) clang/lib/AST/Interp/Interp.h (+1-25)
• (modified) clang/lib/AST/Interp/Opcodes.td (-4)
• (modified) clang/test/AST/Interp/unions.cpp (+41)

diff --git a/clang/lib/AST/Interp/Compiler.cpp b/clang/lib/AST/Interp/Compiler.cpp
index 1fa7a944448fb..5bed71392740e 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 196fc15a77519..af33d507ef8d7 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 220dff0c556b1..3478eb174e518 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 1f52950b1e09b..4607df6c1d644 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