[clang][bytecode] Misc union fixes

[tbaederr]

First, don't forget to also activate fields which are bitfields on assignment.

Second, when deactivating fields, recurse into records.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

First, don't forget to also activate fields which are bitfields on assignment.

Second, when deactivating fields, recurse into records.

---

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

3 Files Affected:

• (modified) clang/lib/AST/ByteCode/Interp.h (+6-2)
• (modified) clang/lib/AST/ByteCode/Pointer.cpp (+11-4)
• (modified) clang/test/AST/ByteCode/unions.cpp (+53-4)

diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index f9623131809e5..ddd0b44bb37bf 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -1948,8 +1948,10 @@ bool StoreBitField(InterpState &S, CodePtr OpPC) {
   const Pointer &Ptr = S.Stk.peek<Pointer>();
   if (!CheckStore(S, OpPC, Ptr))
     return false;
-  if (Ptr.canBeInitialized())
+  if (Ptr.canBeInitialized()) {
     Ptr.initialize();
+    Ptr.activate();
+  }
   if (const auto *FD = Ptr.getField())
     Ptr.deref<T>() = Value.truncate(FD->getBitWidthValue());
   else
@@ -1963,8 +1965,10 @@ bool StoreBitFieldPop(InterpState &S, CodePtr OpPC) {
   const Pointer &Ptr = S.Stk.pop<Pointer>();
   if (!CheckStore(S, OpPC, Ptr))
     return false;
-  if (Ptr.canBeInitialized())
+  if (Ptr.canBeInitialized()) {
     Ptr.initialize();
+    Ptr.activate();
+  }
   if (const auto *FD = Ptr.getField())
     Ptr.deref<T>() = Value.truncate(FD->getBitWidthValue());
   else
diff --git a/clang/lib/AST/ByteCode/Pointer.cpp b/clang/lib/AST/ByteCode/Pointer.cpp
index 0ad47645d39cc..31be77a5b936b 100644
--- a/clang/lib/AST/ByteCode/Pointer.cpp
+++ b/clang/lib/AST/ByteCode/Pointer.cpp
@@ -481,8 +481,17 @@ void Pointer::activate() const {
   auto activate = [](Pointer &P) -> void {
     P.getInlineDesc()->IsActive = true;
   };
-  auto deactivate = [](Pointer &P) -> void {
+
+  std::function<void(Pointer &)> deactivate;
+  deactivate = [&deactivate](Pointer &P) -> void {
     P.getInlineDesc()->IsActive = false;
+    if (const Record *R = P.getRecord()) {
+      for (const Record::Field &F : R->fields()) {
+        Pointer FieldPtr = P.atField(F.Offset);
+        if (FieldPtr.getInlineDesc()->IsActive)
+          deactivate(FieldPtr);
+      }
+    }
   };
 
   // Unions might be nested etc., so find the topmost Pointer that's
@@ -492,21 +501,19 @@ void Pointer::activate() const {
     UnionPtr = UnionPtr.getBase();
 
   assert(UnionPtr.getFieldDesc()->isUnion());
-
   const Record *UnionRecord = UnionPtr.getRecord();
   for (const Record::Field &F : UnionRecord->fields()) {
     Pointer FieldPtr = UnionPtr.atField(F.Offset);
     if (FieldPtr == *this) {
+      // No need to deactivate, will be activated in the next loop.
     } else {
       deactivate(FieldPtr);
-      // FIXME: Recurse.
     }
   }
 
   Pointer B = *this;
   while (B != UnionPtr) {
     activate(B);
-    // FIXME: Need to de-activate other fields of parent records.
     B = B.getBase();
   }
 }
diff --git a/clang/test/AST/ByteCode/unions.cpp b/clang/test/AST/ByteCode/unions.cpp
index 36f4b72335af3..c65c1e11cb1d9 100644
--- a/clang/test/AST/ByteCode/unions.cpp
+++ b/clang/test/AST/ByteCode/unions.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify=expected,both %s
-// RUN: %clang_cc1 -std=c++20 -fexperimental-new-constant-interpreter -verify=expected,both %s
-// RUN: %clang_cc1 -verify=ref,both %s
-// RUN: %clang_cc1 -std=c++20 -verify=ref,both %s
+// RUN: %clang_cc1            -verify=expected,both %s -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -std=c++20 -verify=expected,both %s -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1            -verify=ref,both      %s
+// RUN: %clang_cc1 -std=c++20 -verify=ref,both      %s
 
 union U {
   int a;
@@ -600,6 +600,55 @@ namespace MoveOrAssignOp {
   }
   static_assert(foo());
 }
+
+namespace BitFields {
+  constexpr bool simple() {
+    union U {
+      unsigned a : 1;
+      unsigned b : 1;
+    };
+
+    U u{1};
+    u.b = 1;
+    return u.b;
+  }
+  static_assert(simple());
+}
+
+namespace deactivateRecurses {
+
+  constexpr int foo() {
+    struct A {
+      struct {
+        int a;
+      };
+      int b;
+    };
+    struct B {
+      struct {
+        int a;
+        int b;
+      };
+    };
+
+    union U {
+      A a;
+      B b;
+    } u;
+
+    u.b.a = 10;
+    ++u.b.a;
+
+    u.a.a = 10;
+    ++u.a.a;
+
+    if (__builtin_constant_p(u.b.a))
+      return 10;
+
+    return 1;
+  }
+  static_assert(foo() == 1);
+  }
 #endif
 
 namespace AddressComparison {