[Sema] Fix handling of fields with initializers in nested anonymous unions.

[efriedma-quic]

Make sure we count the anonymous union as an initialized field, so we properly construct the AST.

Included bonus testcase Test3, which shows a remaining gap: an anonymous union can contain a partially initialized anonymous struct, and we handle that inconsistently.

Fixes #91257

[llvmbot]

@llvm/pr-subscribers-clang

Author: Eli Friedman (efriedma-quic)

Changes

Make sure we count the anonymous union as an initialized field, so we properly construct the AST.

Included bonus testcase Test3, which shows a remaining gap: an anonymous union can contain a partially initialized anonymous struct, and we handle that inconsistently.

Fixes #91257

---

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

3 Files Affected:

• (modified) clang/lib/Sema/SemaInit.cpp (+8-11)
• (modified) clang/test/AST/ast-dump-APValue-anon-union.cpp (+1-1)
• (modified) clang/test/SemaCXX/cxx1y-initializer-aggregates.cpp (+35)

diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 7d9eaf6720461..0a3d3ea019b1c 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -813,19 +813,13 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity,
 
   if (const RecordType *RType = ILE->getType()->getAs<RecordType>()) {
     const RecordDecl *RDecl = RType->getDecl();
-    if (RDecl->isUnion() && ILE->getInitializedFieldInUnion())
+    if (RDecl->isUnion() && ILE->getInitializedFieldInUnion()) {
       FillInEmptyInitForField(0, ILE->getInitializedFieldInUnion(),
                               Entity, ILE, RequiresSecondPass, FillWithNoInit);
-    else if (RDecl->isUnion() && isa<CXXRecordDecl>(RDecl) &&
-             cast<CXXRecordDecl>(RDecl)->hasInClassInitializer()) {
-      for (auto *Field : RDecl->fields()) {
-        if (Field->hasInClassInitializer()) {
-          FillInEmptyInitForField(0, Field, Entity, ILE, RequiresSecondPass,
-                                  FillWithNoInit);
-          break;
-        }
-      }
     } else {
+      assert((!RDecl->isUnion() || !isa<CXXRecordDecl>(RDecl) ||
+              !cast<CXXRecordDecl>(RDecl)->hasInClassInitializer()) &&
+             "We should have computed initialized fields already");
       // The fields beyond ILE->getNumInits() are default initialized, so in
       // order to leave them uninitialized, the ILE is expanded and the extra
       // fields are then filled with NoInitExpr.
@@ -2163,12 +2157,15 @@ void InitListChecker::CheckStructUnionTypes(
         return;
       for (RecordDecl::field_iterator FieldEnd = RD->field_end();
            Field != FieldEnd; ++Field) {
-        if (Field->hasInClassInitializer()) {
+        if (Field->hasInClassInitializer() ||
+            (Field->isAnonymousStructOrUnion() &&
+             Field->getType()->getAsCXXRecordDecl()->hasInClassInitializer())) {
           StructuredList->setInitializedFieldInUnion(*Field);
           // FIXME: Actually build a CXXDefaultInitExpr?
           return;
         }
       }
+      llvm_unreachable("Couldn't find in-class initializer");
     }
 
     // Value-initialize the first member of the union that isn't an unnamed
diff --git a/clang/test/AST/ast-dump-APValue-anon-union.cpp b/clang/test/AST/ast-dump-APValue-anon-union.cpp
index 0e6466ee1fd73..ffe14ed7322de 100644
--- a/clang/test/AST/ast-dump-APValue-anon-union.cpp
+++ b/clang/test/AST/ast-dump-APValue-anon-union.cpp
@@ -36,7 +36,7 @@ void Test() {
 
   constexpr U0 u0a{};
   // CHECK:  | `-VarDecl {{.*}} <col:{{.*}}, col:{{.*}}> col:{{.*}} u0a 'const U0' constexpr listinit
-  // CHECK-NEXT:  |   |-value: Union None
+  // CHECK-NEXT:  |   |-value: Union .U0::(anonymous union at {{.*}}) Union .f Float 3.141500e+00
 
   constexpr U0 u0b{3.1415f};
   // CHECK:  | `-VarDecl {{.*}} <col:{{.*}}, col:{{.*}}> col:{{.*}} u0b 'const U0' constexpr listinit
diff --git a/clang/test/SemaCXX/cxx1y-initializer-aggregates.cpp b/clang/test/SemaCXX/cxx1y-initializer-aggregates.cpp
index 3d5e7726a17e5..ca60cf513b4a2 100644
--- a/clang/test/SemaCXX/cxx1y-initializer-aggregates.cpp
+++ b/clang/test/SemaCXX/cxx1y-initializer-aggregates.cpp
@@ -77,3 +77,38 @@ namespace use_self {
 
   int fib(int n) { return FibTree{n}.v; }
 }
+
+namespace nested_union {
+  union Test1 {
+    union {
+      int inner { 42 };
+    };
+    int outer;
+  };
+  static_assert(Test1{}.inner == 42, "");
+  struct Test2 {
+    union {
+      struct {
+        int inner : 32 { 42 }; // expected-warning {{C++20 extension}}
+      };
+      int outer;
+    };
+  };
+  static_assert(Test2{}.inner == 42, "");
+  struct Int { int x; };
+  struct Test3 {
+    int x;
+    union {
+      struct {
+        const int& y;
+        int inner : 32 { 42 }; // expected-warning {{C++20 extension}}
+      };
+      int outer;
+    };
+  };
+  constexpr char f(Test3) { return 1; } // expected-note {{candidate function}}
+  constexpr char f(Int) { return 2; } // expected-note {{candidate function}}
+  // FIXME: This shouldn't be ambiguous; either we should reject the declaration
+  // of Test3, or we should exclude f(Test3) as a candidate.
+  static_assert(f({1}) == 2, ""); // expected-error {{call to 'f' is ambiguous}}
+}

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[Endilll]

LGTM, but someone else should take a look as well.

[efriedma-quic]

Ping

[zygoloid]

Looks good to me too.

[zygoloid]

Might be useful to also test that the rest of the union member gets initialized.

Suggested change

	struct Test2 {
	union {
	struct {
	int inner : 32 { 42 }; // expected-warning {{C++20 extension}}
	};
	int outer;
	};
	};
	static_assert(Test2{}.inner == 42, "");
	struct Test2 {
	union {
	struct {
	int inner : 32 { 42 }; // expected-warning {{C++20 extension}}
	int inner_no_init;
	};
	int outer;
	};
	};
	static_assert(Test2{}.inner == 42, "");
	static_assert(Test2{}.inner_no_init == 0, "");

[zygoloid]

Maybe an expected-error for Test3 test3 = {1}; would be nice here too. I think that part at least should work -- if I'm understanding correctly, we're only missing the check for the uninitialized reference member during overload resolution / in VerifyOnly mode?