From d5dd37ac139a74701e16f084eb2609ff58893770 Mon Sep 17 00:00:00 2001 From: Roy Jacobson Date: Sun, 25 Dec 2022 23:48:13 +0200 Subject: [PATCH] [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 --- clang/docs/ReleaseNotes.rst | 3 ++ clang/lib/Sema/SemaDeclCXX.cpp | 71 ++++++++++++++++++++------------------- clang/test/AST/ast-dump-funcs.cpp | 12 +++---- clang/test/SemaCXX/GH59624.cpp | 19 +++++++++++ 4 files changed, 65 insertions(+), 40 deletions(-) create mode 100644 clang/test/SemaCXX/GH59624.cpp diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index c4030387..cbf89cb 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 `_). OpenMP Support in Clang ----------------------- diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 9a80a48..19ecf21 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() || - (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() || + (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() || - (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() || + (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 7d47893..fdf5abd 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{{[^ ]*}} col:16 g 'void ()' virtual pure - // CHECK: CXXConstructorDecl 0x{{[^ ]*}} col:8 implicit S 'void (const S &)' inline default_delete noexcept-unevaluated + // CHECK: CXXConstructorDecl 0x{{[^ ]*}} col:8 implicit S 'void (const S &)' inline default_delete trivial noexcept-unevaluated // CHECK: CXXConstructorDecl 0x{{[^ ]*}} col:8 implicit constexpr S 'void (S &&)' inline default noexcept-unevaluated - // CHECK: CXXMethodDecl 0x{{[^ ]*}} col:8 implicit operator= 'S &(const S &)' inline default_delete noexcept-unevaluated - // CHECK: CXXMethodDecl 0x{{[^ ]*}} col:8 implicit operator= 'S &(S &&)' inline default_delete noexcept-unevaluated + // CHECK: CXXMethodDecl 0x{{[^ ]*}} col:8 implicit operator= 'S &(const S &)' inline default_delete trivial noexcept-unevaluated + // CHECK: CXXMethodDecl 0x{{[^ ]*}} col:8 implicit operator= 'S &(S &&)' inline default_delete trivial noexcept-unevaluated // CHECK: CXXDestructorDecl 0x{{[^ ]*}} 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{{[^ ]*}} 'int' 100 // CHECK-NEXT: OverrideAttr - // CHECK: CXXConstructorDecl 0x{{[^ ]*}} col:8 implicit T 'void (const T &)' inline default_delete noexcept-unevaluated - // CHECK: CXXMethodDecl 0x{{[^ ]*}} col:8 implicit operator= 'T &(const T &)' inline default_delete noexcept-unevaluated - // CHECK: CXXMethodDecl 0x{{[^ ]*}} col:8 implicit operator= 'T &(T &&)' inline default_delete noexcept-unevaluated + // CHECK: CXXConstructorDecl 0x{{[^ ]*}} col:8 implicit T 'void (const T &)' inline default_delete trivial noexcept-unevaluated + // CHECK: CXXMethodDecl 0x{{[^ ]*}} col:8 implicit operator= 'T &(const T &)' inline default_delete trivial noexcept-unevaluated + // CHECK: CXXMethodDecl 0x{{[^ ]*}} col:8 implicit operator= 'T &(T &&)' inline default_delete trivial noexcept-unevaluated // CHECK: CXXDestructorDecl 0x{{[^ ]*}} 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 0000000..8d64f58 --- /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), ""); + +} -- 2.7.4