[clang] db4e927 - [clang][bytecode] Misc union fixes (#146824)

[via cfe-commits]

Author: Timm Baeder
Date: 2025-07-06T14:55:29+02:00
New Revision: db4e927f9f02b40a0bc332205b88bc40edc29afc

URL: https://github.com/llvm/llvm-project/commit/db4e927f9f02b40a0bc332205b88bc40edc29afc
DIFF: https://github.com/llvm/llvm-project/commit/db4e927f9f02b40a0bc332205b88bc40edc29afc.diff

LOG: [clang][bytecode] Misc union fixes (#146824)

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

Second, when deactivating fields, recurse into records.

Added: 
    

Modified: 
    clang/lib/AST/ByteCode/Interp.h
    clang/lib/AST/ByteCode/Pointer.cpp
    clang/test/AST/ByteCode/unions.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 5d760f2f891e1..85a07abb5ed01 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -1930,8 +1930,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
@@ -1945,8 +1947,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 f049f1f2936cc..298aac35e3560 100644
--- a/clang/lib/AST/ByteCode/Pointer.cpp
+++ b/clang/lib/AST/ByteCode/Pointer.cpp
@@ -505,8 +505,19 @@ 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);
+      }
+      // FIXME: Bases?
+    }
   };
 
   // Unions might be nested etc., so find the topmost Pointer that's
@@ -516,21 +527,31 @@ void Pointer::activate() const {
     UnionPtr = UnionPtr.getBase();
 
   assert(UnionPtr.getFieldDesc()->isUnion());
-
   const Record *UnionRecord = UnionPtr.getRecord();
+
+  // The direct child pointer of the union that's on the path from
+  // this pointer to the union.
+  Pointer ChildPtr = *this;
+  assert(ChildPtr != UnionPtr);
+  while (true) {
+    if (ChildPtr.getBase() == UnionPtr)
+      break;
+    ChildPtr = ChildPtr.getBase();
+  }
+  assert(ChildPtr.getBase() == UnionPtr);
+
   for (const Record::Field &F : UnionRecord->fields()) {
     Pointer FieldPtr = UnionPtr.atField(F.Offset);
-    if (FieldPtr == *this) {
+    if (FieldPtr == ChildPtr) {
+      // 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 0e5f83b9572b3..f97990c1ff849 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;
@@ -612,6 +612,106 @@ namespace CopyEmptyUnion {
   }
   static_assert(foo() == 1);
 }
+
+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);
+}
+
+namespace AnonymousUnion {
+  struct Long {
+    struct {
+      unsigned is_long;
+    };
+    unsigned Size;
+  };
+
+  struct Short {
+    struct {
+      unsigned is_long;
+      unsigned Size;
+    };
+    char data;
+  };
+
+  union Rep {
+    Short S;
+    Long L;
+  };
+
+#define assert_active(F)   if (!__builtin_is_within_lifetime(&F)) (1/0);
+#define assert_inactive(F) if ( __builtin_is_within_lifetime(&F)) (1/0);
+  consteval int test() {
+    union UU {
+      struct {
+        Rep R;
+        int a;
+      };
+    } U;
+
+    U.R.S.Size = 10;
+    assert_active(U);
+    assert_active(U.R);
+    assert_active(U.R.S);
+    assert_active(U.R.S.Size);
+
+    U.a = 10;
+    assert_active(U.a);
+    assert_active(U);
+
+    assert_active(U);
+    assert_active(U.R);
+    assert_active(U.R.S);
+    assert_active(U.R.S.Size);
+
+    return 1;
+  }
+  static_assert(test() == 1);
+}
 #endif
 
 namespace AddressComparison {