[clang] dff2ca4 - [clang][bytecode] Add special case for anonymous unions (#128681)

[via cfe-commits]

Author: Timm Baeder
Date: 2025-02-25T12:46:06+01:00
New Revision: dff2ca424c20c672b418ec86ac3a120fad4fb364

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

LOG: [clang][bytecode] Add special case for anonymous unions (#128681)

This fixes the expected output to match the one of the current
interpreter.

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 31126e48c7cd7..5e0d2e91fb1b2 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -139,17 +139,19 @@ static bool CheckActive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
 
   Pointer U = Ptr.getBase();
   Pointer C = Ptr;
-  while (!U.isRoot() && U.inUnion() && !U.isActive()) {
-    if (U.getField())
-      C = U;
+  while (!U.isRoot() && !U.isActive()) {
+    // A little arbitrary, but this is what the current interpreter does.
+    // See the AnonymousUnion test in test/AST/ByteCode/unions.cpp.
+    // GCC's output is more similar to what we would get without
+    // this condition.
+    if (U.getRecord() && U.getRecord()->isAnonymousUnion())
+      break;
+
+    C = U;
     U = U.getBase();
   }
   assert(C.isField());
 
-  // Get the inactive field descriptor.
-  const FieldDecl *InactiveField = C.getField();
-  assert(InactiveField);
-
   // Consider:
   // union U {
   //   struct {
@@ -165,6 +167,11 @@ static bool CheckActive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
   if (!U.getFieldDesc()->isUnion())
     return true;
 
+  // Get the inactive field descriptor.
+  assert(!C.isActive());
+  const FieldDecl *InactiveField = C.getField();
+  assert(InactiveField);
+
   // Find the active field of the union.
   const Record *R = U.getRecord();
   assert(R && R->isUnion() && "Not a union");

diff  --git a/clang/lib/AST/ByteCode/Pointer.cpp b/clang/lib/AST/ByteCode/Pointer.cpp
index 3033bd47adf75..92cfa192fd385 100644
--- a/clang/lib/AST/ByteCode/Pointer.cpp
+++ b/clang/lib/AST/ByteCode/Pointer.cpp
@@ -437,28 +437,35 @@ void Pointer::activate() const {
   if (!getInlineDesc()->InUnion)
     return;
 
-  getInlineDesc()->IsActive = true;
+  auto activate = [](Pointer &P) -> void {
+    P.getInlineDesc()->IsActive = true;
+  };
+  auto deactivate = [](Pointer &P) -> void {
+    P.getInlineDesc()->IsActive = false;
+  };
 
-  // Get the union, iterate over its fields and DEactivate all others.
+  // Unions might be nested etc., so find the topmost Pointer that's
+  // not in a union anymore.
   Pointer UnionPtr = getBase();
-  while (!UnionPtr.getFieldDesc()->isUnion())
+  while (!UnionPtr.isRoot() && UnionPtr.inUnion())
     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) {
     } else {
-      FieldPtr.getInlineDesc()->IsActive = false;
+      deactivate(FieldPtr);
       // FIXME: Recurse.
     }
   }
 
-  Pointer B = getBase();
-  while (!B.isRoot() && B.inUnion()) {
+  Pointer B = *this;
+  while (B != UnionPtr) {
+    activate(B);
     // FIXME: Need to de-activate other fields of parent records.
-    B.getInlineDesc()->IsActive = true;
-    assert(B.isActive());
     B = B.getBase();
   }
 }

diff  --git a/clang/lib/AST/ByteCode/Pointer.h b/clang/lib/AST/ByteCode/Pointer.h
index 3970d5833fcdc..fd33ee9955f55 100644
--- a/clang/lib/AST/ByteCode/Pointer.h
+++ b/clang/lib/AST/ByteCode/Pointer.h
@@ -494,9 +494,6 @@ class Pointer {
   /// Returns the field information.
   const FieldDecl *getField() const { return getFieldDesc()->asFieldDecl(); }
 
-  /// Checks if the object is a union.
-  bool isUnion() const;
-
   /// Checks if the storage is extern.
   bool isExtern() const {
     if (isBlockPointer())

diff  --git a/clang/test/AST/ByteCode/unions.cpp b/clang/test/AST/ByteCode/unions.cpp
index c6b5e34810f05..2064cae11e970 100644
--- a/clang/test/AST/ByteCode/unions.cpp
+++ b/clang/test/AST/ByteCode/unions.cpp
@@ -485,4 +485,23 @@ namespace IFD {
   }
   static_assert(test());
 }
+
+namespace AnonymousUnion {
+  struct A {
+    int x;
+    union { int p, q; };
+  };
+  union B {
+    A a;
+    int bb;
+  };
+
+  constexpr B return_init_all() {
+    B b = {.bb = 1};
+    b.a.x = 2;
+    return b;
+  }
+  static_assert(return_init_all().a.p == 7); // both-error {{}} \
+                                             // both-note {{read of member 'p' of union with no active member}}
+}
 #endif