Skip to content

Commit

Permalink
[clang] Fix false positive -Wmissing-field-initializer for anonymous …
Browse files Browse the repository at this point in the history
…unions (#70829)

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 #70384
  • Loading branch information
Fznamznon committed Dec 13, 2023
1 parent b003fed commit a01307a
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 65 deletions.
142 changes: 80 additions & 62 deletions clang/lib/Sema/SemaInit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,8 @@ class InitListChecker {
void FillInEmptyInitForField(unsigned Init, FieldDecl *Field,
const InitializedEntity &ParentEntity,
InitListExpr *ILE, bool &RequiresSecondPass,
bool FillWithNoInit = false);
bool FillWithNoInit = false,
bool WarnIfMissing = false);
void FillInEmptyInitializations(const InitializedEntity &Entity,
InitListExpr *ILE, bool &RequiresSecondPass,
InitListExpr *OuterILE, unsigned OuterIndex,
Expand Down Expand Up @@ -654,11 +655,16 @@ 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) {
return llvm::any_of(*IL, [=](const Stmt *Init) {
return isa_and_nonnull<DesignatedInitExpr>(Init);
});
}

void InitListChecker::FillInEmptyInitForField(
unsigned Init, FieldDecl *Field, const InitializedEntity &ParentEntity,
InitListExpr *ILE, bool &RequiresSecondPass, bool FillWithNoInit,
bool WarnIfMissing) {
SourceLocation Loc = ILE->getEndLoc();
unsigned NumInits = ILE->getNumInits();
InitializedEntity MemberEntity
Expand Down Expand Up @@ -726,15 +732,52 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field,

if (hadError || VerifyOnly) {
// Do nothing
} else 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 (WarnIfMissing) {
auto CheckAnonMember = [&](const FieldDecl *FD,
auto &&CheckAnonMember) -> FieldDecl * {
FieldDecl *Uninitialized = nullptr;
RecordDecl *RD = FD->getType()->getAsRecordDecl();
assert(RD && "Not anonymous member checked?");
for (auto *F : RD->fields()) {
FieldDecl *UninitializedFieldInF = nullptr;
if (F->isAnonymousStructOrUnion())
UninitializedFieldInF = CheckAnonMember(F, CheckAnonMember);
else if (!F->isUnnamedBitfield() &&
!F->getType()->isIncompleteArrayType() &&
!F->hasInClassInitializer())
UninitializedFieldInF = F;

if (RD->isUnion() && !UninitializedFieldInF)
return nullptr;
if (!Uninitialized)
Uninitialized = UninitializedFieldInF;
}
return Uninitialized;
};

FieldDecl *FieldToDiagnose = nullptr;
if (Field->isAnonymousStructOrUnion())
FieldToDiagnose = CheckAnonMember(Field, CheckAnonMember);
else if (!Field->isUnnamedBitfield() &&
!Field->getType()->isIncompleteArrayType())
FieldToDiagnose = Field;

if (FieldToDiagnose)
SemaRef.Diag(Loc, diag::warn_missing_field_initializers)
<< FieldToDiagnose;
}

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))) {
Expand Down Expand Up @@ -802,9 +845,25 @@ 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 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()) &&
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;
Expand Down Expand Up @@ -832,7 +891,7 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity,
return;

FillInEmptyInitForField(Init, Field, Entity, ILE, RequiresSecondPass,
FillWithNoInit);
FillWithNoInit, WarnIfMissingField);
if (hadError)
return;

Expand Down Expand Up @@ -947,13 +1006,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,
Expand Down Expand Up @@ -2225,12 +2277,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();
Expand All @@ -2254,24 +2302,17 @@ void InitListChecker::CheckStructUnionTypes(

// 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();
InitializedFields.insert(F);
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;
}

Expand Down Expand Up @@ -2350,7 +2391,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.
Expand All @@ -2360,28 +2400,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.
for (RecordDecl::field_iterator it = HasDesignatedInit ? RD->field_begin()
: Field,
end = RD->field_end();
it != end; ++it) {
if (HasDesignatedInit && InitializedFields.count(*it))
continue;

if (!it->isUnnamedBitfield() && !it->hasInClassInitializer() &&
!it->getType()->isIncompleteArrayType()) {
SemaRef.Diag(IList->getSourceRange().getEnd(),
diag::warn_missing_field_initializers)
<< *it;
break;
}
}
}

// 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() &&
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Sema/missing-field-initializers.c
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = { {}, {}, {} };

Expand Down
88 changes: 86 additions & 2 deletions clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// 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
Expand Down Expand Up @@ -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}}
Expand All @@ -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}}
Expand Down Expand Up @@ -247,3 +247,87 @@ 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}}
}

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; }; // 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};
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}}

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};

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}}
}

0 comments on commit a01307a

Please sign in to comment.