From 96bc78631f16fe5ce2e7e6000b74d790b32f7a16 Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour <shafik.yaghmour@intel.com>
Date: Mon, 8 May 2023 11:13:08 -0700
Subject: [PATCH] [Clang] Update warning on some designator initializer cases
 involving 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: https://github.com/llvm/llvm-project/issues/62156

Differential Revision:  https://reviews.llvm.org/D149694
---
 clang/include/clang/Basic/DiagnosticSemaKinds.td   |  2 ++
 clang/lib/Sema/SemaInit.cpp                        | 15 ++++++++++----
 .../test/SemaCXX/cxx2b-designated-initializers.cpp | 24 ++++++++++++++++++++++
 3 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c36828f..1d30d6f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -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">;
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index d27cd6b..3db7022 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -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
@@ -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) {
@@ -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
diff --git a/clang/test/SemaCXX/cxx2b-designated-initializers.cpp b/clang/test/SemaCXX/cxx2b-designated-initializers.cpp
index 3774565..5588926f 100644
--- a/clang/test/SemaCXX/cxx2b-designated-initializers.cpp
+++ b/clang/test/SemaCXX/cxx2b-designated-initializers.cpp
@@ -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}}
+}
+}
-- 
2.7.4