[llvm] [clang-tools-extra] [clang] [clang] Fix false positive -Wmissing-field-initializer for anonymous unions (PR #70829)

Mariya Podchishchaeva via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 7 02:05:39 PST 2023


https://github.com/Fznamznon updated https://github.com/llvm/llvm-project/pull/70829

>From ac30780250875802d13450d17e6959f9e2ad3a70 Mon Sep 17 00:00:00 2001
From: "Podchishchaeva, Mariya" <mariya.podchishchaeva at intel.com>
Date: Tue, 31 Oct 2023 09:27:51 -0700
Subject: [PATCH 1/6] [clang] Fix false positive -Wmissing-field-initializer
 for anonymous unions

Normally warning is not reported when a field has default initializer.
Do so for anonymous unions with default initializers as well.
No release note since it is a regression in clang 18.

Fixes https://github.com/llvm/llvm-project/issues/70384
---
 clang/lib/Sema/SemaInit.cpp                   | 99 +++++++++++--------
 .../SemaCXX/cxx2a-initializer-aggregates.cpp  | 66 ++++++++++++-
 2 files changed, 122 insertions(+), 43 deletions(-)

diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index ec796def96ad3..881e67587e430 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -349,17 +349,13 @@ class InitListChecker {
                       bool SubobjectIsDesignatorContext, unsigned &Index,
                       InitListExpr *StructuredList,
                       unsigned &StructuredIndex);
-  bool CheckDesignatedInitializer(const InitializedEntity &Entity,
-                                  InitListExpr *IList, DesignatedInitExpr *DIE,
-                                  unsigned DesigIdx,
-                                  QualType &CurrentObjectType,
-                                  RecordDecl::field_iterator *NextField,
-                                  llvm::APSInt *NextElementIndex,
-                                  unsigned &Index,
-                                  InitListExpr *StructuredList,
-                                  unsigned &StructuredIndex,
-                                  bool FinishSubobjectInit,
-                                  bool TopLevelObject);
+  bool CheckDesignatedInitializer(
+      const InitializedEntity &Entity, InitListExpr *IList,
+      DesignatedInitExpr *DIE, unsigned DesigIdx, QualType &CurrentObjectType,
+      RecordDecl::field_iterator *NextField, llvm::APSInt *NextElementIndex,
+      unsigned &Index, InitListExpr *StructuredList, unsigned &StructuredIndex,
+      bool FinishSubobjectInit, bool TopLevelObject,
+      llvm::SmallPtrSetImpl<FieldDecl *> *InitializedFields = nullptr);
   InitListExpr *getStructuredSubobjectInit(InitListExpr *IList, unsigned Index,
                                            QualType CurrentObjectType,
                                            InitListExpr *StructuredList,
@@ -2248,7 +2244,8 @@ void InitListChecker::CheckStructUnionTypes(
       // the next field that we'll be initializing.
       bool DesignatedInitFailed = CheckDesignatedInitializer(
           Entity, IList, DIE, 0, DeclType, &Field, nullptr, Index,
-          StructuredList, StructuredIndex, true, TopLevelObject);
+          StructuredList, StructuredIndex, true, TopLevelObject,
+          &InitializedFields);
       if (DesignatedInitFailed)
         hadError = true;
 
@@ -2256,7 +2253,6 @@ void InitListChecker::CheckStructUnionTypes(
       DesignatedInitExpr::Designator *D = DIE->getDesignator(0);
       if (!VerifyOnly && D->isFieldDesignator()) {
         FieldDecl *F = D->getFieldDecl();
-        InitializedFields.insert(F);
         if (!DesignatedInitFailed) {
           QualType ET = SemaRef.Context.getBaseElementType(F->getType());
           if (checkDestructorReference(ET, InitLoc, SemaRef)) {
@@ -2365,21 +2361,43 @@ void InitListChecker::CheckStructUnionTypes(
       !RD->isUnion()) {
     // It is possible we have one or more unnamed bitfields remaining.
     // Find first (if any) named field and emit warning.
-    for (RecordDecl::field_iterator it = HasDesignatedInit ? RD->field_begin()
-                                                           : Field,
-                                    end = RD->field_end();
-         it != end; ++it) {
-      if (HasDesignatedInit && InitializedFields.count(*it))
-        continue;
+    auto MissingFieldCheck = [&](const RecordDecl *Record,
+                                 RecordDecl::field_iterator StartField,
+                                 auto &&MissingFieldCheck) -> bool {
+      FieldDecl *FirstUninitialized = nullptr;
+      for (RecordDecl::field_iterator it = StartField,
+                                      end = Record->field_end();
+           it != end; ++it) {
+        bool AllSet = false;
+        if (it->isAnonymousStructOrUnion()) {
+          RecordDecl *RDAnon = it->getType()->getAsRecordDecl();
+          AllSet = MissingFieldCheck(RDAnon, RDAnon->field_begin(),
+                                     MissingFieldCheck);
+        }
+
+        if ((HasDesignatedInit && InitializedFields.count(*it)) ||
+            it->hasInClassInitializer() || AllSet) {
+          if (Record->isUnion())
+            return true;
+          continue;
+        }
 
-      if (!it->isUnnamedBitfield() && !it->hasInClassInitializer() &&
-          !it->getType()->isIncompleteArrayType()) {
+        if (!it->isUnnamedBitfield() &&
+            !it->getType()->isIncompleteArrayType() &&
+            !it->isAnonymousStructOrUnion() && !FirstUninitialized)
+          FirstUninitialized = *it;
+      }
+
+      if (FirstUninitialized) {
         SemaRef.Diag(IList->getSourceRange().getEnd(),
                      diag::warn_missing_field_initializers)
-            << *it;
-        break;
+            << FirstUninitialized;
+        return false;
       }
-    }
+      return true;
+    };
+    MissingFieldCheck(RD, HasDesignatedInit ? RD->field_begin() : Field,
+                      MissingFieldCheck);
   }
 
   // Check that any remaining fields can be value-initialized if we're not
@@ -2537,19 +2555,13 @@ class FieldInitializerValidatorCCC final : public CorrectionCandidateCallback {
 /// actually be initialized.
 ///
 /// @returns true if there was an error, false otherwise.
-bool
-InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity,
-                                            InitListExpr *IList,
-                                            DesignatedInitExpr *DIE,
-                                            unsigned DesigIdx,
-                                            QualType &CurrentObjectType,
-                                          RecordDecl::field_iterator *NextField,
-                                            llvm::APSInt *NextElementIndex,
-                                            unsigned &Index,
-                                            InitListExpr *StructuredList,
-                                            unsigned &StructuredIndex,
-                                            bool FinishSubobjectInit,
-                                            bool TopLevelObject) {
+bool InitListChecker::CheckDesignatedInitializer(
+    const InitializedEntity &Entity, InitListExpr *IList,
+    DesignatedInitExpr *DIE, unsigned DesigIdx, QualType &CurrentObjectType,
+    RecordDecl::field_iterator *NextField, llvm::APSInt *NextElementIndex,
+    unsigned &Index, InitListExpr *StructuredList, unsigned &StructuredIndex,
+    bool FinishSubobjectInit, bool TopLevelObject,
+    llvm::SmallPtrSetImpl<FieldDecl *> *InitializedFields) {
   if (DesigIdx == DIE->size()) {
     // C++20 designated initialization can result in direct-list-initialization
     // of the designated subobject. This is the only way that we can end up
@@ -2853,8 +2865,11 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity,
 
 
     // Update the designator with the field declaration.
-    if (!VerifyOnly)
+    if (!VerifyOnly) {
       D->setFieldDecl(*Field);
+      if (InitializedFields)
+        InitializedFields->insert(*Field);
+    }
 
     // Make sure that our non-designated initializer list has space
     // for a subobject corresponding to this field.
@@ -2929,10 +2944,10 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity,
 
       InitializedEntity MemberEntity =
         InitializedEntity::InitializeMember(*Field, &Entity);
-      if (CheckDesignatedInitializer(MemberEntity, IList, DIE, DesigIdx + 1,
-                                     FieldType, nullptr, nullptr, Index,
-                                     StructuredList, newStructuredIndex,
-                                     FinishSubobjectInit, false))
+      if (CheckDesignatedInitializer(
+              MemberEntity, IList, DIE, DesigIdx + 1, FieldType, nullptr,
+              nullptr, Index, StructuredList, newStructuredIndex,
+              FinishSubobjectInit, false, InitializedFields))
         return true;
     }
 
diff --git a/clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp b/clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
index 510ace58c35a6..87bc01a51d2f2 100644
--- a/clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
+++ b/clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
@@ -4,7 +4,7 @@
 // RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,reorder -Wno-c99-designator -Werror=reorder-init-list -Wno-initializer-overrides
 // RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,override -Wno-c99-designator -Wno-reorder-init-list -Werror=initializer-overrides
 // RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected -Wno-c99-designator -Wno-reorder-init-list -Wno-initializer-overrides
-// RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,wmissing -Wmissing-field-initializers -Wno-c99-designator -Wno-reorder-init-list -Wno-initializer-overrides
+// RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,wmissing -Wmissing-field-initializers -Wno-c99-designator -Wno-reorder-init-list -Wno-initializer-overrides -D NON_PEDANTIC
 
 
 namespace class_with_ctor {
@@ -247,3 +247,67 @@ void foo() {
                            //
 }
 }
+
+namespace GH70384 {
+
+struct A {
+  int m;
+  union { int a; float n = 0; };
+};
+
+struct B {
+  int m;
+  int b;
+  union { int a ; };
+};
+
+union CU {
+  int a = 1;
+  double b;
+};
+
+struct C {
+  int a;
+  union { int b; CU c;};
+};
+
+struct CC {
+  int a;
+  CU c;
+};
+
+void foo() {
+  A a = A{.m = 0};
+  A aa = {0};
+  A aaa = {.a = 7}; // wmissing-warning {{missing field 'm' initializer}}
+  B b = {.m = 1, .b = 3 }; //wmissing-warning {{missing field 'a' initializer}}
+  B bb = {1}; // wmissing-warning {{missing field 'b' initializer}}
+              // wmissing-warning at -1 {{missing field 'a' initializer}}
+  C c = {.a = 1}; // wmissing-warning {{missing field 'b' initializer}}
+  CC cc = {.a = 1}; //// wmissing-warning {{missing field 'c' initializer}}
+}
+
+#if defined NON_PEDANTIC
+struct C1 {
+  int m;
+  union { float b; union {int n = 1; }; };
+};
+
+struct C2 {
+  int m;
+  struct { float b; int n = 1; };
+};
+
+struct C3 {
+  int m;
+  struct { float b = 1; union {int a;}; int n = 1; };
+};
+
+C1 c = C1{.m = 1};
+C1 cc = C1{.b = 1}; // wmissing-warning {{missing field 'm' initializer}}
+C2 c1 = C2{.m = 1}; // wmissing-warning {{missing field 'b' initializer}}
+C2 c22 = C2{.m = 1, .b = 1};
+C3 c2 = C3{.b = 1}; // wmissing-warning {{missing field 'a' initializer}}
+                    // wmissing-warning at -1 {{missing field 'm' initializer}}
+#endif // NON_PEDANTIC
+}

>From b68dad578a757b3f3412ef70a13045f107b14c1f Mon Sep 17 00:00:00 2001
From: "Podchishchaeva, Mariya" <mariya.podchishchaeva at intel.com>
Date: Mon, 6 Nov 2023 07:46:13 -0800
Subject: [PATCH 2/6] Move diagnostic

---
 clang/lib/Sema/SemaInit.cpp                   | 163 ++++++++----------
 clang/test/Sema/missing-field-initializers.c  |   2 +-
 .../SemaCXX/cxx2a-initializer-aggregates.cpp  |   2 +-
 3 files changed, 77 insertions(+), 90 deletions(-)

diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 881e67587e430..3cf1a44f56b73 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -354,8 +354,7 @@ class InitListChecker {
       DesignatedInitExpr *DIE, unsigned DesigIdx, QualType &CurrentObjectType,
       RecordDecl::field_iterator *NextField, llvm::APSInt *NextElementIndex,
       unsigned &Index, InitListExpr *StructuredList, unsigned &StructuredIndex,
-      bool FinishSubobjectInit, bool TopLevelObject,
-      llvm::SmallPtrSetImpl<FieldDecl *> *InitializedFields = nullptr);
+      bool FinishSubobjectInit, bool TopLevelObject);
   InitListExpr *getStructuredSubobjectInit(InitListExpr *IList, unsigned Index,
                                            QualType CurrentObjectType,
                                            InitListExpr *StructuredList,
@@ -461,7 +460,8 @@ class InitListChecker {
   void FillInEmptyInitForField(unsigned Init, FieldDecl *Field,
                                const InitializedEntity &ParentEntity,
                                InitListExpr *ILE, bool &RequiresSecondPass,
-                               bool FillWithNoInit = false);
+                               bool FillWithNoInit = false,
+                               bool MaybeEmitMFIWarning = true);
   void FillInEmptyInitializations(const InitializedEntity &Entity,
                                   InitListExpr *ILE, bool &RequiresSecondPass,
                                   InitListExpr *OuterILE, unsigned OuterIndex,
@@ -650,11 +650,17 @@ void InitListChecker::FillInEmptyInitForBase(
   }
 }
 
-void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field,
-                                        const InitializedEntity &ParentEntity,
-                                              InitListExpr *ILE,
-                                              bool &RequiresSecondPass,
-                                              bool FillWithNoInit) {
+static bool hasAnyDesignatedInits(const InitListExpr *IL) {
+  for (const Stmt *Init : *IL)
+    if (isa_and_nonnull<DesignatedInitExpr>(Init))
+      return true;
+  return false;
+}
+
+void InitListChecker::FillInEmptyInitForField(
+    unsigned Init, FieldDecl *Field, const InitializedEntity &ParentEntity,
+    InitListExpr *ILE, bool &RequiresSecondPass, bool FillWithNoInit,
+    bool MaybeEmitMFIWarning) {
   SourceLocation Loc = ILE->getEndLoc();
   unsigned NumInits = ILE->getNumInits();
   InitializedEntity MemberEntity
@@ -723,6 +729,44 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field,
     if (hadError || VerifyOnly) {
       // Do nothing
     } else if (Init < NumInits) {
+      if (MaybeEmitMFIWarning) {
+        auto CheckAnonMember = [&](const FieldDecl *FD,
+                                   auto &&CheckAnonMember) -> bool {
+          FieldDecl *FirstUninitialized = nullptr;
+          RecordDecl *RD = FD->getType()->getAsRecordDecl();
+          assert(RD && "Not anonymous member checked?");
+          for (auto *F : RD->fields()) {
+            bool AllSet = false;
+            if (F->isAnonymousStructOrUnion())
+              AllSet = CheckAnonMember(F, CheckAnonMember);
+
+            if (AllSet || F->hasInClassInitializer()) {
+              if (RD->isUnion())
+                return true;
+              continue;
+            }
+
+            if (!F->isUnnamedBitfield() &&
+                !F->getType()->isIncompleteArrayType() &&
+                !F->isAnonymousStructOrUnion() && !FirstUninitialized)
+              FirstUninitialized = F;
+          }
+
+          if (FirstUninitialized) {
+            SemaRef.Diag(Loc, diag::warn_missing_field_initializers)
+                << FirstUninitialized;
+            return false;
+          }
+          return true;
+        };
+
+        if (Field->isAnonymousStructOrUnion())
+          CheckAnonMember(Field, CheckAnonMember);
+        else if (!Field->isUnnamedBitfield() &&
+                 !Field->getType()->isIncompleteArrayType())
+          SemaRef.Diag(Loc, diag::warn_missing_field_initializers) << Field;
+      }
+
       ILE->setInit(Init, MemberInit.getAs<Expr>());
     } else if (!isa<ImplicitValueInitExpr>(MemberInit.get())) {
       // Empty initialization requires a constructor call, so
@@ -798,9 +842,19 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity,
         }
       }
     } else {
+      InitListExpr *SForm =
+          ILE->isSyntacticForm() ? ILE : ILE->getSyntacticForm();
       // 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.
+
+      // Some checks that required for MFI warning are bound to how many
+      // elements the initializer list originally was provided, perform them
+      // before the list is expanded
+      bool MaybeEmitMFIWarning =
+          !SForm->isIdiomaticZeroInitializer(SemaRef.getLangOpts()) &&
+          ILE->getNumInits() &&
+          !(hasAnyDesignatedInits(SForm) && !SemaRef.getLangOpts().CPlusPlus);
       unsigned NumElems = numStructUnionElements(ILE->getType());
       if (!RDecl->isUnion() && RDecl->hasFlexibleArrayMember())
         ++NumElems;
@@ -828,7 +882,7 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity,
           return;
 
         FillInEmptyInitForField(Init, Field, Entity, ILE, RequiresSecondPass,
-                                FillWithNoInit);
+                                FillWithNoInit, MaybeEmitMFIWarning);
         if (hadError)
           return;
 
@@ -943,13 +997,6 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity,
   }
 }
 
-static bool hasAnyDesignatedInits(const InitListExpr *IL) {
-  for (const Stmt *Init : *IL)
-    if (isa_and_nonnull<DesignatedInitExpr>(Init))
-      return true;
-  return false;
-}
-
 InitListChecker::InitListChecker(
     Sema &S, const InitializedEntity &Entity, InitListExpr *IL, QualType &T,
     bool VerifyOnly, bool TreatUnavailableAsInvalid, bool InOverloadResolution,
@@ -2221,12 +2268,8 @@ void InitListChecker::CheckStructUnionTypes(
   size_t NumRecordDecls = llvm::count_if(RD->decls(), [&](const Decl *D) {
     return isa<FieldDecl>(D) || isa<RecordDecl>(D);
   });
-  bool CheckForMissingFields =
-    !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts());
   bool HasDesignatedInit = false;
 
-  llvm::SmallPtrSet<FieldDecl *, 4> InitializedFields;
-
   while (Index < IList->getNumInits()) {
     Expr *Init = IList->getInit(Index);
     SourceLocation InitLoc = Init->getBeginLoc();
@@ -2244,30 +2287,23 @@ void InitListChecker::CheckStructUnionTypes(
       // the next field that we'll be initializing.
       bool DesignatedInitFailed = CheckDesignatedInitializer(
           Entity, IList, DIE, 0, DeclType, &Field, nullptr, Index,
-          StructuredList, StructuredIndex, true, TopLevelObject,
-          &InitializedFields);
+          StructuredList, StructuredIndex, true, TopLevelObject);
       if (DesignatedInitFailed)
         hadError = true;
 
       // Find the field named by the designated initializer.
       DesignatedInitExpr::Designator *D = DIE->getDesignator(0);
-      if (!VerifyOnly && D->isFieldDesignator()) {
+      if (!VerifyOnly && D->isFieldDesignator() && !DesignatedInitFailed) {
         FieldDecl *F = D->getFieldDecl();
-        if (!DesignatedInitFailed) {
-          QualType ET = SemaRef.Context.getBaseElementType(F->getType());
-          if (checkDestructorReference(ET, InitLoc, SemaRef)) {
-            hadError = true;
-            return;
-          }
+        QualType ET = SemaRef.Context.getBaseElementType(F->getType());
+        if (checkDestructorReference(ET, InitLoc, SemaRef)) {
+          hadError = true;
+          return;
         }
       }
 
       InitializedSomething = true;
 
-      // Disable check for missing fields when designators are used.
-      // This matches gcc behaviour.
-      if (!SemaRef.getLangOpts().CPlusPlus)
-        CheckForMissingFields = false;
       continue;
     }
 
@@ -2346,7 +2382,6 @@ void InitListChecker::CheckStructUnionTypes(
     CheckSubElementType(MemberEntity, IList, Field->getType(), Index,
                         StructuredList, StructuredIndex);
     InitializedSomething = true;
-    InitializedFields.insert(*Field);
 
     if (RD->isUnion() && StructuredList) {
       // Initialize the first field within the union.
@@ -2356,50 +2391,6 @@ void InitListChecker::CheckStructUnionTypes(
     ++Field;
   }
 
-  // Emit warnings for missing struct field initializers.
-  if (!VerifyOnly && InitializedSomething && CheckForMissingFields &&
-      !RD->isUnion()) {
-    // It is possible we have one or more unnamed bitfields remaining.
-    // Find first (if any) named field and emit warning.
-    auto MissingFieldCheck = [&](const RecordDecl *Record,
-                                 RecordDecl::field_iterator StartField,
-                                 auto &&MissingFieldCheck) -> bool {
-      FieldDecl *FirstUninitialized = nullptr;
-      for (RecordDecl::field_iterator it = StartField,
-                                      end = Record->field_end();
-           it != end; ++it) {
-        bool AllSet = false;
-        if (it->isAnonymousStructOrUnion()) {
-          RecordDecl *RDAnon = it->getType()->getAsRecordDecl();
-          AllSet = MissingFieldCheck(RDAnon, RDAnon->field_begin(),
-                                     MissingFieldCheck);
-        }
-
-        if ((HasDesignatedInit && InitializedFields.count(*it)) ||
-            it->hasInClassInitializer() || AllSet) {
-          if (Record->isUnion())
-            return true;
-          continue;
-        }
-
-        if (!it->isUnnamedBitfield() &&
-            !it->getType()->isIncompleteArrayType() &&
-            !it->isAnonymousStructOrUnion() && !FirstUninitialized)
-          FirstUninitialized = *it;
-      }
-
-      if (FirstUninitialized) {
-        SemaRef.Diag(IList->getSourceRange().getEnd(),
-                     diag::warn_missing_field_initializers)
-            << FirstUninitialized;
-        return false;
-      }
-      return true;
-    };
-    MissingFieldCheck(RD, HasDesignatedInit ? RD->field_begin() : Field,
-                      MissingFieldCheck);
-  }
-
   // Check that any remaining fields can be value-initialized if we're not
   // building a structured list. (If we are, we'll check this later.)
   if (!StructuredList && Field != FieldEnd && !RD->isUnion() &&
@@ -2560,8 +2551,7 @@ bool InitListChecker::CheckDesignatedInitializer(
     DesignatedInitExpr *DIE, unsigned DesigIdx, QualType &CurrentObjectType,
     RecordDecl::field_iterator *NextField, llvm::APSInt *NextElementIndex,
     unsigned &Index, InitListExpr *StructuredList, unsigned &StructuredIndex,
-    bool FinishSubobjectInit, bool TopLevelObject,
-    llvm::SmallPtrSetImpl<FieldDecl *> *InitializedFields) {
+    bool FinishSubobjectInit, bool TopLevelObject) {
   if (DesigIdx == DIE->size()) {
     // C++20 designated initialization can result in direct-list-initialization
     // of the designated subobject. This is the only way that we can end up
@@ -2865,11 +2855,8 @@ bool InitListChecker::CheckDesignatedInitializer(
 
 
     // Update the designator with the field declaration.
-    if (!VerifyOnly) {
+    if (!VerifyOnly)
       D->setFieldDecl(*Field);
-      if (InitializedFields)
-        InitializedFields->insert(*Field);
-    }
 
     // Make sure that our non-designated initializer list has space
     // for a subobject corresponding to this field.
@@ -2944,10 +2931,10 @@ bool InitListChecker::CheckDesignatedInitializer(
 
       InitializedEntity MemberEntity =
         InitializedEntity::InitializeMember(*Field, &Entity);
-      if (CheckDesignatedInitializer(
-              MemberEntity, IList, DIE, DesigIdx + 1, FieldType, nullptr,
-              nullptr, Index, StructuredList, newStructuredIndex,
-              FinishSubobjectInit, false, InitializedFields))
+      if (CheckDesignatedInitializer(MemberEntity, IList, DIE, DesigIdx + 1,
+                                     FieldType, nullptr, nullptr, Index,
+                                     StructuredList, newStructuredIndex,
+                                     FinishSubobjectInit, false))
         return true;
     }
 
diff --git a/clang/test/Sema/missing-field-initializers.c b/clang/test/Sema/missing-field-initializers.c
index 1e65b2d62e1ab..8653591ff1187 100644
--- a/clang/test/Sema/missing-field-initializers.c
+++ b/clang/test/Sema/missing-field-initializers.c
@@ -18,7 +18,7 @@ struct Foo bar1[] = {
   1, 2,
   1, 2,
   1
-}; // expected-warning {{missing field 'b' initializer}}
+}; // expected-warning at -1 {{missing field 'b' initializer}}
 
 struct Foo bar2[] = { {}, {}, {} };
 
diff --git a/clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp b/clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
index 87bc01a51d2f2..25aa2c4f5f2cc 100644
--- a/clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
+++ b/clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
@@ -39,6 +39,7 @@ A a1 = {
 };
 int arr[3] = {[1] = 5}; // pedantic-error {{array designators are a C99 extension}}
 B b = {.a.x = 0}; // pedantic-error {{nested designators are a C99 extension}}
+                  // wmissing-warning at -1 {{missing field 'y' initializer}}
 A a2 = {
   .x = 1, // pedantic-error {{mixture of designated and non-designated initializers in the same initializer list is a C99 extension}}
   2 // pedantic-note {{first non-designated initializer is here}}
@@ -60,7 +61,6 @@ B b2 = {.a = 1}; // pedantic-error {{brace elision for designated initializer is
 B b3 = {.a = 1, 2}; // pedantic-error {{mixture of designated and non-designated}} pedantic-note {{first non-designated}} pedantic-error {{brace elision}}
 B b4 = {.a = 1, 2, 3}; // pedantic-error {{mixture of designated and non-designated}} pedantic-note {{first non-designated}} pedantic-error {{brace elision}} expected-error {{excess elements}}
 B b5 = {.a = nullptr}; // expected-error {{cannot initialize}}
-                       // wmissing-warning at -1 {{missing field 'y' initializer}}
 struct C { int :0, x, :0, y, :0; };
 C c = {
   .x = 1, // override-note {{previous}}

>From e67d55eef7887afb20394278a766a8f6201cbeca Mon Sep 17 00:00:00 2001
From: "Podchishchaeva, Mariya" <mariya.podchishchaeva at intel.com>
Date: Tue, 7 Nov 2023 02:58:38 -0800
Subject: [PATCH 3/6] Revert unrelated formatting changes, incorporate review
 feedback

---
 clang/lib/Sema/SemaInit.cpp                   | 104 ++++++++++--------
 .../SemaCXX/cxx2a-initializer-aggregates.cpp  |  22 +++-
 2 files changed, 74 insertions(+), 52 deletions(-)

diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 2aeaa04ed2e91..31df537ecfa35 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -349,12 +349,17 @@ class InitListChecker {
                       bool SubobjectIsDesignatorContext, unsigned &Index,
                       InitListExpr *StructuredList,
                       unsigned &StructuredIndex);
-  bool CheckDesignatedInitializer(
-      const InitializedEntity &Entity, InitListExpr *IList,
-      DesignatedInitExpr *DIE, unsigned DesigIdx, QualType &CurrentObjectType,
-      RecordDecl::field_iterator *NextField, llvm::APSInt *NextElementIndex,
-      unsigned &Index, InitListExpr *StructuredList, unsigned &StructuredIndex,
-      bool FinishSubobjectInit, bool TopLevelObject);
+  bool CheckDesignatedInitializer(const InitializedEntity &Entity,
+                                  InitListExpr *IList, DesignatedInitExpr *DIE,
+                                  unsigned DesigIdx,
+                                  QualType &CurrentObjectType,
+                                  RecordDecl::field_iterator *NextField,
+                                  llvm::APSInt *NextElementIndex,
+                                  unsigned &Index,
+                                  InitListExpr *StructuredList,
+                                  unsigned &StructuredIndex,
+                                  bool FinishSubobjectInit,
+                                  bool TopLevelObject);
   InitListExpr *getStructuredSubobjectInit(InitListExpr *IList, unsigned Index,
                                            QualType CurrentObjectType,
                                            InitListExpr *StructuredList,
@@ -461,7 +466,7 @@ class InitListChecker {
                                const InitializedEntity &ParentEntity,
                                InitListExpr *ILE, bool &RequiresSecondPass,
                                bool FillWithNoInit = false,
-                               bool MaybeEmitMFIWarning = true);
+                               bool WarnIfMissing = true);
   void FillInEmptyInitializations(const InitializedEntity &Entity,
                                   InitListExpr *ILE, bool &RequiresSecondPass,
                                   InitListExpr *OuterILE, unsigned OuterIndex,
@@ -660,7 +665,7 @@ static bool hasAnyDesignatedInits(const InitListExpr *IL) {
 void InitListChecker::FillInEmptyInitForField(
     unsigned Init, FieldDecl *Field, const InitializedEntity &ParentEntity,
     InitListExpr *ILE, bool &RequiresSecondPass, bool FillWithNoInit,
-    bool MaybeEmitMFIWarning) {
+    bool WarnIfMissing) {
   SourceLocation Loc = ILE->getEndLoc();
   unsigned NumInits = ILE->getNumInits();
   InitializedEntity MemberEntity
@@ -729,42 +734,36 @@ void InitListChecker::FillInEmptyInitForField(
     if (hadError || VerifyOnly) {
       // Do nothing
     } else if (Init < NumInits) {
-      if (MaybeEmitMFIWarning) {
+      if (WarnIfMissing) {
         auto CheckAnonMember = [&](const FieldDecl *FD,
-                                   auto &&CheckAnonMember) -> bool {
-          FieldDecl *FirstUninitialized = nullptr;
+                                   auto &&CheckAnonMember) -> FieldDecl * {
+          FieldDecl *Uninitialized = nullptr;
           RecordDecl *RD = FD->getType()->getAsRecordDecl();
           assert(RD && "Not anonymous member checked?");
           for (auto *F : RD->fields()) {
-            bool AllSet = false;
             if (F->isAnonymousStructOrUnion())
-              AllSet = CheckAnonMember(F, CheckAnonMember);
-
-            if (AllSet || F->hasInClassInitializer()) {
-              if (RD->isUnion())
-                return true;
-              continue;
-            }
-
-            if (!F->isUnnamedBitfield() &&
-                !F->getType()->isIncompleteArrayType() &&
-                !F->isAnonymousStructOrUnion() && !FirstUninitialized)
-              FirstUninitialized = F;
-          }
-
-          if (FirstUninitialized) {
-            SemaRef.Diag(Loc, diag::warn_missing_field_initializers)
-                << FirstUninitialized;
-            return false;
+              Uninitialized = CheckAnonMember(F, CheckAnonMember);
+            else if (!F->isUnnamedBitfield() &&
+                     !F->getType()->isIncompleteArrayType() && !Uninitialized &&
+                     !F->hasInClassInitializer())
+              Uninitialized = F;
+
+            if (RD->isUnion() && (F->hasInClassInitializer() || !Uninitialized))
+              return nullptr;
           }
-          return true;
+          return Uninitialized;
         };
 
+        FieldDecl *FieldToDiagnose = nullptr;
         if (Field->isAnonymousStructOrUnion())
-          CheckAnonMember(Field, CheckAnonMember);
+          FieldToDiagnose = CheckAnonMember(Field, CheckAnonMember);
         else if (!Field->isUnnamedBitfield() &&
                  !Field->getType()->isIncompleteArrayType())
-          SemaRef.Diag(Loc, diag::warn_missing_field_initializers) << Field;
+          FieldToDiagnose = Field;
+
+        if (FieldToDiagnose)
+          SemaRef.Diag(Loc, diag::warn_missing_field_initializers)
+              << FieldToDiagnose;
       }
 
       ILE->setInit(Init, MemberInit.getAs<Expr>());
@@ -848,13 +847,19 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity,
       // order to leave them uninitialized, the ILE is expanded and the extra
       // fields are then filled with NoInitExpr.
 
-      // Some checks that required for MFI warning are bound to how many
-      // elements the initializer list originally was provided, perform them
-      // before the list is expanded
-      bool MaybeEmitMFIWarning =
+      // Some checks that required for missing fields warning are bound to how
+      // many elements the initializer list originally was provided, perform
+      // them before the list is expanded.
+      bool WarnIfMissingField =
           !SForm->isIdiomaticZeroInitializer(SemaRef.getLangOpts()) &&
-          ILE->getNumInits() &&
-          !(hasAnyDesignatedInits(SForm) && !SemaRef.getLangOpts().CPlusPlus);
+          ILE->getNumInits();
+
+      // Disable check for missing fields when designators are used in C to
+      // match gcc behaviour.
+      // FIXME: Should we emulate possible gcc warning bug?
+      WarnIfMissingField &=
+          !(!SemaRef.getLangOpts().CPlusPlus && hasAnyDesignatedInits(SForm));
+
       unsigned NumElems = numStructUnionElements(ILE->getType());
       if (!RDecl->isUnion() && RDecl->hasFlexibleArrayMember())
         ++NumElems;
@@ -882,7 +887,7 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity,
           return;
 
         FillInEmptyInitForField(Init, Field, Entity, ILE, RequiresSecondPass,
-                                FillWithNoInit, MaybeEmitMFIWarning);
+                                FillWithNoInit, WarnIfMissingField);
         if (hadError)
           return;
 
@@ -2546,12 +2551,19 @@ class FieldInitializerValidatorCCC final : public CorrectionCandidateCallback {
 /// actually be initialized.
 ///
 /// @returns true if there was an error, false otherwise.
-bool InitListChecker::CheckDesignatedInitializer(
-    const InitializedEntity &Entity, InitListExpr *IList,
-    DesignatedInitExpr *DIE, unsigned DesigIdx, QualType &CurrentObjectType,
-    RecordDecl::field_iterator *NextField, llvm::APSInt *NextElementIndex,
-    unsigned &Index, InitListExpr *StructuredList, unsigned &StructuredIndex,
-    bool FinishSubobjectInit, bool TopLevelObject) {
+bool
+InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity,
+                                            InitListExpr *IList,
+                                            DesignatedInitExpr *DIE,
+                                            unsigned DesigIdx,
+                                            QualType &CurrentObjectType,
+                                          RecordDecl::field_iterator *NextField,
+                                            llvm::APSInt *NextElementIndex,
+                                            unsigned &Index,
+                                            InitListExpr *StructuredList,
+                                            unsigned &StructuredIndex,
+                                            bool FinishSubobjectInit,
+                                            bool TopLevelObject) {
   if (DesigIdx == DIE->size()) {
     // C++20 designated initialization can result in direct-list-initialization
     // of the designated subobject. This is the only way that we can end up
diff --git a/clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp b/clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
index 25aa2c4f5f2cc..bb8d9facd7e38 100644
--- a/clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
+++ b/clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
@@ -1,10 +1,10 @@
 // RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,pedantic,override,reorder -pedantic-errors
 // RUN: %clang_cc1 -std=c++17 %s -verify=expected,pedantic,override,reorder -Wno-c++20-designator -pedantic-errors
-// RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,pedantic -Werror=c99-designator -Wno-reorder-init-list -Wno-initializer-overrides
+// RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,pedantic -Werror=c99-designator -Wno-reorder-init-list -Wno-initializer-overrides -Werror=nested-anon-types -Werror=gnu-anonymous-struct
 // RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,reorder -Wno-c99-designator -Werror=reorder-init-list -Wno-initializer-overrides
 // RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,override -Wno-c99-designator -Wno-reorder-init-list -Werror=initializer-overrides
 // RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected -Wno-c99-designator -Wno-reorder-init-list -Wno-initializer-overrides
-// RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,wmissing -Wmissing-field-initializers -Wno-c99-designator -Wno-reorder-init-list -Wno-initializer-overrides -D NON_PEDANTIC
+// RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,wmissing -Wmissing-field-initializers -Wno-c99-designator -Wno-reorder-init-list -Wno-initializer-overrides
 
 
 namespace class_with_ctor {
@@ -284,23 +284,25 @@ void foo() {
   B bb = {1}; // wmissing-warning {{missing field 'b' initializer}}
               // wmissing-warning at -1 {{missing field 'a' initializer}}
   C c = {.a = 1}; // wmissing-warning {{missing field 'b' initializer}}
-  CC cc = {.a = 1}; //// wmissing-warning {{missing field 'c' initializer}}
+  CC cc = {.a = 1}; // wmissing-warning {{missing field 'c' initializer}}
 }
 
-#if defined NON_PEDANTIC
 struct C1 {
   int m;
   union { float b; union {int n = 1; }; };
+  // pedantic-error at -1 {{anonymous types declared in an anonymous union are an extension}}
 };
 
 struct C2 {
   int m;
-  struct { float b; int n = 1; };
+  struct { float b; int n = 1; }; // pedantic-error {{anonymous structs are a GNU extension}}
 };
 
 struct C3 {
   int m;
   struct { float b = 1; union {int a;}; int n = 1; };
+  // pedantic-error at -1 {{anonymous structs are a GNU extension}}
+  // pedantic-error at -2 {{anonymous types declared in an anonymous struct are an extension}}
 };
 
 C1 c = C1{.m = 1};
@@ -309,5 +311,13 @@ C2 c1 = C2{.m = 1}; // wmissing-warning {{missing field 'b' initializer}}
 C2 c22 = C2{.m = 1, .b = 1};
 C3 c2 = C3{.b = 1}; // wmissing-warning {{missing field 'a' initializer}}
                     // wmissing-warning at -1 {{missing field 'm' initializer}}
-#endif // NON_PEDANTIC
+
+struct C4 {
+  union {
+    struct { int n; }; // pedantic-error {{anonymous structs are a GNU extension}}
+    // pedantic-error at -1 {{anonymous types declared in an anonymous union are an extension}}
+    int m = 0; };
+  int z;
+};
+C4 a = {.z = 1};
 }

>From 6b46b540a84f560bd3bf3f9fe2fc41467841e637 Mon Sep 17 00:00:00 2001
From: "Podchishchaeva, Mariya" <mariya.podchishchaeva at intel.com>
Date: Tue, 14 Nov 2023 02:38:13 -0800
Subject: [PATCH 4/6] Incorporate review feedback

---
 clang/lib/Sema/SemaInit.cpp                    | 18 ++++++++++--------
 .../SemaCXX/cxx2a-initializer-aggregates.cpp   | 10 ++++++++++
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 31df537ecfa35..f1a661a2a63bb 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -656,10 +656,9 @@ void InitListChecker::FillInEmptyInitForBase(
 }
 
 static bool hasAnyDesignatedInits(const InitListExpr *IL) {
-  for (const Stmt *Init : *IL)
-    if (isa_and_nonnull<DesignatedInitExpr>(Init))
-      return true;
-  return false;
+  return llvm::any_of(*IL, [=](const Stmt *Init) {
+    return isa_and_nonnull<DesignatedInitExpr>(Init);
+  });
 }
 
 void InitListChecker::FillInEmptyInitForField(
@@ -741,15 +740,18 @@ void InitListChecker::FillInEmptyInitForField(
           RecordDecl *RD = FD->getType()->getAsRecordDecl();
           assert(RD && "Not anonymous member checked?");
           for (auto *F : RD->fields()) {
+            FieldDecl *UninitializedFieldInF = nullptr;
             if (F->isAnonymousStructOrUnion())
-              Uninitialized = CheckAnonMember(F, CheckAnonMember);
+              UninitializedFieldInF = CheckAnonMember(F, CheckAnonMember);
             else if (!F->isUnnamedBitfield() &&
-                     !F->getType()->isIncompleteArrayType() && !Uninitialized &&
+                     !F->getType()->isIncompleteArrayType() &&
                      !F->hasInClassInitializer())
-              Uninitialized = F;
+              UninitializedFieldInF = F;
 
-            if (RD->isUnion() && (F->hasInClassInitializer() || !Uninitialized))
+            if (RD->isUnion() && !UninitializedFieldInF)
               return nullptr;
+            if (!Uninitialized)
+              Uninitialized = UninitializedFieldInF;
           }
           return Uninitialized;
         };
diff --git a/clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp b/clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
index bb8d9facd7e38..0d977e07ed034 100644
--- a/clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
+++ b/clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
@@ -320,4 +320,14 @@ struct C4 {
   int z;
 };
 C4 a = {.z = 1};
+
+struct C5 {
+  int a;
+  struct { // pedantic-error {{anonymous structs are a GNU extension}}
+    int x;
+    struct { int y = 0; };  // pedantic-error {{anonymous types declared in an anonymous struct are an extension}}
+                            // pedantic-error at -1 {{anonymous structs are a GNU extension}}
+  };
+};
+C5 c5 = C5{.a = 0}; //wmissing-warning {{missing field 'x' initializer}}
 }

>From 329a036839e1bdbda41551bf381a9bc090468d25 Mon Sep 17 00:00:00 2001
From: "Podchishchaeva, Mariya" <mariya.podchishchaeva at intel.com>
Date: Thu, 23 Nov 2023 06:16:20 -0800
Subject: [PATCH 5/6] Warn unconditionally

---
 clang/lib/Sema/SemaInit.cpp | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index f1a661a2a63bb..44e45b5ce0c54 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -466,7 +466,7 @@ class InitListChecker {
                                const InitializedEntity &ParentEntity,
                                InitListExpr *ILE, bool &RequiresSecondPass,
                                bool FillWithNoInit = false,
-                               bool WarnIfMissing = true);
+                               bool WarnIfMissing = false);
   void FillInEmptyInitializations(const InitializedEntity &Entity,
                                   InitListExpr *ILE, bool &RequiresSecondPass,
                                   InitListExpr *OuterILE, unsigned OuterIndex,
@@ -732,7 +732,7 @@ void InitListChecker::FillInEmptyInitForField(
 
     if (hadError || VerifyOnly) {
       // Do nothing
-    } else if (Init < NumInits) {
+    } else {
       if (WarnIfMissing) {
         auto CheckAnonMember = [&](const FieldDecl *FD,
                                    auto &&CheckAnonMember) -> FieldDecl * {
@@ -768,14 +768,16 @@ void InitListChecker::FillInEmptyInitForField(
               << FieldToDiagnose;
       }
 
-      ILE->setInit(Init, MemberInit.getAs<Expr>());
-    } else if (!isa<ImplicitValueInitExpr>(MemberInit.get())) {
-      // Empty initialization requires a constructor call, so
-      // extend the initializer list to include the constructor
-      // call and make a note that we'll need to take another pass
-      // through the initializer list.
-      ILE->updateInit(SemaRef.Context, Init, MemberInit.getAs<Expr>());
-      RequiresSecondPass = true;
+      if (Init < NumInits) {
+        ILE->setInit(Init, MemberInit.getAs<Expr>());
+      } else if (!isa<ImplicitValueInitExpr>(MemberInit.get())) {
+        // Empty initialization requires a constructor call, so
+        // extend the initializer list to include the constructor
+        // call and make a note that we'll need to take another pass
+        // through the initializer list.
+        ILE->updateInit(SemaRef.Context, Init, MemberInit.getAs<Expr>());
+        RequiresSecondPass = true;
+      }
     }
   } else if (InitListExpr *InnerILE
                = dyn_cast<InitListExpr>(ILE->getInit(Init))) {

>From 1b9b15b9d72692123e6e37ba57ed8208b61cd1a8 Mon Sep 17 00:00:00 2001
From: "Podchishchaeva, Mariya" <mariya.podchishchaeva at intel.com>
Date: Thu, 7 Dec 2023 02:04:52 -0800
Subject: [PATCH 6/6] Apply suggestions

---
 clang/lib/Sema/SemaInit.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index ef7de04116142..7d0947f08955f 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -851,8 +851,8 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity,
       // order to leave them uninitialized, the ILE is expanded and the extra
       // fields are then filled with NoInitExpr.
 
-      // Some checks that required for missing fields warning are bound to how
-      // many elements the initializer list originally was provided, perform
+      // Some checks that are required for missing fields warning are bound to
+      // how many elements the initializer list originally was provided; perform
       // them before the list is expanded.
       bool WarnIfMissingField =
           !SForm->isIdiomaticZeroInitializer(SemaRef.getLangOpts()) &&
@@ -862,7 +862,7 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity,
       // match gcc behaviour.
       // FIXME: Should we emulate possible gcc warning bug?
       WarnIfMissingField &=
-          !(!SemaRef.getLangOpts().CPlusPlus && hasAnyDesignatedInits(SForm));
+          SemaRef.getLangOpts().CPlusPlus || !hasAnyDesignatedInits(SForm);
 
       unsigned NumElems = numStructUnionElements(ILE->getType());
       if (!RDecl->isUnion() && RDecl->hasFlexibleArrayMember())



More information about the cfe-commits mailing list