[clang] [Clang] Fix constexpr-ness on implicitly deleted destructors (PR #116359)
A. Jiang via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 20 10:21:20 PST 2024
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" <de34 at 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 at -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 at -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 at -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 at -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" <de34 at 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" <de34 at 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() {}
More information about the cfe-commits
mailing list