[PATCH] D149694: [Clang] Update warning on some designator initializer cases involving unions

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik created this revision.
shafik added reviewers: rsmith, aaron.ballman, erichkeane.
Herald added a project: All.
shafik requested review of this revision.

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


https://reviews.llvm.org/D149694

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaCXX/cxx2b-designated-initializers.cpp


Index: clang/test/SemaCXX/cxx2b-designated-initializers.cpp
===================================================================
--- clang/test/SemaCXX/cxx2b-designated-initializers.cpp
+++ clang/test/SemaCXX/cxx2b-designated-initializers.cpp
@@ -19,3 +19,27 @@
 }
 
 } // 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}}
+}
+}
Index: clang/lib/Sema/SemaInit.cpp
===================================================================
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -393,12 +393,15 @@
 
   /// 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
@@ -2544,6 +2547,7 @@
         // subobject [0].b.
         diagnoseInitOverride(ExistingInit,
                              SourceRange(D->getBeginLoc(), DIE->getEndLoc()),
+                             /*UnionOverride=*/false,
                              /*FullyOverwritten=*/false);
 
         if (!VerifyOnly) {
@@ -2689,7 +2693,9 @@
           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
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -186,6 +186,8 @@
   "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">;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D149694.518862.patch
Type: text/x-patch
Size: 3668 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230502/1784eef8/attachment.bin>