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.podchishcha...@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@-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@-1 {{missing field 'm' initializer}} +#endif // NON_PEDANTIC +} >From b68dad578a757b3f3412ef70a13045f107b14c1f Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <mariya.podchishcha...@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@-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@-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@-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.podchishcha...@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@-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@-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@-1 {{anonymous structs are a GNU extension}} + // pedantic-error@-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@-1 {{missing field 'm' initializer}} -#endif // NON_PEDANTIC + +struct C4 { + union { + struct { int n; }; // pedantic-error {{anonymous structs are a GNU extension}} + // pedantic-error@-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.podchishcha...@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@-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.podchishcha...@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.podchishcha...@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()) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
