https://github.com/frederick-vs-ja updated https://github.com/llvm/llvm-project/pull/116359
>From c950170822a58ca98e3f50e95b160c83ec1c63f1 Mon Sep 17 00:00:00 2001 From: "A. Jiang" <d...@live.cn> Date: Fri, 15 Nov 2024 21:49:23 +0800 Subject: [PATCH 1/3] [Clang] Fix constexpr-ness on implicitly deleted destructors In C++20, a defaulted but implicitly deleted destructor is constexpr if and only if the class has no virtual base class. This hasn't been changed in C++23 by P2448R2. Constexpr-ness on a deleted destructor affects almost nothing. The `__is_literal` intrinsic is related, while the corresponding `std::is_literal_type(_v)` utility has been removed in C++20. A recently added example in `test/AST/ByteCode/cxx23.cpp` will become valid, and the example is already accepted by GCC. Clang currently behaves correctly in C++23 mode, because the constexpr-ness on defaulted destructor is relaxed by P2448R2. But we should make similar relaxation for an implicitly deleted destructor. --- .../clang/AST/CXXRecordDeclDefinitionBits.def | 3 + clang/include/clang/AST/DeclCXX.h | 7 ++ clang/lib/AST/DeclCXX.cpp | 24 ++++--- clang/test/AST/ByteCode/cxx23.cpp | 4 +- clang/test/SemaCXX/literal-type.cpp | 68 +++++++++++++++++++ 5 files changed, 96 insertions(+), 10 deletions(-) diff --git a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def index 6620840df0ced2..7f47fb0890f50e 100644 --- a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def +++ b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def @@ -81,6 +81,9 @@ FIELD(IsStandardLayout, 1, NO_MERGE) /// member. FIELD(IsCXX11StandardLayout, 1, NO_MERGE) +/// True when the class has a virtual base class. +FIELD(HasVBases, 1, NO_MERGE) + /// True when any base class has any declared non-static data /// members or bit-fields. /// This is a helper bit of state used to implement IsStandardLayout more diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index 2693cc0e95b4b2..6aadb9794328ae 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -890,6 +890,13 @@ class CXXRecordDecl : public RecordDecl { needsOverloadResolutionForDestructor()) && "destructor should not be deleted"); data().DefaultedDestructorIsDeleted = true; + // C++23 [dcl.constexpr]p3.2: + // if the function is a constructor or destructor, its class does not have + // any virtual base classes. + // C++20 [dcl.constexpr]p5: + // The definition of a constexpr destructor whose function-body is + // [not = delete] shall additionally satisfy... + data().DefaultedDestructorIsConstexpr = !data().HasVBases; } /// Determine whether this class should get an implicit move diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index 4394a0724b3c17..f094482eec6165 100644 --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -77,10 +77,11 @@ CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D) : UserDeclaredConstructor(false), UserDeclaredSpecialMembers(0), Aggregate(true), PlainOldData(true), Empty(true), Polymorphic(false), Abstract(false), IsStandardLayout(true), IsCXX11StandardLayout(true), - HasBasesWithFields(false), HasBasesWithNonStaticDataMembers(false), - HasPrivateFields(false), HasProtectedFields(false), - HasPublicFields(false), HasMutableFields(false), HasVariantMembers(false), - HasOnlyCMembers(true), HasInitMethod(false), HasInClassInitializer(false), + HasVBases(false), HasBasesWithFields(false), + HasBasesWithNonStaticDataMembers(false), HasPrivateFields(false), + HasProtectedFields(false), HasPublicFields(false), + HasMutableFields(false), HasVariantMembers(false), HasOnlyCMembers(true), + HasInitMethod(false), HasInClassInitializer(false), HasUninitializedReferenceMember(false), HasUninitializedFields(false), HasInheritedConstructor(false), HasInheritedDefaultConstructor(false), HasInheritedAssignment(false), @@ -316,6 +317,8 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases, } if (Base->isVirtual()) { + data().HasVBases = true; + // Add this base if it's not already in the list. if (SeenVBaseTypes.insert(C.getCanonicalType(BaseType)).second) VBases.push_back(Base); @@ -547,9 +550,9 @@ void CXXRecordDecl::addedClassSubobject(CXXRecordDecl *Subobj) { data().NeedOverloadResolutionForDestructor = true; } - // C++2a [dcl.constexpr]p4: - // The definition of a constexpr destructor [shall] satisfy the - // following requirement: + // C++20 [dcl.constexpr]p5: + // The definition of a constexpr destructor whose function-body is not + // = delete [shall] additionally satisfy the following requirement: // -- for every subobject of class type or (possibly multi-dimensional) // array thereof, that class type shall have a constexpr destructor if (!Subobj->hasConstexprDestructor()) @@ -1214,8 +1217,13 @@ void CXXRecordDecl::addedMember(Decl *D) { data().DefaultedCopyAssignmentIsDeleted = true; if (FieldRec->hasNonTrivialMoveAssignment()) data().DefaultedMoveAssignmentIsDeleted = true; - if (FieldRec->hasNonTrivialDestructor()) + if (FieldRec->hasNonTrivialDestructor()) { data().DefaultedDestructorIsDeleted = true; + // C++20 [dcl.constexpr]p5: + // The definition of a constexpr destructor whose function-body is + // [not = delete] shall additionally satisfy... + data().DefaultedDestructorIsConstexpr = true; + } } // For an anonymous union member, our overload resolution will perform diff --git a/clang/test/AST/ByteCode/cxx23.cpp b/clang/test/AST/ByteCode/cxx23.cpp index 1803fb8ab2e9a4..6a62ac11cde792 100644 --- a/clang/test/AST/ByteCode/cxx23.cpp +++ b/clang/test/AST/ByteCode/cxx23.cpp @@ -263,7 +263,7 @@ namespace AnonUnionDtor { template <class T> struct opt { - union { // all20-note {{is not literal}} + union { char c; T data; }; @@ -279,7 +279,7 @@ namespace AnonUnionDtor { }; consteval void foo() { - opt<A> a; // all20-error {{variable of non-literal type}} + opt<A> a; } void bar() { foo(); } diff --git a/clang/test/SemaCXX/literal-type.cpp b/clang/test/SemaCXX/literal-type.cpp index 88535c169fe01c..44b6ba62262fd0 100644 --- a/clang/test/SemaCXX/literal-type.cpp +++ b/clang/test/SemaCXX/literal-type.cpp @@ -20,6 +20,7 @@ static_assert(__is_literal(VectorExt), "fail"); // [...] // -- a class type that has all of the following properties: // -- it has a trivial destructor +// [P0784R7 changed the condition to "constexpr destructor" in C++20] // -- every constructor call and full-expression in the // brace-or-equal-initializers for non-static data members (if an) is // a constant expression, @@ -108,3 +109,70 @@ void test() { } #endif + +#if __cplusplus >= 201103L +namespace GH85550 { +struct HasDefaultCtorAndNonConstexprDtor { + constexpr HasDefaultCtorAndNonConstexprDtor() = default; + ~HasDefaultCtorAndNonConstexprDtor() {} +}; + +union UnionWithNonLiteralMember { + HasDefaultCtorAndNonConstexprDtor x; + int y; + + constexpr UnionWithNonLiteralMember() : x{} {} +}; +#if __cplusplus >= 202002L +static_assert(__is_literal(UnionWithNonLiteralMember), "fail"); +#else +static_assert(!__is_literal(UnionWithNonLiteralMember), "fail"); +#endif + +union UnionWithNonLiteralMemberExplicitDtor1 { + HasDefaultCtorAndNonConstexprDtor x; + int y; + // expected-note@-2 {{destructor of 'UnionWithNonLiteralMemberExplicitDtor1' is implicitly deleted because variant field 'x' has a non-trivial destructor}} + + constexpr UnionWithNonLiteralMemberExplicitDtor1() : x{} {} + ~UnionWithNonLiteralMemberExplicitDtor1() = default; // expected-warning {{explicitly defaulted destructor is implicitly deleted}} + // expected-note@-1 {{replace 'default' with 'delete'}} +}; +#if __cplusplus >= 202002L +static_assert(__is_literal(UnionWithNonLiteralMemberExplicitDtor1), "fail"); +#else +static_assert(!__is_literal(UnionWithNonLiteralMemberExplicitDtor1), "fail"); +#endif + +union UnionWithNonLiteralMemberExplicitDtor2 { + HasDefaultCtorAndNonConstexprDtor x; + int y; + + constexpr UnionWithNonLiteralMemberExplicitDtor2() : x{} {} + ~UnionWithNonLiteralMemberExplicitDtor2() = delete; +}; +static_assert(!__is_literal(UnionWithNonLiteralMemberExplicitDtor2), "fail"); + +#if __cplusplus >= 202002L +union UnionWithNonLiteralMemberConstexprDtor1 { + HasDefaultCtorAndNonConstexprDtor x; + int y; + // expected-note@-2 {{destructor of 'UnionWithNonLiteralMemberConstexprDtor1' is implicitly deleted because variant field 'x' has a non-trivial destructor}} + + constexpr UnionWithNonLiteralMemberConstexprDtor1() : x{} {} + constexpr ~UnionWithNonLiteralMemberConstexprDtor1() = default; // expected-warning {{explicitly defaulted destructor is implicitly deleted}} + // expected-note@-1 {{replace 'default' with 'delete'}} +}; +static_assert(__is_literal(UnionWithNonLiteralMemberConstexprDtor1), "fail"); + +union UnionWithNonLiteralMemberConstexprDtor2 { + HasDefaultCtorAndNonConstexprDtor x; + int y; + + constexpr UnionWithNonLiteralMemberConstexprDtor2() : x{} {} + constexpr ~UnionWithNonLiteralMemberConstexprDtor2() = delete; +}; +static_assert(__is_literal(UnionWithNonLiteralMemberConstexprDtor2), "fail"); +#endif +} +#endif >From f9aa408a2aad8da79547703aaf44015a0345ce98 Mon Sep 17 00:00:00 2001 From: "A. Jiang" <d...@live.cn> Date: Thu, 21 Nov 2024 02:10:11 +0800 Subject: [PATCH 2/3] Address review comments, add release notes. --- clang/docs/ReleaseNotes.rst | 29 +++++++++++++++++++ .../clang/AST/CXXRecordDeclDefinitionBits.def | 3 -- clang/include/clang/AST/DeclCXX.h | 6 ++-- clang/lib/AST/DeclCXX.cpp | 11 +++---- 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index ba582160cf9920..5b8f9ef9c2c85c 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -148,6 +148,35 @@ C++ Specific Potentially Breaking Changes // Now diagnoses with an error. void f(int& i [[clang::lifetimebound]]); +- Clang will now consider the implicitly deleted destructor of a union or + a non-union class without virtual base class to be ``constexpr`` in C++20 + mode. Previously, Clang does so since C++23, but the standard specification + for this changed in C++20. (GH#85550) + + .. code-block:: c++ + struct NonLiteral { + NonLiteral() {} + ~NonLiteral() {} + }; + + template <class T> + struct Opt { + union { + char c; + T data; + }; + bool engaged = false; + + constexpr Opt() {} + constexpr ~Opt() { + if (engaged) + data.~T(); + } + }; + + // Previously only accepted in C++23 and later, now also accepted in C++20. + consteval void foo() { Opt<NonLiteral>{}; } + ABI Changes in This Version --------------------------- diff --git a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def index 7f47fb0890f50e..6620840df0ced2 100644 --- a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def +++ b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def @@ -81,9 +81,6 @@ FIELD(IsStandardLayout, 1, NO_MERGE) /// member. FIELD(IsCXX11StandardLayout, 1, NO_MERGE) -/// True when the class has a virtual base class. -FIELD(HasVBases, 1, NO_MERGE) - /// True when any base class has any declared non-static data /// members or bit-fields. /// This is a helper bit of state used to implement IsStandardLayout more diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index 6aadb9794328ae..6ec782530544e7 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -891,12 +891,12 @@ class CXXRecordDecl : public RecordDecl { "destructor should not be deleted"); data().DefaultedDestructorIsDeleted = true; // C++23 [dcl.constexpr]p3.2: - // if the function is a constructor or destructor, its class does not have - // any virtual base classes. + // if the function is a constructor or destructor, its class does not have + // any virtual base classes. // C++20 [dcl.constexpr]p5: // The definition of a constexpr destructor whose function-body is // [not = delete] shall additionally satisfy... - data().DefaultedDestructorIsConstexpr = !data().HasVBases; + data().DefaultedDestructorIsConstexpr = data().NumVBases == 0; } /// Determine whether this class should get an implicit move diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index f094482eec6165..8399732eb45aaa 100644 --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -77,11 +77,10 @@ CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D) : UserDeclaredConstructor(false), UserDeclaredSpecialMembers(0), Aggregate(true), PlainOldData(true), Empty(true), Polymorphic(false), Abstract(false), IsStandardLayout(true), IsCXX11StandardLayout(true), - HasVBases(false), HasBasesWithFields(false), - HasBasesWithNonStaticDataMembers(false), HasPrivateFields(false), - HasProtectedFields(false), HasPublicFields(false), - HasMutableFields(false), HasVariantMembers(false), HasOnlyCMembers(true), - HasInitMethod(false), HasInClassInitializer(false), + HasBasesWithFields(false), HasBasesWithNonStaticDataMembers(false), + HasPrivateFields(false), HasProtectedFields(false), + HasPublicFields(false), HasMutableFields(false), HasVariantMembers(false), + HasOnlyCMembers(true), HasInitMethod(false), HasInClassInitializer(false), HasUninitializedReferenceMember(false), HasUninitializedFields(false), HasInheritedConstructor(false), HasInheritedDefaultConstructor(false), HasInheritedAssignment(false), @@ -317,8 +316,6 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases, } if (Base->isVirtual()) { - data().HasVBases = true; - // Add this base if it's not already in the list. if (SeenVBaseTypes.insert(C.getCanonicalType(BaseType)).second) VBases.push_back(Base); >From b833c82106855aaec809ab726b03afbb0c414e1e Mon Sep 17 00:00:00 2001 From: "A. Jiang" <d...@live.cn> Date: Thu, 21 Nov 2024 02:20:53 +0800 Subject: [PATCH 3/3] Try to fix release notes --- clang/docs/ReleaseNotes.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 5b8f9ef9c2c85c..8ec72e0c35782d 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -154,6 +154,7 @@ C++ Specific Potentially Breaking Changes for this changed in C++20. (GH#85550) .. code-block:: c++ + struct NonLiteral { NonLiteral() {} ~NonLiteral() {} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits