Skip to content

[clang] Fix false positive -Wmissing-field-initializer for anonymous unions #70829

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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}}
}