[clang] 91fefaa - Revert "[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 12:40:19 PST 2023


Author: Roy Jacobson
Date: 2023-01-04T22:39:04+02:00
New Revision: 91fefaa62ea5bfa6b7e85214ab58aae45017365f

URL: https://github.com/llvm/llvm-project/commit/91fefaa62ea5bfa6b7e85214ab58aae45017365f
DIFF: https://github.com/llvm/llvm-project/commit/91fefaa62ea5bfa6b7e85214ab58aae45017365f.diff

LOG: Revert "[Sema] Don't mark deleted special member functions as non-trivial"

This reverts commit d5dd37ac139a74701e16f084eb2609ff58893770.

Apparently there's some ABI difference in the Sony builder that fails a test.
Will hopefully investigate tomorrow. https://lab.llvm.org/buildbot/#/builders/139/builds/33769

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/Sema/SemaDeclCXX.cpp
    clang/test/AST/ast-dump-funcs.cpp

Removed: 
    clang/test/SemaCXX/GH59624.cpp


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6430bf2936b55..0fe0097400823 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -754,9 +754,6 @@ 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 19ecf21a00ccd..9a80a4874b6db 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -14475,6 +14475,11 @@ 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;
 
@@ -14484,11 +14489,6 @@ 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,6 +14813,11 @@ 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;
 
@@ -14822,11 +14827,6 @@ 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,6 +15197,18 @@ 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;
 
@@ -15206,18 +15218,6 @@ 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,6 +15331,18 @@ 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;
 
@@ -15340,18 +15352,6 @@ 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,9 +17569,6 @@ 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 fdf5abd2ecd35..7d47893d4596d 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 trivial noexcept-unevaluated
+  // CHECK: CXXConstructorDecl 0x{{[^ ]*}} <line:[[@LINE-33]]:8> col:8 implicit S 'void (const S &)' inline default_delete 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 trivial noexcept-unevaluated
-  // CHECK: CXXMethodDecl 0x{{[^ ]*}} <col:8> col:8 implicit operator= 'S &(S &&)' inline default_delete trivial 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: 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 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: 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: 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
deleted file mode 100644
index 8d64f5851ca25..0000000000000
--- a/clang/test/SemaCXX/GH59624.cpp
+++ /dev/null
@@ -1,19 +0,0 @@
-// 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