Skip to content

Commit

Permalink
[Clang] Update warning on some designator initializer cases involving…
Browse files Browse the repository at this point in the history
… unions

Currently when using designated initializers in C++ we have a few
extension. Two extension which are dangerous involved assigning to
multiple members of union which will likely be a mistake since unions
can only have one active member. I have updated to be a warning by
default.

The second case if when we assign to multiple union members and one of
the previous members had a non-trivial destructor, which could lead to
leaking resources. This one is now an error by default.

Fixes: #62156

Differential Revision:  https://reviews.llvm.org/D149694
  • Loading branch information
shafik committed May 8, 2023
1 parent 0dec49c commit 96bc786
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 4 deletions.
2 changes: 2 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Expand Up @@ -186,6 +186,8 @@ def warn_initializer_overrides : Warning<
"this subobject">, InGroup<InitializerOverrides>;
def ext_initializer_overrides : ExtWarn<warn_initializer_overrides.Summary>,
InGroup<InitializerOverrides>, SFINAEFailure;
def ext_initializer_union_overrides : ExtWarn<warn_initializer_overrides.Summary>,
InGroup<InitializerOverrides>, DefaultError, SFINAEFailure;
def err_initializer_overrides_destructed : Error<
"initializer would partially override prior initialization of object of "
"type %1 with non-trivial destruction">;
Expand Down
15 changes: 11 additions & 4 deletions clang/lib/Sema/SemaInit.cpp
Expand Up @@ -394,12 +394,15 @@ class InitListChecker {

/// Diagnose that OldInit (or part thereof) has been overridden by NewInit.
void diagnoseInitOverride(Expr *OldInit, SourceRange NewInitRange,
bool UnionOverride = false,
bool FullyOverwritten = true) {
// Overriding an initializer via a designator is valid with C99 designated
// initializers, but ill-formed with C++20 designated initializers.
unsigned DiagID = SemaRef.getLangOpts().CPlusPlus
? diag::ext_initializer_overrides
: diag::warn_initializer_overrides;
unsigned DiagID =
SemaRef.getLangOpts().CPlusPlus
? (UnionOverride ? diag::ext_initializer_union_overrides
: diag::ext_initializer_overrides)
: diag::warn_initializer_overrides;

if (InOverloadResolution && SemaRef.getLangOpts().CPlusPlus) {
// In overload resolution, we have to strictly enforce the rules, and so
Expand Down Expand Up @@ -2546,6 +2549,7 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity,
// subobject [0].b.
diagnoseInitOverride(ExistingInit,
SourceRange(D->getBeginLoc(), DIE->getEndLoc()),
/*UnionOverride=*/false,
/*FullyOverwritten=*/false);

if (!VerifyOnly) {
Expand Down Expand Up @@ -2691,7 +2695,10 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity,
if (ExistingInit) {
// We're about to throw away an initializer, emit warning.
diagnoseInitOverride(
ExistingInit, SourceRange(D->getBeginLoc(), DIE->getEndLoc()));
ExistingInit, SourceRange(D->getBeginLoc(), DIE->getEndLoc()),
/*UnionOverride=*/true,
/*FullyOverwritten=*/SemaRef.getLangOpts().CPlusPlus ? false
: true);
}

// remove existing initializer
Expand Down
24 changes: 24 additions & 0 deletions clang/test/SemaCXX/cxx2b-designated-initializers.cpp
Expand Up @@ -19,3 +19,27 @@ void g(void) {
}

} // end namespace PR61118

namespace GH62156 {
union U1 {
int x;
float y;
};

struct NonTrivial {
NonTrivial();
~NonTrivial();
};

union U2 {
NonTrivial x;
float y;
};

void f() {
U1 u{.x=2, // expected-note {{previous initialization is here}}
.y=1}; // expected-error {{initializer partially overrides prior initialization of this subobject}}
new U2{.x = NonTrivial{}, // expected-note {{previous initialization is here}}
.y=1}; // expected-error {{initializer would partially override prior initialization of object of type 'NonTrivial' with non-trivial destruction}}
}
}

0 comments on commit 96bc786

Please sign in to comment.