[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 269633.
ahatanak added a comment.

Rebase patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57626/new/

https://reviews.llvm.org/D57626

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaObjCXX/attr-trivial-abi.mm


Index: clang/test/SemaObjCXX/attr-trivial-abi.mm
===================================================================
--- clang/test/SemaObjCXX/attr-trivial-abi.mm
+++ clang/test/SemaObjCXX/attr-trivial-abi.mm
@@ -10,23 +10,23 @@
   int a;
 };
 
-struct __attribute__((trivial_abi)) S2 { // expected-warning {{'trivial_abi' cannot be applied to 'S2'}}
+struct __attribute__((trivial_abi)) S2 { // expected-warning {{'trivial_abi' cannot be applied to 'S2'}} expected-note {{has a __weak field}}
   __weak id a;
 };
 
-struct __attribute__((trivial_abi)) S3 { // expected-warning {{'trivial_abi' cannot be applied to 'S3'}}
+struct __attribute__((trivial_abi)) S3 { // expected-warning {{'trivial_abi' cannot be applied to 'S3'}} expected-note {{is polymorphic}}
   virtual void m();
 };
 
 struct S3_2 {
   virtual void m();
-} __attribute__((trivial_abi)); // expected-warning {{'trivial_abi' cannot be applied to 'S3_2'}}
+} __attribute__((trivial_abi)); // expected-warning {{'trivial_abi' cannot be applied to 'S3_2'}} expected-note {{is polymorphic}}
 
 struct S4 {
   int a;
 };
 
-struct __attribute__((trivial_abi)) S5 : public virtual S4 { // expected-warning {{'trivial_abi' cannot be applied to 'S5'}}
+struct __attribute__((trivial_abi)) S5 : public virtual S4 { // expected-warning {{'trivial_abi' cannot be applied to 'S5'}} expected-note {{has a virtual base}}
 };
 
 struct __attribute__((trivial_abi)) S9 : public S4 {
@@ -36,19 +36,19 @@
   __weak id a;
 };
 
-struct __attribute__((trivial_abi)) S12 { // expected-warning {{'trivial_abi' cannot be applied to 'S12'}}
+struct __attribute__((trivial_abi)) S12 { // expected-warning {{'trivial_abi' cannot be applied to 'S12'}} expected-note {{has a __weak field}}
   __weak id a;
 };
 
-struct __attribute__((trivial_abi)) S13 { // expected-warning {{'trivial_abi' cannot be applied to 'S13'}}
+struct __attribute__((trivial_abi)) S13 { // expected-warning {{'trivial_abi' cannot be applied to 'S13'}} expected-note {{has a __weak field}}
   __weak id a[2];
 };
 
-struct __attribute__((trivial_abi)) S7 { // expected-warning {{'trivial_abi' cannot be applied to 'S7'}}
+struct __attribute__((trivial_abi)) S7 { // expected-warning {{'trivial_abi' cannot be applied to 'S7'}} expected-note {{has a field of a non-trivial class type}}
   S6 a;
 };
 
-struct __attribute__((trivial_abi)) S11 { // expected-warning {{'trivial_abi' cannot be applied to 'S11'}}
+struct __attribute__((trivial_abi)) S11 { // expected-warning {{'trivial_abi' cannot be applied to 'S11'}} expected-note {{has a field of a non-trivial class type}}
   S6 a[2];
 };
 
@@ -66,7 +66,7 @@
 S10<__weak id> p2;
 
 template<>
-struct __attribute__((trivial_abi)) S10<id> { // expected-warning {{'trivial_abi' cannot be applied to 'S10<id>'}}
+struct __attribute__((trivial_abi)) S10<id> { // expected-warning {{'trivial_abi' cannot be applied to 'S10<id>'}} expected-note {{has a __weak field}}
   __weak id a;
 };
 
@@ -90,8 +90,39 @@
 S16<int> s16;
 
 template<class T>
-struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}}
+struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}} expected-note {{has a __weak field}}
   __weak id a;
 };
 
 S17<int> s17;
+
+namespace deletedCopyMoveConstructor {
+  struct __attribute__((trivial_abi)) CopyMoveDeleted { // expected-warning {{'trivial_abi' cannot be applied to 'CopyMoveDeleted'}} expected-note {{copy constructors and move constructors are all deleted}}
+    CopyMoveDeleted(const CopyMoveDeleted &) = delete;
+    CopyMoveDeleted(CopyMoveDeleted &&) = delete;
+  };
+
+  struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{copy constructors and move constructors are all deleted}}
+    CopyMoveDeleted a;
+  };
+
+  struct __attribute__((trivial_abi)) CopyDeleted {
+    CopyDeleted(const CopyDeleted &) = delete;
+    CopyDeleted(CopyDeleted &&) = default;
+  };
+
+  struct __attribute__((trivial_abi)) MoveDeleted {
+    MoveDeleted(const MoveDeleted &) = default;
+    MoveDeleted(MoveDeleted &&) = delete;
+  };
+
+  struct __attribute__((trivial_abi)) S19 { // expected-warning {{'trivial_abi' cannot be applied to 'S19'}} expected-note {{copy constructors and move constructors are all deleted}}
+    CopyDeleted a;
+    MoveDeleted b;
+  };
+
+  // This is fine since the move constructor isn't deleted.
+  struct __attribute__((trivial_abi)) S20 {
+    int &&a; // a member of rvalue reference type deletes the copy constructor.
+  };
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -9686,27 +9686,53 @@
 }
 
 void Sema::checkIllFormedTrivialABIStruct(CXXRecordDecl &RD) {
-  auto PrintDiagAndRemoveAttr = [&]() {
+  auto PrintDiagAndRemoveAttr = [&](unsigned N) {
     // No diagnostics if this is a template instantiation.
-    if (!isTemplateInstantiation(RD.getTemplateSpecializationKind()))
+    if (!isTemplateInstantiation(RD.getTemplateSpecializationKind())) {
       Diag(RD.getAttr<TrivialABIAttr>()->getLocation(),
            diag::ext_cannot_use_trivial_abi) << &RD;
+      Diag(RD.getAttr<TrivialABIAttr>()->getLocation(),
+           diag::note_cannot_use_trivial_abi_reason) << &RD << N;
+    }
     RD.dropAttr<TrivialABIAttr>();
   };
 
+  // Ill-formed if the copy and move constructors are deleted.
+  auto HasNonDeletedCopyOrMoveConstructor = [&]() {
+    if (RD.needsImplicitCopyConstructor() &&
+        !RD.defaultedCopyConstructorIsDeleted())
+      return true;
+    if (RD.needsImplicitMoveConstructor() &&
+        !RD.defaultedMoveConstructorIsDeleted())
+      return true;
+    for (const CXXConstructorDecl *CD : RD.ctors())
+      if (CD->isCopyOrMoveConstructor() && !CD->isDeleted())
+        return true;
+    return false;
+  };
+
+  if (!HasNonDeletedCopyOrMoveConstructor()) {
+    PrintDiagAndRemoveAttr(0);
+    return;
+  }
+
   // Ill-formed if the struct has virtual functions.
   if (RD.isPolymorphic()) {
-    PrintDiagAndRemoveAttr();
+    PrintDiagAndRemoveAttr(1);
     return;
   }
 
   for (const auto &B : RD.bases()) {
     // Ill-formed if the base class is non-trivial for the purpose of calls or a
     // virtual base.
-    if ((!B.getType()->isDependentType() &&
-         !B.getType()->getAsCXXRecordDecl()->canPassInRegisters()) ||
-        B.isVirtual()) {
-      PrintDiagAndRemoveAttr();
+    if (!B.getType()->isDependentType() &&
+        !B.getType()->getAsCXXRecordDecl()->canPassInRegisters()) {
+      PrintDiagAndRemoveAttr(2);
+      return;
+    }
+
+    if (B.isVirtual()) {
+      PrintDiagAndRemoveAttr(3);
       return;
     }
   }
@@ -9716,14 +9742,14 @@
     // non-trivial for the purpose of calls.
     QualType FT = FD->getType();
     if (FT.getObjCLifetime() == Qualifiers::OCL_Weak) {
-      PrintDiagAndRemoveAttr();
+      PrintDiagAndRemoveAttr(4);
       return;
     }
 
     if (const auto *RT = FT->getBaseElementTypeUnsafe()->getAs<RecordType>())
       if (!RT->isDependentType() &&
           !cast<CXXRecordDecl>(RT->getDecl())->canPassInRegisters()) {
-        PrintDiagAndRemoveAttr();
+        PrintDiagAndRemoveAttr(5);
         return;
       }
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3301,6 +3301,12 @@
 
 def ext_cannot_use_trivial_abi : ExtWarn<
   "'trivial_abi' cannot be applied to %0">, InGroup<IgnoredAttributes>;
+def note_cannot_use_trivial_abi_reason : Note<
+  "'trivial_abi' is disallowed on %0 because %select{"
+  "its copy constructors and move constructors are all deleted|"
+  "it is polymorphic|"
+  "it has a base of a non-trivial class type|it has a virtual base|"
+  "it has a __weak field|it has a field of a non-trivial class type}1">;
 
 // Availability attribute
 def warn_availability_unknown_platform : Warning<
Index: clang/include/clang/Basic/AttrDocs.td
===================================================================
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -2810,6 +2810,7 @@
 Attribute ``trivial_abi`` has no effect in the following cases:
 
 - The class directly declares a virtual base or virtual methods.
+- Copy constructors and move constructors of the class are all deleted.
 - The class has a base class that is non-trivial for the purposes of calls.
 - The class has a non-static data member whose type is non-trivial for the purposes of calls, which includes:
 

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits