[clang] [Sema] Fix handling of fields with initializers in nested anonymous unions. (PR #91692)

Eli Friedman via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 3 20:32:27 PDT 2024


https://github.com/efriedma-quic updated https://github.com/llvm/llvm-project/pull/91692

>From 6123b5f43b11f968474baa15623c4bf4d14ed188 Mon Sep 17 00:00:00 2001
From: Eli Friedman <efriedma at quicinc.com>
Date: Thu, 9 May 2024 19:40:46 -0700
Subject: [PATCH] [Sema] Fix handling of fields with initializers in nested
 anonymous unions.

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
---
 clang/lib/Sema/SemaInit.cpp                   | 23 +++++------
 .../test/AST/ast-dump-APValue-anon-union.cpp  |  2 +-
 .../SemaCXX/cxx1y-initializer-aggregates.cpp  | 38 +++++++++++++++++++
 3 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 79bdc8e9f8783..9ed3e8a0df025 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -814,19 +814,13 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity,
 
   if (const RecordType *RType = ILE->getType()->getAs<RecordType>()) {
     const RecordDecl *RDecl = RType->getDecl();
-    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;
-        }
-      }
+    if (RDecl->isUnion() && ILE->getInitializedFieldInUnion()) {
+      FillInEmptyInitForField(0, ILE->getInitializedFieldInUnion(), Entity, ILE,
+                              RequiresSecondPass, FillWithNoInit);
     } 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.
@@ -2164,12 +2158,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..03a6800898d18 100644
--- a/clang/test/SemaCXX/cxx1y-initializer-aggregates.cpp
+++ b/clang/test/SemaCXX/cxx1y-initializer-aggregates.cpp
@@ -77,3 +77,41 @@ 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 inner_no_init;
+      };
+      int outer;
+    };
+  };
+  static_assert(Test2{}.inner == 42, "");
+  static_assert(Test2{}.inner_no_init == 0, "");
+  struct Int { int x; };
+  struct Test3 {
+    int x;
+    union {
+      struct { // expected-note {{in implicit initialization}}
+        const int& y; // expected-note {{uninitialized reference member is here}}
+        int inner : 32 { 42 }; // expected-warning {{C++20 extension}}
+      };
+      int outer;
+    };
+  };
+  Test3 test3 = {1}; // expected-error {{reference member of type 'const int &' uninitialized}}
+  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}}
+}



More information about the cfe-commits mailing list