royjacobson created this revision. Herald added a project: All. royjacobson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
If a union has explicit initializers for some members, we shouldn't delete its default constructor. Fixes https://github.com/llvm/llvm-project/issues/48416. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D145851 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaDeclCXX.cpp clang/test/SemaCXX/cxx0x-nontrivial-union.cpp Index: clang/test/SemaCXX/cxx0x-nontrivial-union.cpp =================================================================== --- clang/test/SemaCXX/cxx0x-nontrivial-union.cpp +++ clang/test/SemaCXX/cxx0x-nontrivial-union.cpp @@ -144,3 +144,37 @@ Test2<X> t2x; // expected-error {{call to implicitly-deleted default constructor of 'Test2<X>'}} } + +namespace GH48416 { + +struct non_trivial_constructor { + non_trivial_constructor() {} +}; + + +union U1 { + int a; + non_trivial_constructor b; // expected-note {{has a non-trivial default constructor}} +}; + +union U2 { + int a{}; + non_trivial_constructor b; +}; + +union U3 { + int a; + non_trivial_constructor b{}; +}; + +union U4 { + int a{}; // expected-note {{previous initialization is here}} + non_trivial_constructor b{}; // expected-error {{initializing multiple members of union}} +}; + +U1 u1; // expected-error {{call to implicitly-deleted default constructor}} +U2 u2; +U3 u3; +U4 u4; + +} // namespace GH48416 Index: clang/lib/Sema/SemaDeclCXX.cpp =================================================================== --- clang/lib/Sema/SemaDeclCXX.cpp +++ clang/lib/Sema/SemaDeclCXX.cpp @@ -9157,7 +9157,15 @@ // must be accessible and non-deleted, but need not be trivial. Such a // destructor is never actually called, but is semantically checked as // if it were. - DiagKind = 4; + if (CSM == Sema::CXXDefaultConstructor) { + // Default constructor of a union can also be valid if one of the + // members has an explicit initializer. + auto* Constructor = dyn_cast<CXXConstructorDecl>(Decl); + auto* RD = dyn_cast<CXXRecordDecl>(Field->getParent()); + if (!Constructor || !RD || !RD->hasInClassInitializer()) + DiagKind = 4; + } else + DiagKind = 4; } if (DiagKind == -1) Index: clang/docs/ReleaseNotes.rst =================================================================== --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -202,6 +202,8 @@ - Fix crash when evaluating consteval constructor of derived class whose base has more than one field. (`#60166 <https://github.com/llvm/llvm-project/issues/60166>`_) +- Fix incorrect deletion of the default constructor of unions in some + cases. (`#48416 <https://github.com/llvm/llvm-project/issues/48416>`_) Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^
Index: clang/test/SemaCXX/cxx0x-nontrivial-union.cpp =================================================================== --- clang/test/SemaCXX/cxx0x-nontrivial-union.cpp +++ clang/test/SemaCXX/cxx0x-nontrivial-union.cpp @@ -144,3 +144,37 @@ Test2<X> t2x; // expected-error {{call to implicitly-deleted default constructor of 'Test2<X>'}} } + +namespace GH48416 { + +struct non_trivial_constructor { + non_trivial_constructor() {} +}; + + +union U1 { + int a; + non_trivial_constructor b; // expected-note {{has a non-trivial default constructor}} +}; + +union U2 { + int a{}; + non_trivial_constructor b; +}; + +union U3 { + int a; + non_trivial_constructor b{}; +}; + +union U4 { + int a{}; // expected-note {{previous initialization is here}} + non_trivial_constructor b{}; // expected-error {{initializing multiple members of union}} +}; + +U1 u1; // expected-error {{call to implicitly-deleted default constructor}} +U2 u2; +U3 u3; +U4 u4; + +} // namespace GH48416 Index: clang/lib/Sema/SemaDeclCXX.cpp =================================================================== --- clang/lib/Sema/SemaDeclCXX.cpp +++ clang/lib/Sema/SemaDeclCXX.cpp @@ -9157,7 +9157,15 @@ // must be accessible and non-deleted, but need not be trivial. Such a // destructor is never actually called, but is semantically checked as // if it were. - DiagKind = 4; + if (CSM == Sema::CXXDefaultConstructor) { + // Default constructor of a union can also be valid if one of the + // members has an explicit initializer. + auto* Constructor = dyn_cast<CXXConstructorDecl>(Decl); + auto* RD = dyn_cast<CXXRecordDecl>(Field->getParent()); + if (!Constructor || !RD || !RD->hasInClassInitializer()) + DiagKind = 4; + } else + DiagKind = 4; } if (DiagKind == -1) Index: clang/docs/ReleaseNotes.rst =================================================================== --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -202,6 +202,8 @@ - Fix crash when evaluating consteval constructor of derived class whose base has more than one field. (`#60166 <https://github.com/llvm/llvm-project/issues/60166>`_) +- Fix incorrect deletion of the default constructor of unions in some + cases. (`#48416 <https://github.com/llvm/llvm-project/issues/48416>`_) Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits