[Clang] Fix constexpr-ness on implicitly deleted destructors #116359
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
frederick-vs-ja
added
c++20
clang:frontend
frederick-vs-ja
force-pushed
the
is-literal-cxx20
branch
2 times, most recently
from
b93d63f to
f39b5a0
Compare
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.
frederick-vs-ja
force-pushed
the
is-literal-cxx20
branch
from
f39b5a0 to
c950170
Compare
|
@llvm/pr-subscribers-clang Author: A. Jiang (frederick-vs-ja) ChangesIn 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 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. Fixes #85550. Full diff: https://github.com/llvm/llvm-project/pull/116359.diff 5 Files Affected:
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
|
cor3ntin
reviewed
Nov 27, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/9270 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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_literalintrinsic is related, while the correspondingstd::is_literal_type(_v)utility has been removed in C++20. A recently added example intest/AST/ByteCode/cxx23.cppwill 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.
Fixes #85550.