[clang] d5dd37a - [Sema] Don't mark deleted special member functions as non-trivial
Roy Jacobson via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 4 11:27:11 PST 2023
Author: Roy Jacobson
Date: 2023-01-04T21:27:04+02:00
New Revision: d5dd37ac139a74701e16f084eb2609ff58893770
URL: https://github.com/llvm/llvm-project/commit/d5dd37ac139a74701e16f084eb2609ff58893770
DIFF: https://github.com/llvm/llvm-project/commit/d5dd37ac139a74701e16f084eb2609ff58893770.diff
LOG: [Sema] Don't mark deleted special member functions as non-trivial
As noted in https://github.com/llvm/llvm-project/issues/59624, we sometimes mark implicitly
deleted special member functions as non-trivial. This is unnecessary work and leads to some
weird type traits errors.
This fixes the problem by making the implicitly deleted special member functions always
trivial.
Reviewed By: #clang-language-wg, erichkeane
Differential Revision: https://reviews.llvm.org/D140664
Added:
clang/test/SemaCXX/GH59624.cpp
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/AST/ast-dump-funcs.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c4030387ae55e..cbf89cb342c73 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -757,6 +757,9 @@ ABI Changes in Clang
classified such types as non-POD (for the purposes of Itanium ABI). Clang now
matches the gcc behavior (except on Darwin and PS4). You can switch back to
the old ABI behavior with the flag: ``-fclang-abi-compat=15.0``.
+- Some types with implicitly deleted special member functions were accidentally
+ marked as non-trivially copyable. This has been fixed
+ (`#59624 <https://github.com/llvm/llvm-project/issues/59624>`_).
OpenMP Support in Clang
-----------------------
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 9a80a4874b6db..19ecf21a00ccd 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -14475,11 +14475,6 @@ CXXMethodDecl *Sema::DeclareImplicitCopyAssignment(CXXRecordDecl *ClassDecl) {
nullptr);
CopyAssignment->setParams(FromParam);
- CopyAssignment->setTrivial(
- ClassDecl->needsOverloadResolutionForCopyAssignment()
- ? SpecialMemberIsTrivial(CopyAssignment, CXXCopyAssignment)
- : ClassDecl->hasTrivialCopyAssignment());
-
// Note that we have added this copy-assignment operator.
++getASTContext().NumImplicitCopyAssignmentOperatorsDeclared;
@@ -14489,6 +14484,11 @@ CXXMethodDecl *Sema::DeclareImplicitCopyAssignment(CXXRecordDecl *ClassDecl) {
if (ShouldDeleteSpecialMember(CopyAssignment, CXXCopyAssignment)) {
ClassDecl->setImplicitCopyAssignmentIsDeleted();
SetDeclDeleted(CopyAssignment, ClassLoc);
+ } else {
+ CopyAssignment->setTrivial(
+ ClassDecl->needsOverloadResolutionForCopyAssignment()
+ ? SpecialMemberIsTrivial(CopyAssignment, CXXCopyAssignment)
+ : ClassDecl->hasTrivialCopyAssignment());
}
if (S)
@@ -14813,11 +14813,6 @@ CXXMethodDecl *Sema::DeclareImplicitMoveAssignment(CXXRecordDecl *ClassDecl) {
nullptr);
MoveAssignment->setParams(FromParam);
- MoveAssignment->setTrivial(
- ClassDecl->needsOverloadResolutionForMoveAssignment()
- ? SpecialMemberIsTrivial(MoveAssignment, CXXMoveAssignment)
- : ClassDecl->hasTrivialMoveAssignment());
-
// Note that we have added this copy-assignment operator.
++getASTContext().NumImplicitMoveAssignmentOperatorsDeclared;
@@ -14827,6 +14822,11 @@ CXXMethodDecl *Sema::DeclareImplicitMoveAssignment(CXXRecordDecl *ClassDecl) {
if (ShouldDeleteSpecialMember(MoveAssignment, CXXMoveAssignment)) {
ClassDecl->setImplicitMoveAssignmentIsDeleted();
SetDeclDeleted(MoveAssignment, ClassLoc);
+ } else {
+ MoveAssignment->setTrivial(
+ ClassDecl->needsOverloadResolutionForMoveAssignment()
+ ? SpecialMemberIsTrivial(MoveAssignment, CXXMoveAssignment)
+ : ClassDecl->hasTrivialMoveAssignment());
}
if (S)
@@ -15197,18 +15197,6 @@ CXXConstructorDecl *Sema::DeclareImplicitCopyConstructor(
/*TInfo=*/TSI, SC_None, nullptr);
CopyConstructor->setParams(FromParam);
- CopyConstructor->setTrivial(
- ClassDecl->needsOverloadResolutionForCopyConstructor()
- ? SpecialMemberIsTrivial(CopyConstructor, CXXCopyConstructor)
- : ClassDecl->hasTrivialCopyConstructor());
-
- CopyConstructor->setTrivialForCall(
- ClassDecl->hasAttr<TrivialABIAttr>() ||
- (ClassDecl->needsOverloadResolutionForCopyConstructor()
- ? SpecialMemberIsTrivial(CopyConstructor, CXXCopyConstructor,
- TAH_ConsiderTrivialABI)
- : ClassDecl->hasTrivialCopyConstructorForCall()));
-
// Note that we have declared this constructor.
++getASTContext().NumImplicitCopyConstructorsDeclared;
@@ -15218,6 +15206,18 @@ CXXConstructorDecl *Sema::DeclareImplicitCopyConstructor(
if (ShouldDeleteSpecialMember(CopyConstructor, CXXCopyConstructor)) {
ClassDecl->setImplicitCopyConstructorIsDeleted();
SetDeclDeleted(CopyConstructor, ClassLoc);
+ } else {
+ CopyConstructor->setTrivial(
+ ClassDecl->needsOverloadResolutionForCopyConstructor()
+ ? SpecialMemberIsTrivial(CopyConstructor, CXXCopyConstructor)
+ : ClassDecl->hasTrivialCopyConstructor());
+
+ CopyConstructor->setTrivialForCall(
+ ClassDecl->hasAttr<TrivialABIAttr>() ||
+ (ClassDecl->needsOverloadResolutionForCopyConstructor()
+ ? SpecialMemberIsTrivial(CopyConstructor, CXXCopyConstructor,
+ TAH_ConsiderTrivialABI)
+ : ClassDecl->hasTrivialCopyConstructorForCall()));
}
if (S)
@@ -15331,18 +15331,6 @@ CXXConstructorDecl *Sema::DeclareImplicitMoveConstructor(
SC_None, nullptr);
MoveConstructor->setParams(FromParam);
- MoveConstructor->setTrivial(
- ClassDecl->needsOverloadResolutionForMoveConstructor()
- ? SpecialMemberIsTrivial(MoveConstructor, CXXMoveConstructor)
- : ClassDecl->hasTrivialMoveConstructor());
-
- MoveConstructor->setTrivialForCall(
- ClassDecl->hasAttr<TrivialABIAttr>() ||
- (ClassDecl->needsOverloadResolutionForMoveConstructor()
- ? SpecialMemberIsTrivial(MoveConstructor, CXXMoveConstructor,
- TAH_ConsiderTrivialABI)
- : ClassDecl->hasTrivialMoveConstructorForCall()));
-
// Note that we have declared this constructor.
++getASTContext().NumImplicitMoveConstructorsDeclared;
@@ -15352,6 +15340,18 @@ CXXConstructorDecl *Sema::DeclareImplicitMoveConstructor(
if (ShouldDeleteSpecialMember(MoveConstructor, CXXMoveConstructor)) {
ClassDecl->setImplicitMoveConstructorIsDeleted();
SetDeclDeleted(MoveConstructor, ClassLoc);
+ } else {
+ MoveConstructor->setTrivial(
+ ClassDecl->needsOverloadResolutionForMoveConstructor()
+ ? SpecialMemberIsTrivial(MoveConstructor, CXXMoveConstructor)
+ : ClassDecl->hasTrivialMoveConstructor());
+
+ MoveConstructor->setTrivialForCall(
+ ClassDecl->hasAttr<TrivialABIAttr>() ||
+ (ClassDecl->needsOverloadResolutionForMoveConstructor()
+ ? SpecialMemberIsTrivial(MoveConstructor, CXXMoveConstructor,
+ TAH_ConsiderTrivialABI)
+ : ClassDecl->hasTrivialMoveConstructorForCall()));
}
if (S)
@@ -17569,6 +17569,9 @@ void Sema::SetDeclDeleted(Decl *Dcl, SourceLocation DelLoc) {
// A deleted function is implicitly inline.
Fn->setImplicitlyInline();
Fn->setDeletedAsWritten();
+
+ Fn->setTrivial(true);
+ Fn->setTrivialForCall(true);
}
void Sema::SetDeclDefaulted(Decl *Dcl, SourceLocation DefaultLoc) {
diff --git a/clang/test/AST/ast-dump-funcs.cpp b/clang/test/AST/ast-dump-funcs.cpp
index 7d47893d4596d..fdf5abd2ecd35 100644
--- a/clang/test/AST/ast-dump-funcs.cpp
+++ b/clang/test/AST/ast-dump-funcs.cpp
@@ -54,10 +54,10 @@ struct S {
virtual void g() = 0;
// CHECK: CXXMethodDecl 0x{{[^ ]*}} <line:[[@LINE-1]]:3, col:22> col:16 g 'void ()' virtual pure
- // CHECK: CXXConstructorDecl 0x{{[^ ]*}} <line:[[@LINE-33]]:8> col:8 implicit S 'void (const S &)' inline default_delete noexcept-unevaluated
+ // CHECK: CXXConstructorDecl 0x{{[^ ]*}} <line:[[@LINE-33]]:8> col:8 implicit S 'void (const S &)' inline default_delete trivial noexcept-unevaluated
// CHECK: CXXConstructorDecl 0x{{[^ ]*}} <col:8> col:8 implicit constexpr S 'void (S &&)' inline default noexcept-unevaluated
- // CHECK: CXXMethodDecl 0x{{[^ ]*}} <col:8> col:8 implicit operator= 'S &(const S &)' inline default_delete noexcept-unevaluated
- // CHECK: CXXMethodDecl 0x{{[^ ]*}} <col:8> col:8 implicit operator= 'S &(S &&)' inline default_delete noexcept-unevaluated
+ // CHECK: CXXMethodDecl 0x{{[^ ]*}} <col:8> col:8 implicit operator= 'S &(const S &)' inline default_delete trivial noexcept-unevaluated
+ // CHECK: CXXMethodDecl 0x{{[^ ]*}} <col:8> col:8 implicit operator= 'S &(S &&)' inline default_delete trivial noexcept-unevaluated
// CHECK: CXXDestructorDecl 0x{{[^ ]*}} <col:8> col:8 implicit ~S 'void ()' inline default noexcept-unevaluated
};
@@ -70,9 +70,9 @@ struct T : S { // T is not referenced, but S is
// CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}} <col:23> 'int' 100
// CHECK-NEXT: OverrideAttr
- // CHECK: CXXConstructorDecl 0x{{[^ ]*}} <line:[[@LINE-9]]:8> col:8 implicit T 'void (const T &)' inline default_delete noexcept-unevaluated
- // CHECK: CXXMethodDecl 0x{{[^ ]*}} <col:8> col:8 implicit operator= 'T &(const T &)' inline default_delete noexcept-unevaluated
- // CHECK: CXXMethodDecl 0x{{[^ ]*}} <col:8> col:8 implicit operator= 'T &(T &&)' inline default_delete noexcept-unevaluated
+ // CHECK: CXXConstructorDecl 0x{{[^ ]*}} <line:[[@LINE-9]]:8> col:8 implicit T 'void (const T &)' inline default_delete trivial noexcept-unevaluated
+ // CHECK: CXXMethodDecl 0x{{[^ ]*}} <col:8> col:8 implicit operator= 'T &(const T &)' inline default_delete trivial noexcept-unevaluated
+ // CHECK: CXXMethodDecl 0x{{[^ ]*}} <col:8> col:8 implicit operator= 'T &(T &&)' inline default_delete trivial noexcept-unevaluated
// CHECK: CXXDestructorDecl 0x{{[^ ]*}} <col:8> col:8 implicit ~T 'void ()' inline default noexcept-unevaluated
};
diff --git a/clang/test/SemaCXX/GH59624.cpp b/clang/test/SemaCXX/GH59624.cpp
new file mode 100644
index 0000000000000..8d64f5851ca25
--- /dev/null
+++ b/clang/test/SemaCXX/GH59624.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++20
+// expected-no-diagnostics
+
+namespace GH59624 {
+
+struct Foo{
+ int x{0};
+};
+
+struct Bar{
+ const Foo y;
+};
+
+// Deleted move assignment shouldn't make type non-trivially copyable:
+
+static_assert(__is_trivially_copyable(Bar), "");
+
+}
More information about the cfe-commits
mailing list