PR19668, PR23034: Fix handling of move constructors and deleted copy

constructors when deciding whether classes should be passed indirectly.

This fixes ABI differences between Clang and GCC:

 * Previously, Clang ignored the move constructor when making this
   determination. It now takes the move constructor into account, per
   https://github.com/itanium-cxx-abi/cxx-abi/pull/17 (this change may
   seem recent, but the ABI change was agreed on the Itanium C++ ABI
   list a long time ago).

 * Previously, Clang's behavior when the copy constructor was deleted
   was unstable -- depending on whether the lazy declaration of the
   copy constructor had been triggered, you might get different behavior.
   We now eagerly declare the copy constructor whenever its deletedness
   is unclear, and ignore deleted copy/move constructors when looking for
   a trivial such constructor.

This also fixes an ABI difference between Clang and MSVC:

 * If the copy constructor would be implicitly deleted (but has not been
   lazily declared yet), for instance because the class has an rvalue
   reference member, we would pass it directly. We now pass such a class
   indirectly, matching MSVC.

Based on a patch by Vassil Vassilev, which was based on a patch by Bernd
Schmidt, which was based on a patch by Reid Kleckner!

This is a re-commit of r310401, which was reverted in r310464 due to ARM
failures (which should now be fixed).


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@310983 91177308-0d34-0410-b5e6-96231b3b80d8

1 files changed, 4 insertions, 9 deletions

diff --git a/lib/CodeGen/ItaniumCXXABI.cpp b/lib/CodeGen/ItaniumCXXABI.cpp
index c1f892a317..5c33f8c1af 100644
--- a/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/lib/CodeGen/ItaniumCXXABI.cpp
@@ -63,11 +63,8 @@ public:
   bool classifyReturnType(CGFunctionInfo &FI) const override;
 
   RecordArgABI getRecordArgABI(const CXXRecordDecl *RD) const override {
-    // Structures with either a non-trivial destructor or a non-trivial
-    // copy constructor are always indirect.
-    // FIXME: Use canCopyArgument() when it is fixed to handle lazily declared
-    // special members.
-    if (RD->hasNonTrivialDestructor() || RD->hasNonTrivialCopyConstructor())
+    // If C++ prohibits us from making a copy, pass by address.
+    if (!canCopyArgument(RD))
       return RAA_Indirect;
     return RAA_Default;
   }
@@ -1014,10 +1011,8 @@ bool ItaniumCXXABI::classifyReturnType(CGFunctionInfo &FI) const {
   if (!RD)
     return false;
 
-  // Return indirectly if we have a non-trivial copy ctor or non-trivial dtor.
-  // FIXME: Use canCopyArgument() when it is fixed to handle lazily declared
-  // special members.
-  if (RD->hasNonTrivialDestructor() || RD->hasNonTrivialCopyConstructor()) {
+  // If C++ prohibits us from making a copy, return by address.
+  if (!canCopyArgument(RD)) {
     auto Align = CGM.getContext().getTypeAlignInChars(FI.getReturnType());
     FI.getReturnInfo() = ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
     return true;