[PATCH] D26822: Sema, CodeGen: Ensure that an implicit copy ctor is always available under the Microsoft C++ ABI.

[Peter Collingbourne via llvm-commits]

pcc created this revision.
pcc added a reviewer: rnk.
pcc added a subscriber: llvm-commits.

This is needed because whether the constructor is deleted can control whether
we pass structs by value directly.

To fix this properly we probably want a more direct way for CodeGen to ask
whether the constructor was deleted.

Fixes PR31049.


https://reviews.llvm.org/D26822

Files:
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/uncopyable-args.cpp


Index: clang/test/CodeGenCXX/uncopyable-args.cpp
===================================================================
--- clang/test/CodeGenCXX/uncopyable-args.cpp
+++ clang/test/CodeGenCXX/uncopyable-args.cpp
@@ -204,3 +204,34 @@
 
 // WIN64-LABEL: declare void @"\01?foo at two_copy_ctors@@YAXUB at 1@@Z"(%"struct.two_copy_ctors::B"*)
 }
+
+namespace definition_only {
+struct A {
+  A();
+  A(A &&o);
+  void *p;
+};
+void *foo(A a) { return a.p; }
+// WIN64-LABEL: define i8* @"\01?foo at definition_only@@YAPEAXUA at 1@@Z"{{.*}}
+// WIN64-NEXT: :
+// WIN64-NEXT: getelementptr
+// WIN64-NEXT: load
+}
+
+namespace deleted_by_member {
+struct B {
+  B();
+  B(B &&o) : p(o.p) {}
+  void *p;
+};
+struct A {
+  A();
+  B b;
+};
+void *foo(A a) { return a.b.p; }
+// WIN64-LABEL: define i8* @"\01?foo at deleted_by_member@@YAPEAXUA at 1@@Z"{{.*}}
+// WIN64-NEXT: :
+// WIN64-NEXT: getelementptr
+// WIN64-NEXT: getelementptr
+// WIN64-NEXT: load
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7387,8 +7387,15 @@
     // If the properties or semantics of the copy constructor couldn't be
     // determined while the class was being declared, force a declaration
     // of it now.
+    //
+    // We also need to declare the implicit copy constructor under the Microsoft
+    // C++ ABI because whether the constructor is deleted can control whether we
+    // pass the record by value directly (see MicrosoftCXXABI::getRecordArgABI).
+    // FIXME: We should provide a more direct way for CodeGen to ask whether the
+    // constructor was deleted.
     if (ClassDecl->needsOverloadResolutionForCopyConstructor() ||
-        ClassDecl->hasInheritedConstructor())
+        ClassDecl->hasInheritedConstructor() ||
+        Context.getTargetInfo().getCXXABI().isMicrosoft())
       DeclareImplicitCopyConstructor(ClassDecl);
   }
 
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===================================================================
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -830,25 +830,24 @@
         getContext().getTypeSize(RD->getTypeForDecl()) > 64)
       return RAA_Indirect;
 
+    // We need Sema to have determined whether the implicit copy constructor is
+    // deleted. FIXME: We should provide a more direct way for CodeGen to ask
+    // whether the constructor was deleted.
+    assert(!RD->needsImplicitCopyConstructor());
+
     // We have a trivial copy constructor or no copy constructors, but we have
     // to make sure it isn't deleted.
-    bool CopyDeleted = false;
     for (const CXXConstructorDecl *CD : RD->ctors()) {
       if (CD->isCopyConstructor()) {
         assert(CD->isTrivial());
         // We had at least one undeleted trivial copy ctor.  Return directly.
         if (!CD->isDeleted())
           return RAA_Default;
-        CopyDeleted = true;
       }
     }
 
     // The trivial copy constructor was deleted.  Return indirectly.
-    if (CopyDeleted)
-      return RAA_Indirect;
-
-    // There were no copy ctors.  Return in RAX.
-    return RAA_Default;
+    return RAA_Indirect;
   }
 
   llvm_unreachable("invalid enum");


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D26822.78440.patch
Type: text/x-patch
Size: 3250 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161118/822099eb/attachment.bin>