Skip to content

Conversation

shafik
Copy link
Collaborator

@shafik shafik commented Feb 20, 2024

In 765d8a1 we impelemented a fix for incorrect deletion of default constructors in unions. This fix missed a case and so this PR will extend the fix to cover the additional case.

Fixes: #81774

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 20, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 20, 2024

@llvm/pr-subscribers-clang

Author: Shafik Yaghmour (shafik)

Changes

In 765d8a1 we impelemented a fix for incorrect deletion of default constructors in unions. This fix missed a case and so this PR will extend the fix to cover the additional case.

Fixes: #81774


Full diff: https://github.com/llvm/llvm-project/pull/82407.diff

4 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+3)
  • (modified) clang/lib/Sema/SemaDeclCXX.cpp (+15-3)
  • (modified) clang/test/CodeGen/union-non-trivial-member.cpp (+17)
  • (modified) clang/test/SemaCXX/cxx0x-nontrivial-union.cpp (+11)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5bca2c965c866b..452382eb6c4a1e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -292,6 +292,9 @@ Bug Fixes to C++ Support
   was only accepted at namespace scope but not at local function scope.
 - Clang no longer tries to call consteval constructors at runtime when they appear in a member initializer.
   (`#782154 <https://github.com/llvm/llvm-project/issues/82154>`_`)
+- Fix for clang incorrectly rejecting the default construction of a union with
+  nontrivial member when another member has an initializer.
+  (`#81774 <https://github.com/llvm/llvm-project/issues/81774>`_)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 79263bc3ff671d..25a4b4381ca25e 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -9442,9 +9442,21 @@ bool SpecialMemberDeletionInfo::shouldDeleteForSubobjectCall(
 
   int DiagKind = -1;
 
-  if (SMOR.getKind() == Sema::SpecialMemberOverloadResult::NoMemberOrDeleted)
-    DiagKind = !Decl ? 0 : 1;
-  else if (SMOR.getKind() == Sema::SpecialMemberOverloadResult::Ambiguous)
+  if (SMOR.getKind() == Sema::SpecialMemberOverloadResult::NoMemberOrDeleted) {
+    if (CSM == Sema::CXXDefaultConstructor && Field &&
+        Field->getParent()->isUnion()) {
+      // [class.default.ctor]p2:
+      //   A defaulted default constructor for class X is defined as deleted if
+      //   - X is a union that has a variant member with a non-trivial default
+      //     constructor and no variant member of X has a default member
+      //     initializer
+      const auto *RD = cast<CXXRecordDecl>(Field->getParent());
+      if (!RD->hasInClassInitializer())
+        DiagKind = !Decl ? 0 : 1;
+    } else {
+      DiagKind = !Decl ? 0 : 1;
+    }
+  } else if (SMOR.getKind() == Sema::SpecialMemberOverloadResult::Ambiguous)
     DiagKind = 2;
   else if (!isAccessible(Subobj, Decl))
     DiagKind = 3;
diff --git a/clang/test/CodeGen/union-non-trivial-member.cpp b/clang/test/CodeGen/union-non-trivial-member.cpp
index fdc9fd16911e14..8b055a9970fc75 100644
--- a/clang/test/CodeGen/union-non-trivial-member.cpp
+++ b/clang/test/CodeGen/union-non-trivial-member.cpp
@@ -15,14 +15,25 @@ union UnionNonTrivial {
     non_trivial_constructor b{};
 };
 
+struct Handle {
+    Handle(int) {}
+};
+
+union UnionNonTrivialEqualInit {
+    int NoState = 0;
+    Handle CustomState;
+};
+
 void f() {
     UnionInt u1;
     UnionNonTrivial u2;
+    UnionNonTrivialEqualInit u3;
 }
 
 // CHECK:      define dso_local void @_Z1fv()
 // CHECK:        call void @_ZN8UnionIntC1Ev
 // CHECK-NEXT:   call void @_ZN15UnionNonTrivialC1Ev
+// CHECK-NEXT:   call void @_ZN24UnionNonTrivialEqualInitC1Ev
 
 // CHECK:      define {{.*}}void @_ZN8UnionIntC1Ev
 // CHECK:        call void @_ZN8UnionIntC2Ev
@@ -30,8 +41,14 @@ void f() {
 // CHECK:      define {{.*}}void @_ZN15UnionNonTrivialC1Ev
 // CHECK:        call void @_ZN15UnionNonTrivialC2Ev
 
+// CHECK:      define {{.*}}void @_ZN24UnionNonTrivialEqualInitC1Ev
+// CHECK:        call void @_ZN24UnionNonTrivialEqualInitC2Ev
+
 // CHECK:      define {{.*}}void @_ZN8UnionIntC2Ev
 // CHECK:        store i32 1000
 
 // CHECK:      define {{.*}}void @_ZN15UnionNonTrivialC2Ev
 // CHECK:        call void @_ZN23non_trivial_constructorC1Ev
+
+// CHECK:      define {{.*}}void @_ZN24UnionNonTrivialEqualInitC2Ev
+// CHECK:        store i32 0
diff --git a/clang/test/SemaCXX/cxx0x-nontrivial-union.cpp b/clang/test/SemaCXX/cxx0x-nontrivial-union.cpp
index c7cdf76d850dbe..141fe96f512471 100644
--- a/clang/test/SemaCXX/cxx0x-nontrivial-union.cpp
+++ b/clang/test/SemaCXX/cxx0x-nontrivial-union.cpp
@@ -188,3 +188,14 @@ static_assert(U2().b.x == 100, "");
 static_assert(U3().b.x == 100, "");
 
 } // namespace GH48416
+
+namespace GH81774 {
+struct Handle {
+    Handle(int) {}
+};
+// Should be well-formed b/c NoState has a brace-or-equal-initializer
+union a {
+        int NoState = 0;
+        Handle CustomState;
+} b;
+} // namespace GH81774

…ith nontrivial member

In 765d8a1 we impelemented a fix for incorrect deletion of
default constructors in unions. This fix missed a case and so this PR will
extend the fix to cover the additional case.

Fixes: llvm#81774
@shafik shafik force-pushed the fixdincorrectdelofctoronsomeunionspart2 branch 2 times, most recently from e1bf9ef to 9894851 Compare February 22, 2024 02:20
@shafik shafik force-pushed the fixdincorrectdelofctoronsomeunionspart2 branch from 9894851 to af9639f Compare February 22, 2024 05:55
@shafik shafik force-pushed the fixdincorrectdelofctoronsomeunionspart2 branch from ab431f4 to 3e5db7b Compare February 24, 2024 05:15
@MitalAshok
Copy link
Contributor

This is CWG2084. Could you also add some tests to CXX/drs/ so that www/cxx_dr_status.html can be updated?

@cor3ntin
Copy link
Contributor

cor3ntin commented Oct 3, 2025

This is CWG2084. Could you also add some tests to CXX/drs/ so that www/cxx_dr_status.html can be updated?

Can you address that? thanks!

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@shafik shafik merged commit 435d084 into llvm:main Oct 6, 2025
10 checks passed
@void2012
Copy link

void2012 commented Oct 7, 2025

@shafik Can this be safely backported to the upcoming LLVM 21.x releases?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clang incorrectly rejects default construction of union with nontrivial member, part 2
7 participants