From: Diana Picus <diana.picus@linaro.org>
Date: Wed, 9 Aug 2017 12:22:25 +0000 (+0000)
Subject: Revert "PR19668, PR23034: Fix handling of move constructors and deleted copy construc... 
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7cf93adc1cf360d4b79939d155a330535fef5ebf;p=clang

Revert "PR19668, PR23034: Fix handling of move constructors and deleted copy constructors when deciding whether classes should be passed indirectly."

This reverts commit r310401 because it seems to have broken some ARM
bot(s).

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

diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h
index c39eaee9b1..9d64f0244e 100644
--- a/include/clang/AST/DeclCXX.h
+++ b/include/clang/AST/DeclCXX.h
@@ -375,7 +375,6 @@ class CXXRecordDecl : public RecordDecl {
     /// \brief These flags are \c true if a defaulted corresponding special
     /// member can't be fully analyzed without performing overload resolution.
     /// @{
-    unsigned NeedOverloadResolutionForCopyConstructor : 1;
     unsigned NeedOverloadResolutionForMoveConstructor : 1;
     unsigned NeedOverloadResolutionForMoveAssignment : 1;
     unsigned NeedOverloadResolutionForDestructor : 1;
@@ -384,7 +383,6 @@ class CXXRecordDecl : public RecordDecl {
     /// \brief These flags are \c true if an implicit defaulted corresponding
     /// special member would be defined as deleted.
     /// @{
-    unsigned DefaultedCopyConstructorIsDeleted : 1;
     unsigned DefaultedMoveConstructorIsDeleted : 1;
     unsigned DefaultedMoveAssignmentIsDeleted : 1;
     unsigned DefaultedDestructorIsDeleted : 1;
@@ -417,12 +415,6 @@ class CXXRecordDecl : public RecordDecl {
     /// constructor.
     unsigned HasDefaultedDefaultConstructor : 1;
 
-    /// \brief True if this class can be passed in a non-address-preserving
-    /// fashion (such as in registers) according to the C++ language rules.
-    /// This does not imply anything about how the ABI in use will actually
-    /// pass an object of this class.
-    unsigned CanPassInRegisters : 1;
-
     /// \brief True if a defaulted default constructor for this class would
     /// be constexpr.
     unsigned DefaultedDefaultConstructorIsConstexpr : 1;
@@ -819,50 +811,18 @@ public:
     return data().FirstFriend.isValid();
   }
 
-  /// \brief \c true if a defaulted copy constructor for this class would be
-  /// deleted.
-  bool defaultedCopyConstructorIsDeleted() const {
-    assert((!needsOverloadResolutionForCopyConstructor() ||
-            (data().DeclaredSpecialMembers & SMF_CopyConstructor)) &&
-           "this property has not yet been computed by Sema");
-    return data().DefaultedCopyConstructorIsDeleted;
-  }
-
-  /// \brief \c true if a defaulted move constructor for this class would be
-  /// deleted.
-  bool defaultedMoveConstructorIsDeleted() const {
-    assert((!needsOverloadResolutionForMoveConstructor() ||
-            (data().DeclaredSpecialMembers & SMF_MoveConstructor)) &&
-           "this property has not yet been computed by Sema");
-    return data().DefaultedMoveConstructorIsDeleted;
-  }
-
-  /// \brief \c true if a defaulted destructor for this class would be deleted.
-  bool defaultedDestructorIsDeleted() const {
-    return !data().DefaultedDestructorIsDeleted;
-  }
-
-  /// \brief \c true if we know for sure that this class has a single,
-  /// accessible, unambiguous copy constructor that is not deleted.
-  bool hasSimpleCopyConstructor() const {
-    return !hasUserDeclaredCopyConstructor() &&
-           !data().DefaultedCopyConstructorIsDeleted;
-  }
-
   /// \brief \c true if we know for sure that this class has a single,
   /// accessible, unambiguous move constructor that is not deleted.
   bool hasSimpleMoveConstructor() const {
     return !hasUserDeclaredMoveConstructor() && hasMoveConstructor() &&
            !data().DefaultedMoveConstructorIsDeleted;
   }
-
   /// \brief \c true if we know for sure that this class has a single,
   /// accessible, unambiguous move assignment operator that is not deleted.
   bool hasSimpleMoveAssignment() const {
     return !hasUserDeclaredMoveAssignment() && hasMoveAssignment() &&
            !data().DefaultedMoveAssignmentIsDeleted;
   }
-
   /// \brief \c true if we know for sure that this class has an accessible
   /// destructor that is not deleted.
   bool hasSimpleDestructor() const {
@@ -918,16 +878,7 @@ public:
   /// \brief Determine whether we need to eagerly declare a defaulted copy
   /// constructor for this class.
   bool needsOverloadResolutionForCopyConstructor() const {
-    // C++17 [class.copy.ctor]p6:
-    //   If the class definition declares a move constructor or move assignment
-    //   operator, the implicitly declared copy constructor is defined as
-    //   deleted.
-    // In MSVC mode, sometimes a declared move assignment does not delete an
-    // implicit copy constructor, so defer this choice to Sema.
-    if (data().UserDeclaredSpecialMembers &
-        (SMF_MoveConstructor | SMF_MoveAssignment))
-      return true;
-    return data().NeedOverloadResolutionForCopyConstructor;
+    return data().HasMutableFields;
   }
 
   /// \brief Determine whether an implicit copy constructor for this type
@@ -968,16 +919,7 @@ public:
            needsImplicitMoveConstructor();
   }
 
-  /// \brief Set that we attempted to declare an implicit copy
-  /// constructor, but overload resolution failed so we deleted it.
-  void setImplicitCopyConstructorIsDeleted() {
-    assert((data().DefaultedCopyConstructorIsDeleted ||
-            needsOverloadResolutionForCopyConstructor()) &&
-           "Copy constructor should not be deleted");
-    data().DefaultedCopyConstructorIsDeleted = true;
-  }
-
-  /// \brief Set that we attempted to declare an implicit move
+  /// \brief Set that we attempted to declare an implicitly move
   /// constructor, but overload resolution failed so we deleted it.
   void setImplicitMoveConstructorIsDeleted() {
     assert((data().DefaultedMoveConstructorIsDeleted ||
@@ -1374,18 +1316,6 @@ public:
     return data().HasIrrelevantDestructor;
   }
 
-  /// \brief Determine whether this class has at least one trivial, non-deleted
-  /// copy or move constructor.
-  bool canPassInRegisters() const {
-    return data().CanPassInRegisters;
-  }
-
-  /// \brief Set that we can pass this RecordDecl in registers.
-  // FIXME: This should be set as part of completeDefinition.
-  void setCanPassInRegisters(bool CanPass) {
-    data().CanPassInRegisters = CanPass;
-  }
-
   /// \brief Determine whether this class has a non-literal or/ volatile type
   /// non-static data member or base class.
   bool hasNonLiteralTypeFieldsOrBases() const {
diff --git a/lib/AST/ASTImporter.cpp b/lib/AST/ASTImporter.cpp
index 2c0bb11cc4..6e33b98d2f 100644
--- a/lib/AST/ASTImporter.cpp
+++ b/lib/AST/ASTImporter.cpp
@@ -956,16 +956,12 @@ bool ASTNodeImporter::ImportDefinition(RecordDecl *From, RecordDecl *To,
     ToData.HasUninitializedFields = FromData.HasUninitializedFields;
     ToData.HasInheritedConstructor = FromData.HasInheritedConstructor;
     ToData.HasInheritedAssignment = FromData.HasInheritedAssignment;
-    ToData.NeedOverloadResolutionForCopyConstructor
-      = FromData.NeedOverloadResolutionForCopyConstructor;
     ToData.NeedOverloadResolutionForMoveConstructor
       = FromData.NeedOverloadResolutionForMoveConstructor;
     ToData.NeedOverloadResolutionForMoveAssignment
       = FromData.NeedOverloadResolutionForMoveAssignment;
     ToData.NeedOverloadResolutionForDestructor
       = FromData.NeedOverloadResolutionForDestructor;
-    ToData.DefaultedCopyConstructorIsDeleted
-      = FromData.DefaultedCopyConstructorIsDeleted;
     ToData.DefaultedMoveConstructorIsDeleted
       = FromData.DefaultedMoveConstructorIsDeleted;
     ToData.DefaultedMoveAssignmentIsDeleted
@@ -977,7 +973,6 @@ bool ASTNodeImporter::ImportDefinition(RecordDecl *From, RecordDecl *To,
       = FromData.HasConstexprNonCopyMoveConstructor;
     ToData.HasDefaultedDefaultConstructor
       = FromData.HasDefaultedDefaultConstructor;
-    ToData.CanPassInRegisters = FromData.CanPassInRegisters;
     ToData.DefaultedDefaultConstructorIsConstexpr
       = FromData.DefaultedDefaultConstructorIsConstexpr;
     ToData.HasConstexprDefaultConstructor
diff --git a/lib/AST/DeclCXX.cpp b/lib/AST/DeclCXX.cpp
index fe10d1cdd9..5cab488822 100644
--- a/lib/AST/DeclCXX.cpp
+++ b/lib/AST/DeclCXX.cpp
@@ -55,18 +55,15 @@ CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D)
       HasOnlyCMembers(true), HasInClassInitializer(false),
       HasUninitializedReferenceMember(false), HasUninitializedFields(false),
       HasInheritedConstructor(false), HasInheritedAssignment(false),
-      NeedOverloadResolutionForCopyConstructor(false),
       NeedOverloadResolutionForMoveConstructor(false),
       NeedOverloadResolutionForMoveAssignment(false),
       NeedOverloadResolutionForDestructor(false),
-      DefaultedCopyConstructorIsDeleted(false),
       DefaultedMoveConstructorIsDeleted(false),
       DefaultedMoveAssignmentIsDeleted(false),
       DefaultedDestructorIsDeleted(false), HasTrivialSpecialMembers(SMF_All),
       DeclaredNonTrivialSpecialMembers(0), HasIrrelevantDestructor(true),
       HasConstexprNonCopyMoveConstructor(false),
       HasDefaultedDefaultConstructor(false),
-      CanPassInRegisters(false),
       DefaultedDefaultConstructorIsConstexpr(true),
       HasConstexprDefaultConstructor(false),
       HasNonLiteralTypeFieldsOrBases(false), ComputedVisibleConversions(false),
@@ -355,10 +352,8 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases,
       setHasVolatileMember(true);
 
     // Keep track of the presence of mutable fields.
-    if (BaseClassDecl->hasMutableFields()) {
+    if (BaseClassDecl->hasMutableFields())
       data().HasMutableFields = true;
-      data().NeedOverloadResolutionForCopyConstructor = true;
-    }
 
     if (BaseClassDecl->hasUninitializedReferenceMember())
       data().HasUninitializedReferenceMember = true;
@@ -411,8 +406,6 @@ void CXXRecordDecl::addedClassSubobject(CXXRecordDecl *Subobj) {
   //    -- a direct or virtual base class B that cannot be copied/moved [...]
   //    -- a non-static data member of class type M (or array thereof)
   //       that cannot be copied or moved [...]
-  if (!Subobj->hasSimpleCopyConstructor())
-    data().NeedOverloadResolutionForCopyConstructor = true;
   if (!Subobj->hasSimpleMoveConstructor())
     data().NeedOverloadResolutionForMoveConstructor = true;
 
@@ -433,7 +426,6 @@ void CXXRecordDecl::addedClassSubobject(CXXRecordDecl *Subobj) {
   //    -- any non-static data member has a type with a destructor
   //       that is deleted or inaccessible from the defaulted [ctor or dtor].
   if (!Subobj->hasSimpleDestructor()) {
-    data().NeedOverloadResolutionForCopyConstructor = true;
     data().NeedOverloadResolutionForMoveConstructor = true;
     data().NeedOverloadResolutionForDestructor = true;
   }
@@ -719,10 +711,8 @@ void CXXRecordDecl::addedMember(Decl *D) {
       data().IsStandardLayout = false;
 
     // Keep track of the presence of mutable fields.
-    if (Field->isMutable()) {
+    if (Field->isMutable())
       data().HasMutableFields = true;
-      data().NeedOverloadResolutionForCopyConstructor = true;
-    }
 
     // C++11 [class.union]p8, DR1460:
     //   If X is a union, a non-static data member of X that is not an anonymous
@@ -766,12 +756,6 @@ void CXXRecordDecl::addedMember(Decl *D) {
       //   A standard-layout class is a class that:
       //    -- has no non-static data members of type [...] reference,
       data().IsStandardLayout = false;
-
-      // C++1z [class.copy.ctor]p10:
-      //   A defaulted copy constructor for a class X is defined as deleted if X has:
-      //    -- a non-static data member of rvalue reference type
-      if (T->isRValueReferenceType())
-        data().DefaultedCopyConstructorIsDeleted = true;
     }
 
     if (!Field->hasInClassInitializer() && !Field->isMutable()) {
@@ -825,10 +809,6 @@ void CXXRecordDecl::addedMember(Decl *D) {
         // We may need to perform overload resolution to determine whether a
         // field can be moved if it's const or volatile qualified.
         if (T.getCVRQualifiers() & (Qualifiers::Const | Qualifiers::Volatile)) {
-          // We need to care about 'const' for the copy constructor because an
-          // implicit copy constructor might be declared with a non-const
-          // parameter.
-          data().NeedOverloadResolutionForCopyConstructor = true;
           data().NeedOverloadResolutionForMoveConstructor = true;
           data().NeedOverloadResolutionForMoveAssignment = true;
         }
@@ -839,8 +819,6 @@ void CXXRecordDecl::addedMember(Decl *D) {
         //    -- X is a union-like class that has a variant member with a
         //       non-trivial [corresponding special member]
         if (isUnion()) {
-          if (FieldRec->hasNonTrivialCopyConstructor())
-            data().DefaultedCopyConstructorIsDeleted = true;
           if (FieldRec->hasNonTrivialMoveConstructor())
             data().DefaultedMoveConstructorIsDeleted = true;
           if (FieldRec->hasNonTrivialMoveAssignment())
@@ -852,8 +830,6 @@ void CXXRecordDecl::addedMember(Decl *D) {
         // For an anonymous union member, our overload resolution will perform
         // overload resolution for its members.
         if (Field->isAnonymousStructOrUnion()) {
-          data().NeedOverloadResolutionForCopyConstructor |=
-              FieldRec->data().NeedOverloadResolutionForCopyConstructor;
           data().NeedOverloadResolutionForMoveConstructor |=
               FieldRec->data().NeedOverloadResolutionForMoveConstructor;
           data().NeedOverloadResolutionForMoveAssignment |=
@@ -939,10 +915,8 @@ void CXXRecordDecl::addedMember(Decl *D) {
         }
         
         // Keep track of the presence of mutable fields.
-        if (FieldRec->hasMutableFields()) {
+        if (FieldRec->hasMutableFields())
           data().HasMutableFields = true;
-          data().NeedOverloadResolutionForCopyConstructor = true;
-        }
 
         // C++11 [class.copy]p13:
         //   If the implicitly-defined constructor would satisfy the
@@ -1476,7 +1450,7 @@ void CXXRecordDecl::completeDefinition() {
 
 void CXXRecordDecl::completeDefinition(CXXFinalOverriderMap *FinalOverriders) {
   RecordDecl::completeDefinition();
-
+  
   // If the class may be abstract (but hasn't been marked as such), check for
   // any pure final overriders.
   if (mayBeAbstract()) {
diff --git a/lib/CodeGen/CGCXXABI.cpp b/lib/CodeGen/CGCXXABI.cpp
index 033258643d..e29e525edd 100644
--- a/lib/CodeGen/CGCXXABI.cpp
+++ b/lib/CodeGen/CGCXXABI.cpp
@@ -30,9 +30,38 @@ void CGCXXABI::ErrorUnsupportedABI(CodeGenFunction &CGF, StringRef S) {
 }
 
 bool CGCXXABI::canCopyArgument(const CXXRecordDecl *RD) const {
+  // If RD has a non-trivial move or copy constructor, we cannot copy the
+  // argument.
+  if (RD->hasNonTrivialCopyConstructor() || RD->hasNonTrivialMoveConstructor())
+    return false;
+
+  // If RD has a non-trivial destructor, we cannot copy the argument.
+  if (RD->hasNonTrivialDestructor())
+    return false;
+
   // We can only copy the argument if there exists at least one trivial,
   // non-deleted copy or move constructor.
-  return RD->canPassInRegisters();
+  // FIXME: This assumes that all lazily declared copy and move constructors are
+  // not deleted.  This assumption might not be true in some corner cases.
+  bool CopyDeleted = false;
+  bool MoveDeleted = false;
+  for (const CXXConstructorDecl *CD : RD->ctors()) {
+    if (CD->isCopyConstructor() || CD->isMoveConstructor()) {
+      assert(CD->isTrivial());
+      // We had at least one undeleted trivial copy or move ctor.  Return
+      // directly.
+      if (!CD->isDeleted())
+        return true;
+      if (CD->isCopyConstructor())
+        CopyDeleted = true;
+      else
+        MoveDeleted = true;
+    }
+  }
+
+  // If all trivial copy and move constructors are deleted, we cannot copy the
+  // argument.
+  return !(CopyDeleted && MoveDeleted);
 }
 
 llvm::Constant *CGCXXABI::GetBogusMemberPointer(QualType T) {
diff --git a/lib/CodeGen/ItaniumCXXABI.cpp b/lib/CodeGen/ItaniumCXXABI.cpp
index 5c33f8c1af..c1f892a317 100644
--- a/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/lib/CodeGen/ItaniumCXXABI.cpp
@@ -63,8 +63,11 @@ public:
   bool classifyReturnType(CGFunctionInfo &FI) const override;
 
   RecordArgABI getRecordArgABI(const CXXRecordDecl *RD) const override {
-    // If C++ prohibits us from making a copy, pass by address.
-    if (!canCopyArgument(RD))
+    // 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())
       return RAA_Indirect;
     return RAA_Default;
   }
@@ -1011,8 +1014,10 @@ bool ItaniumCXXABI::classifyReturnType(CGFunctionInfo &FI) const {
   if (!RD)
     return false;
 
-  // If C++ prohibits us from making a copy, return by address.
-  if (!canCopyArgument(RD)) {
+  // 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()) {
     auto Align = CGM.getContext().getTypeAlignInChars(FI.getReturnType());
     FI.getReturnInfo() = ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
     return true;
diff --git a/lib/CodeGen/MicrosoftCXXABI.cpp b/lib/CodeGen/MicrosoftCXXABI.cpp
index 0a3cc30665..409bad72ee 100644
--- a/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -819,44 +819,46 @@ MicrosoftCXXABI::getRecordArgABI(const CXXRecordDecl *RD) const {
     return RAA_Default;
 
   case llvm::Triple::x86_64:
-    // If a class has a destructor, we'd really like to pass it indirectly
+    // Win64 passes objects with non-trivial copy ctors indirectly.
+    if (RD->hasNonTrivialCopyConstructor())
+      return RAA_Indirect;
+
+    // If an object has a destructor, we'd really like to pass it indirectly
     // because it allows us to elide copies.  Unfortunately, MSVC makes that
     // impossible for small types, which it will pass in a single register or
     // stack slot. Most objects with dtors are large-ish, so handle that early.
     // We can't call out all large objects as being indirect because there are
     // multiple x64 calling conventions and the C++ ABI code shouldn't dictate
     // how we pass large POD types.
-    //
-    // Note: This permits small classes with nontrivial destructors to be
-    // passed in registers, which is non-conforming.
     if (RD->hasNonTrivialDestructor() &&
         getContext().getTypeSize(RD->getTypeForDecl()) > 64)
       return RAA_Indirect;
 
-    // If a class has at least one non-deleted, trivial copy constructor, it
-    // is passed according to the C ABI. Otherwise, it is passed indirectly.
-    //
-    // Note: This permits classes with non-trivial copy or move ctors to be
-    // passed in registers, so long as they *also* have a trivial copy ctor,
-    // which is non-conforming.
-    if (RD->needsImplicitCopyConstructor()) {
-      // If the copy ctor has not yet been declared, we can read its triviality
-      // off the AST.
-      if (!RD->defaultedCopyConstructorIsDeleted() &&
-          RD->hasTrivialCopyConstructor())
-        return RAA_Default;
-    } else {
-      // Otherwise, we need to find the copy constructor(s) and ask.
-      for (const CXXConstructorDecl *CD : RD->ctors()) {
-        if (CD->isCopyConstructor()) {
-          // We had at least one nondeleted trivial copy ctor.  Return directly.
-          if (!CD->isDeleted() && CD->isTrivial())
-            return RAA_Default;
-        }
+    // If this is true, the implicit copy constructor that Sema would have
+    // created would not be deleted. FIXME: We should provide a more direct way
+    // for CodeGen to ask whether the constructor was deleted.
+    if (!RD->hasUserDeclaredCopyConstructor() &&
+        !RD->hasUserDeclaredMoveConstructor() &&
+        !RD->needsOverloadResolutionForMoveConstructor() &&
+        !RD->hasUserDeclaredMoveAssignment() &&
+        !RD->needsOverloadResolutionForMoveAssignment())
+      return RAA_Default;
+
+    // Otherwise, Sema should have created an implicit copy constructor if
+    // needed.
+    assert(!RD->needsImplicitCopyConstructor());
+
+    // We have to make sure the trivial copy constructor isn't deleted.
+    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;
       }
     }
 
-    // We have no trivial, non-deleted copy constructor.
+    // The trivial copy constructor was deleted.  Return indirectly.
     return RAA_Indirect;
   }
 
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index c05e5f0207..e9070881af 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -5726,53 +5726,6 @@ static void DefineImplicitSpecialMember(Sema &S, CXXMethodDecl *MD,
   }
 }
 
-/// Determine whether a type is permitted to be passed or returned in
-/// registers, per C++ [class.temporary]p3.
-static bool computeCanPassInRegisters(Sema &S, CXXRecordDecl *D) {
-  if (D->isDependentType() || D->isInvalidDecl())
-    return false;
-
-  // Per C++ [class.temporary]p3, the relevant condition is:
-  //   each copy constructor, move constructor, and destructor of X is
-  //   either trivial or deleted, and X has at least one non-deleted copy
-  //   or move constructor
-  bool HasNonDeletedCopyOrMove = false;
-
-  if (D->needsImplicitCopyConstructor() &&
-      !D->defaultedCopyConstructorIsDeleted()) {
-    if (!D->hasTrivialCopyConstructor())
-      return false;
-    HasNonDeletedCopyOrMove = true; 
-  }
-
-  if (S.getLangOpts().CPlusPlus11 && D->needsImplicitMoveConstructor() &&
-      !D->defaultedMoveConstructorIsDeleted()) {
-    if (!D->hasTrivialMoveConstructor())
-      return false;
-    HasNonDeletedCopyOrMove = true;
-  }
-
-  if (D->needsImplicitDestructor() && !D->defaultedDestructorIsDeleted() &&
-      !D->hasTrivialDestructor())
-    return false;
-
-  for (const CXXMethodDecl *MD : D->methods()) {
-    if (MD->isDeleted())
-      continue;
-
-    auto *CD = dyn_cast<CXXConstructorDecl>(MD);
-    if (CD && CD->isCopyOrMoveConstructor())
-      HasNonDeletedCopyOrMove = true;
-    else if (!isa<CXXDestructorDecl>(MD))
-      continue;
-
-    if (!MD->isTrivial())
-      return false;
-  }
-
-  return HasNonDeletedCopyOrMove;
-}
-
 /// \brief Perform semantic checks on a class definition that has been
 /// completing, introducing implicitly-declared members, checking for
 /// abstract types, etc.
@@ -5917,8 +5870,6 @@ void Sema::CheckCompletedCXXClass(CXXRecordDecl *Record) {
   }
 
   checkClassLevelDLLAttribute(Record);
-
-  Record->setCanPassInRegisters(computeCanPassInRegisters(*this, Record));
 }
 
 /// Look up the special member function that would be called by a special
@@ -7545,7 +7496,8 @@ void Sema::ActOnFinishCXXMemberSpecification(Scope* S, SourceLocation RLoc,
               reinterpret_cast<Decl**>(FieldCollector->getCurFields()),
               FieldCollector->getCurNumFields()), LBrac, RBrac, AttrList);
 
-  CheckCompletedCXXClass(dyn_cast_or_null<CXXRecordDecl>(TagDecl));
+  CheckCompletedCXXClass(
+                        dyn_cast_or_null<CXXRecordDecl>(TagDecl));
 }
 
 /// AddImplicitlyDeclaredMembersToClass - Adds any implicitly-declared
@@ -11977,10 +11929,8 @@ CXXConstructorDecl *Sema::DeclareImplicitCopyConstructor(
   Scope *S = getScopeForContext(ClassDecl);
   CheckImplicitSpecialMemberDeclaration(S, CopyConstructor);
 
-  if (ShouldDeleteSpecialMember(CopyConstructor, CXXCopyConstructor)) {
-    ClassDecl->setImplicitCopyConstructorIsDeleted();
+  if (ShouldDeleteSpecialMember(CopyConstructor, CXXCopyConstructor))
     SetDeclDeleted(CopyConstructor, ClassLoc);
-  }
 
   if (S)
     PushOnScopeChains(CopyConstructor, S, false);
diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index 085341571c..abed258656 100644
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -1559,11 +1559,9 @@ void ASTDeclReader::ReadCXXDefinitionData(
   Data.HasUninitializedFields = Record.readInt();
   Data.HasInheritedConstructor = Record.readInt();
   Data.HasInheritedAssignment = Record.readInt();
-  Data.NeedOverloadResolutionForCopyConstructor = Record.readInt();
   Data.NeedOverloadResolutionForMoveConstructor = Record.readInt();
   Data.NeedOverloadResolutionForMoveAssignment = Record.readInt();
   Data.NeedOverloadResolutionForDestructor = Record.readInt();
-  Data.DefaultedCopyConstructorIsDeleted = Record.readInt();
   Data.DefaultedMoveConstructorIsDeleted = Record.readInt();
   Data.DefaultedMoveAssignmentIsDeleted = Record.readInt();
   Data.DefaultedDestructorIsDeleted = Record.readInt();
@@ -1572,7 +1570,6 @@ void ASTDeclReader::ReadCXXDefinitionData(
   Data.HasIrrelevantDestructor = Record.readInt();
   Data.HasConstexprNonCopyMoveConstructor = Record.readInt();
   Data.HasDefaultedDefaultConstructor = Record.readInt();
-  Data.CanPassInRegisters = Record.readInt();
   Data.DefaultedDefaultConstructorIsConstexpr = Record.readInt();
   Data.HasConstexprDefaultConstructor = Record.readInt();
   Data.HasNonLiteralTypeFieldsOrBases = Record.readInt();
@@ -1700,11 +1697,9 @@ void ASTDeclReader::MergeDefinitionData(
   MATCH_FIELD(HasUninitializedFields)
   MATCH_FIELD(HasInheritedConstructor)
   MATCH_FIELD(HasInheritedAssignment)
-  MATCH_FIELD(NeedOverloadResolutionForCopyConstructor)
   MATCH_FIELD(NeedOverloadResolutionForMoveConstructor)
   MATCH_FIELD(NeedOverloadResolutionForMoveAssignment)
   MATCH_FIELD(NeedOverloadResolutionForDestructor)
-  MATCH_FIELD(DefaultedCopyConstructorIsDeleted)
   MATCH_FIELD(DefaultedMoveConstructorIsDeleted)
   MATCH_FIELD(DefaultedMoveAssignmentIsDeleted)
   MATCH_FIELD(DefaultedDestructorIsDeleted)
@@ -1713,7 +1708,6 @@ void ASTDeclReader::MergeDefinitionData(
   MATCH_FIELD(HasIrrelevantDestructor)
   OR_FIELD(HasConstexprNonCopyMoveConstructor)
   OR_FIELD(HasDefaultedDefaultConstructor)
-  MATCH_FIELD(CanPassInRegisters)
   MATCH_FIELD(DefaultedDefaultConstructorIsConstexpr)
   OR_FIELD(HasConstexprDefaultConstructor)
   MATCH_FIELD(HasNonLiteralTypeFieldsOrBases)
diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp
index 62c9fb4620..9a739579b3 100644
--- a/lib/Serialization/ASTWriter.cpp
+++ b/lib/Serialization/ASTWriter.cpp
@@ -5875,11 +5875,9 @@ void ASTRecordWriter::AddCXXDefinitionData(const CXXRecordDecl *D) {
   Record->push_back(Data.HasUninitializedFields);
   Record->push_back(Data.HasInheritedConstructor);
   Record->push_back(Data.HasInheritedAssignment);
-  Record->push_back(Data.NeedOverloadResolutionForCopyConstructor);
   Record->push_back(Data.NeedOverloadResolutionForMoveConstructor);
   Record->push_back(Data.NeedOverloadResolutionForMoveAssignment);
   Record->push_back(Data.NeedOverloadResolutionForDestructor);
-  Record->push_back(Data.DefaultedCopyConstructorIsDeleted);
   Record->push_back(Data.DefaultedMoveConstructorIsDeleted);
   Record->push_back(Data.DefaultedMoveAssignmentIsDeleted);
   Record->push_back(Data.DefaultedDestructorIsDeleted);
@@ -5888,7 +5886,6 @@ void ASTRecordWriter::AddCXXDefinitionData(const CXXRecordDecl *D) {
   Record->push_back(Data.HasIrrelevantDestructor);
   Record->push_back(Data.HasConstexprNonCopyMoveConstructor);
   Record->push_back(Data.HasDefaultedDefaultConstructor);
-  Record->push_back(Data.CanPassInRegisters);
   Record->push_back(Data.DefaultedDefaultConstructorIsConstexpr);
   Record->push_back(Data.HasConstexprDefaultConstructor);
   Record->push_back(Data.HasNonLiteralTypeFieldsOrBases);
diff --git a/test/CodeGenCXX/uncopyable-args.cpp b/test/CodeGenCXX/uncopyable-args.cpp
index ef7168cdaa..307a5cf11b 100644
--- a/test/CodeGenCXX/uncopyable-args.cpp
+++ b/test/CodeGenCXX/uncopyable-args.cpp
@@ -1,6 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
-// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s -fms-compatibility -fms-compatibility-version=18 | FileCheck %s -check-prefix=WIN64 -check-prefix=WIN64-18
-// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s -fms-compatibility -fms-compatibility-version=19 | FileCheck %s -check-prefix=WIN64 -check-prefix=WIN64-19
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s | FileCheck %s -check-prefix=WIN64
 
 namespace trivial {
 // Trivial structs should be passed directly.
@@ -53,11 +52,12 @@ void foo(A);
 void bar() {
   foo({});
 }
-// CHECK-LABEL: define void @_ZN9move_ctor3barEv()
-// CHECK: call void @_Z{{.*}}C1Ev(
-// CHECK-NOT: call
-// CHECK: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
-// CHECK-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+// FIXME: The copy ctor is implicitly deleted.
+// CHECK-DISABLED-LABEL: define void @_ZN9move_ctor3barEv()
+// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
+// CHECK-DISABLED-NOT: call
+// CHECK-DISABLED: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
+// CHECK-DISABLED-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
 
 // WIN64-LABEL: declare void @"\01?foo@move_ctor@@YAXUA@1@@Z"(%"struct.move_ctor::A"*)
 }
@@ -73,11 +73,12 @@ void foo(A);
 void bar() {
   foo({});
 }
-// CHECK-LABEL: define void @_ZN11all_deleted3barEv()
-// CHECK: call void @_Z{{.*}}C1Ev(
-// CHECK-NOT: call
-// CHECK: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
-// CHECK-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
+// FIXME: The copy ctor is deleted.
+// CHECK-DISABLED-LABEL: define void @_ZN11all_deleted3barEv()
+// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
+// CHECK-DISABLED-NOT: call
+// CHECK-DISABLED: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
+// CHECK-DISABLED-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
 
 // WIN64-LABEL: declare void @"\01?foo@all_deleted@@YAXUA@1@@Z"(%"struct.all_deleted::A"*)
 }
@@ -92,15 +93,14 @@ void foo(A);
 void bar() {
   foo({});
 }
-// CHECK-LABEL: define void @_ZN18implicitly_deleted3barEv()
-// CHECK: call void @_Z{{.*}}C1Ev(
-// CHECK-NOT: call
-// CHECK: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
-// CHECK-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
-
-// In MSVC 2013, the copy ctor is not deleted by a move assignment. In MSVC 2015, it is.
-// WIN64-18-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(i64
-// WIN64-19-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(%"struct.implicitly_deleted::A"*)
+// FIXME: The copy and move ctors are implicitly deleted.
+// CHECK-DISABLED-LABEL: define void @_ZN18implicitly_deleted3barEv()
+// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
+// CHECK-DISABLED-NOT: call
+// CHECK-DISABLED: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
+// CHECK-DISABLED-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
+
+// WIN64-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(%"struct.implicitly_deleted::A"*)
 }
 
 namespace one_deleted {
@@ -113,11 +113,12 @@ void foo(A);
 void bar() {
   foo({});
 }
-// CHECK-LABEL: define void @_ZN11one_deleted3barEv()
-// CHECK: call void @_Z{{.*}}C1Ev(
-// CHECK-NOT: call
-// CHECK: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
-// CHECK-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
+// FIXME: The copy constructor is implicitly deleted.
+// CHECK-DISABLED-LABEL: define void @_ZN11one_deleted3barEv()
+// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
+// CHECK-DISABLED-NOT: call
+// CHECK-DISABLED: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
+// CHECK-DISABLED-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
 
 // WIN64-LABEL: declare void @"\01?foo@one_deleted@@YAXUA@1@@Z"(%"struct.one_deleted::A"*)
 }
@@ -194,10 +195,12 @@ void foo(B);
 void bar() {
   foo({});
 }
-// CHECK-LABEL: define void @_ZN14two_copy_ctors3barEv()
-// CHECK: call void @_Z{{.*}}C1Ev(
-// CHECK: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
-// CHECK-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
+// FIXME: This class has a non-trivial copy ctor and a trivial copy ctor.  It's
+// not clear whether we should pass by address or in registers.
+// CHECK-DISABLED-LABEL: define void @_ZN14two_copy_ctors3barEv()
+// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
+// CHECK-DISABLED: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
+// CHECK-DISABLED-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
 
 // WIN64-LABEL: declare void @"\01?foo@two_copy_ctors@@YAXUB@1@@Z"(%"struct.two_copy_ctors::B"*)
 }
@@ -209,7 +212,6 @@ struct A {
   void *p;
 };
 void *foo(A a) { return a.p; }
-// CHECK-LABEL: define i8* @_ZN15definition_only3fooENS_1AE(%"struct.definition_only::A"*
 // WIN64-LABEL: define i8* @"\01?foo@definition_only@@YAPEAXUA@1@@Z"(%"struct.definition_only::A"*
 }
 
@@ -224,7 +226,6 @@ struct A {
   B b;
 };
 void *foo(A a) { return a.b.p; }
-// CHECK-LABEL: define i8* @_ZN17deleted_by_member3fooENS_1AE(%"struct.deleted_by_member::A"*
 // WIN64-LABEL: define i8* @"\01?foo@deleted_by_member@@YAPEAXUA@1@@Z"(%"struct.deleted_by_member::A"*
 }
 
@@ -238,7 +239,6 @@ struct A : B {
   A();
 };
 void *foo(A a) { return a.p; }
-// CHECK-LABEL: define i8* @_ZN15deleted_by_base3fooENS_1AE(%"struct.deleted_by_base::A"*
 // WIN64-LABEL: define i8* @"\01?foo@deleted_by_base@@YAPEAXUA@1@@Z"(%"struct.deleted_by_base::A"*
 }
 
@@ -253,7 +253,6 @@ struct A {
   B b;
 };
 void *foo(A a) { return a.b.p; }
-// CHECK-LABEL: define i8* @_ZN22deleted_by_member_copy3fooENS_1AE(%"struct.deleted_by_member_copy::A"*
 // WIN64-LABEL: define i8* @"\01?foo@deleted_by_member_copy@@YAPEAXUA@1@@Z"(%"struct.deleted_by_member_copy::A"*
 }
 
@@ -267,7 +266,6 @@ struct A : B {
   A();
 };
 void *foo(A a) { return a.p; }
-// CHECK-LABEL: define i8* @_ZN20deleted_by_base_copy3fooENS_1AE(%"struct.deleted_by_base_copy::A"*
 // WIN64-LABEL: define i8* @"\01?foo@deleted_by_base_copy@@YAPEAXUA@1@@Z"(%"struct.deleted_by_base_copy::A"*
 }
 
@@ -277,75 +275,6 @@ struct A {
   A(const A &o) = delete;
   void *p;
 };
-// CHECK-LABEL: define i8* @_ZN15explicit_delete3fooENS_1AE(%"struct.explicit_delete::A"*
 // WIN64-LABEL: define i8* @"\01?foo@explicit_delete@@YAPEAXUA@1@@Z"(%"struct.explicit_delete::A"*
 void *foo(A a) { return a.p; }
 }
-
-namespace implicitly_deleted_copy_ctor {
-struct A {
-  // No move ctor due to copy assignment.
-  A &operator=(const A&);
-  // Deleted copy ctor due to rvalue ref member.
-  int &&ref;
-};
-// CHECK-LABEL: define {{.*}} @_ZN28implicitly_deleted_copy_ctor3fooENS_1AE(%"struct.implicitly_deleted_copy_ctor::A"*
-// WIN64-LABEL: define {{.*}} @"\01?foo@implicitly_deleted_copy_ctor@@YAAEAHUA@1@@Z"(%"struct.implicitly_deleted_copy_ctor::A"*
-int &foo(A a) { return a.ref; }
-
-struct B {
-  // Passed direct: has non-deleted trivial copy ctor.
-  B &operator=(const B&);
-  int &ref;
-};
-int &foo(B b) { return b.ref; }
-// CHECK-LABEL: define {{.*}} @_ZN28implicitly_deleted_copy_ctor3fooENS_1BE(i32*
-// WIN64-LABEL: define {{.*}} @"\01?foo@implicitly_deleted_copy_ctor@@YAAEAHUB@1@@Z"(i64
-
-struct X { X(const X&); };
-struct Y { Y(const Y&) = default; };
-
-union C {
-  C &operator=(const C&);
-  // Passed indirect: copy ctor deleted due to variant member with nontrivial copy ctor.
-  X x;
-  int n;
-};
-int foo(C c) { return c.n; }
-// CHECK-LABEL: define {{.*}} @_ZN28implicitly_deleted_copy_ctor3fooENS_1CE(%"union.implicitly_deleted_copy_ctor::C"*
-// WIN64-LABEL: define {{.*}} @"\01?foo@implicitly_deleted_copy_ctor@@YAHTC@1@@Z"(%"union.implicitly_deleted_copy_ctor::C"*
-
-struct D {
-  D &operator=(const D&);
-  // Passed indirect: copy ctor deleted due to variant member with nontrivial copy ctor.
-  union {
-    X x;
-    int n;
-  };
-};
-int foo(D d) { return d.n; }
-// CHECK-LABEL: define {{.*}} @_ZN28implicitly_deleted_copy_ctor3fooENS_1DE(%"struct.implicitly_deleted_copy_ctor::D"*
-// WIN64-LABEL: define {{.*}} @"\01?foo@implicitly_deleted_copy_ctor@@YAHUD@1@@Z"(%"struct.implicitly_deleted_copy_ctor::D"*
-
-union E {
-  // Passed direct: has non-deleted trivial copy ctor.
-  E &operator=(const E&);
-  Y y;
-  int n;
-};
-int foo(E e) { return e.n; }
-// CHECK-LABEL: define {{.*}} @_ZN28implicitly_deleted_copy_ctor3fooENS_1EE(i32
-// WIN64-LABEL: define {{.*}} @"\01?foo@implicitly_deleted_copy_ctor@@YAHTE@1@@Z"(i32
-
-struct F {
-  // Passed direct: has non-deleted trivial copy ctor.
-  F &operator=(const F&);
-  union {
-    Y y;
-    int n;
-  };
-};
-int foo(F f) { return f.n; }
-// CHECK-LABEL: define {{.*}} @_ZN28implicitly_deleted_copy_ctor3fooENS_1FE(i32
-// WIN64-LABEL: define {{.*}} @"\01?foo@implicitly_deleted_copy_ctor@@YAHUF@1@@Z"(i32
-}
diff --git a/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp b/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
index 7bc8421bab..6037127feb 100644
--- a/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ b/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1108,35 +1108,26 @@ TEST(ConstructorDeclaration, IsExplicit) {
 }
 
 TEST(ConstructorDeclaration, Kinds) {
-  EXPECT_TRUE(matches(
-      "struct S { S(); };",
-      cxxConstructorDecl(isDefaultConstructor(), unless(isImplicit()))));
-  EXPECT_TRUE(notMatches(
-      "struct S { S(); };",
-      cxxConstructorDecl(isCopyConstructor(), unless(isImplicit()))));
-  EXPECT_TRUE(notMatches(
-      "struct S { S(); };",
-      cxxConstructorDecl(isMoveConstructor(), unless(isImplicit()))));
-
-  EXPECT_TRUE(notMatches(
-      "struct S { S(const S&); };",
-      cxxConstructorDecl(isDefaultConstructor(), unless(isImplicit()))));
-  EXPECT_TRUE(matches(
-      "struct S { S(const S&); };",
-      cxxConstructorDecl(isCopyConstructor(), unless(isImplicit()))));
-  EXPECT_TRUE(notMatches(
-      "struct S { S(const S&); };",
-      cxxConstructorDecl(isMoveConstructor(), unless(isImplicit()))));
-
-  EXPECT_TRUE(notMatches(
-      "struct S { S(S&&); };",
-      cxxConstructorDecl(isDefaultConstructor(), unless(isImplicit()))));
-  EXPECT_TRUE(notMatches(
-      "struct S { S(S&&); };",
-      cxxConstructorDecl(isCopyConstructor(), unless(isImplicit()))));
-  EXPECT_TRUE(matches(
-      "struct S { S(S&&); };",
-      cxxConstructorDecl(isMoveConstructor(), unless(isImplicit()))));
+  EXPECT_TRUE(matches("struct S { S(); };",
+                      cxxConstructorDecl(isDefaultConstructor())));
+  EXPECT_TRUE(notMatches("struct S { S(); };",
+                         cxxConstructorDecl(isCopyConstructor())));
+  EXPECT_TRUE(notMatches("struct S { S(); };",
+                         cxxConstructorDecl(isMoveConstructor())));
+
+  EXPECT_TRUE(notMatches("struct S { S(const S&); };",
+                         cxxConstructorDecl(isDefaultConstructor())));
+  EXPECT_TRUE(matches("struct S { S(const S&); };",
+                      cxxConstructorDecl(isCopyConstructor())));
+  EXPECT_TRUE(notMatches("struct S { S(const S&); };",
+                         cxxConstructorDecl(isMoveConstructor())));
+
+  EXPECT_TRUE(notMatches("struct S { S(S&&); };",
+                         cxxConstructorDecl(isDefaultConstructor())));
+  EXPECT_TRUE(notMatches("struct S { S(S&&); };",
+                         cxxConstructorDecl(isCopyConstructor())));
+  EXPECT_TRUE(matches("struct S { S(S&&); };",
+                      cxxConstructorDecl(isMoveConstructor())));
 }
 
 TEST(ConstructorDeclaration, IsUserProvided) {