Implement DR1402: if a field or base class is not movable, the derived class's

move constructor/move assignment operator are not declared, rather than being
defined as deleted, so move operations on the derived class fall back to
copying rather than moving.

If a move operation on the derived class is explicitly defaulted, the
unmovable subobject will be copied instead of being moved.


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

5 files changed, 241 insertions, 47 deletions

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 863f9db164..10b3819396 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2852,9 +2852,6 @@ def note_deleted_copy_user_declared_move : Note<
 def note_deleted_assign_field : Note<
   "%select{copy|move}0 assignment operator of %0 is implicitly deleted "
   "because field %1 is of %select{reference|const-qualified}3 type %2">;
-def note_deleted_move_assign_virtual_base : Note<
-  "move assignment operator of %0 is implicitly deleted because it has a "
-  "virtual base class %1">;
 
 // This should eventually be an error.
 def warn_undefined_internal : Warning<
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 9e193bebbe..cc75f3f4aa 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -4651,8 +4651,9 @@ static NamedDecl* DiagnoseInvalidRedeclaration(
 
     if (unsigned Idx = NearMatch->second) {
       ParmVarDecl *FDParam = FD->getParamDecl(Idx-1);
-      SemaRef.Diag(FDParam->getTypeSpecStartLoc(),
-             diag::note_member_def_close_param_match)
+      SourceLocation Loc = FDParam->getTypeSpecStartLoc();
+      if (Loc.isInvalid()) Loc = FD->getLocation();
+      SemaRef.Diag(Loc, diag::note_member_def_close_param_match)
           << Idx << FDParam->getType() << NewFD->getParamDecl(Idx-1)->getType();
     } else if (Correction) {
       SemaRef.Diag(FD->getLocation(), diag::note_previous_decl)
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index c6cd9a2479..856f921a78 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -4472,28 +4472,9 @@ bool SpecialMemberDeletionInfo::shouldDeleteForClassSubobject(
   // -- any direct or virtual base class [...] has a type with a destructor
   //    that is deleted or inaccessible
   if (!(CSM == Sema::CXXDefaultConstructor &&
-        Field && Field->hasInClassInitializer())) {
-    Sema::SpecialMemberOverloadResult *SMOR = lookupIn(Class);
-    CXXMethodDecl *Member = SMOR->getMethod();
-    if (shouldDeleteForSubobjectCall(Subobj, SMOR, false))
-      return true;
-
-    // FIXME: CWG 1402 moves these bullets elsewhere.
-    if (CSM == Sema::CXXMoveConstructor) {
-      CXXConstructorDecl *Ctor = cast<CXXConstructorDecl>(Member);
-      // -- for the move constructor, a [...] direct or virtual base class with
-      //    a type that does not have a move constructor and is not trivially
-      //    copyable.
-      if (!Ctor->isMoveConstructor() && !Class->isTriviallyCopyable())
-        return true;
-    } else if (CSM == Sema::CXXMoveAssignment) {
-      // -- for the move assignment operator, a direct base class with a type
-      //    that does not have a move assignment operator and is not trivially
-      //    copyable.
-      if (!Member->isMoveAssignmentOperator() && !Class->isTriviallyCopyable())
-        return true;
-    }
-  }
+        Field && Field->hasInClassInitializer()) &&
+      shouldDeleteForSubobjectCall(Subobj, lookupIn(Class), false))
+    return true;
 
   // C++11 [class.ctor]p5, C++11 [class.copy]p11:
   // -- any direct or virtual base class or non-static data member has a
@@ -4512,21 +4493,8 @@ bool SpecialMemberDeletionInfo::shouldDeleteForClassSubobject(
 /// Check whether we should delete a special member function due to the class
 /// having a particular direct or virtual base class.
 bool SpecialMemberDeletionInfo::shouldDeleteForBase(CXXBaseSpecifier *Base) {
-  // C++11 [class.copy]p23:
-  // -- for the move assignment operator, any direct or indirect virtual
-  //    base class.
-  if (CSM == Sema::CXXMoveAssignment && Base->isVirtual()) {
-    if (Diagnose)
-      S.Diag(Base->getLocStart(), diag::note_deleted_move_assign_virtual_base)
-        << MD->getParent() << Base->getType();
-    return true;
-  }
-
-  if (shouldDeleteForClassSubobject(Base->getType()->getAsCXXRecordDecl(),
-                                    Base))
-    return true;
-
-  return false;
+  CXXRecordDecl *BaseClass = Base->getType()->getAsCXXRecordDecl();
+  return shouldDeleteForClassSubobject(BaseClass, Base);
 }
 
 /// Check whether we should delete a special member function due to the class
@@ -4682,10 +4650,12 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM,
         (!getLangOpts().MicrosoftMode || CSM == CXXCopyConstructor)) {
       if (!Diagnose) return true;
       UserDeclaredMove = RD->getMoveConstructor();
+      assert(UserDeclaredMove);
     } else if (RD->hasUserDeclaredMoveAssignment() &&
                (!getLangOpts().MicrosoftMode || CSM == CXXCopyAssignment)) {
       if (!Diagnose) return true;
       UserDeclaredMove = RD->getMoveAssignmentOperator();
+      assert(UserDeclaredMove);
     }
 
     if (UserDeclaredMove) {
@@ -4887,7 +4857,7 @@ void Sema::AddImplicitlyDeclaredMembersToClass(CXXRecordDecl *ClassDecl) {
       DeclareImplicitCopyAssignment(ClassDecl);
   }
 
-  if (getLangOpts().CPlusPlus0x && ClassDecl->needsImplicitMoveAssignment()){
+  if (getLangOpts().CPlusPlus0x && ClassDecl->needsImplicitMoveAssignment()) {
     ++ASTContext::NumImplicitMoveAssignmentOperators;
 
     // Likewise for the move assignment operator.
@@ -7396,8 +7366,8 @@ BuildSingleCopyAssign(Sema &S, SourceLocation Loc, QualType T,
     while (F.hasNext()) {
       NamedDecl *D = F.next();
       if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(D))
-        if (Copying ? Method->isCopyAssignmentOperator() :
-                      Method->isMoveAssignmentOperator())
+        if (Method->isCopyAssignmentOperator() ||
+            (!Copying && Method->isMoveAssignmentOperator()))
           continue;
 
       F.erase();
@@ -8087,7 +8057,115 @@ Sema::ComputeDefaultedMoveAssignmentExceptionSpec(CXXRecordDecl *ClassDecl) {
   return ExceptSpec;
 }
 
+/// Determine whether the class type has any direct or indirect virtual base
+/// classes which have a non-trivial move assignment operator.
+static bool
+hasVirtualBaseWithNonTrivialMoveAssignment(Sema &S, CXXRecordDecl *ClassDecl) {
+  for (CXXRecordDecl::base_class_iterator Base = ClassDecl->vbases_begin(),
+                                          BaseEnd = ClassDecl->vbases_end();
+       Base != BaseEnd; ++Base) {
+    CXXRecordDecl *BaseClass =
+        cast<CXXRecordDecl>(Base->getType()->getAs<RecordType>()->getDecl());
+
+    // Try to declare the move assignment. If it would be deleted, then the
+    // class does not have a non-trivial move assignment.
+    if (BaseClass->needsImplicitMoveAssignment())
+      S.DeclareImplicitMoveAssignment(BaseClass);
+
+    // If the class has both a trivial move assignment and a non-trivial move
+    // assignment, hasTrivialMoveAssignment() is false.
+    if (BaseClass->hasDeclaredMoveAssignment() &&
+        !BaseClass->hasTrivialMoveAssignment())
+      return true;
+  }
+
+  return false;
+}
+
+/// Determine whether the given type either has a move constructor or is
+/// trivially copyable.
+static bool
+hasMoveOrIsTriviallyCopyable(Sema &S, QualType Type, bool IsConstructor) {
+  Type = S.Context.getBaseElementType(Type);
+
+  // FIXME: Technically, non-trivially-copyable non-class types, such as
+  // reference types, are supposed to return false here, but that appears
+  // to be a standard defect.
+  CXXRecordDecl *ClassDecl = Type->getAsCXXRecordDecl();
+  if (!ClassDecl)
+    return true;
+
+  if (Type.isTriviallyCopyableType(S.Context))
+    return true;
+
+  if (IsConstructor) {
+    if (ClassDecl->needsImplicitMoveConstructor())
+      S.DeclareImplicitMoveConstructor(ClassDecl);
+    return ClassDecl->hasDeclaredMoveConstructor();
+  }
+
+  if (ClassDecl->needsImplicitMoveAssignment())
+    S.DeclareImplicitMoveAssignment(ClassDecl);
+  return ClassDecl->hasDeclaredMoveAssignment();
+}
+
+/// Determine whether all non-static data members and direct or virtual bases
+/// of class \p ClassDecl have either a move operation, or are trivially
+/// copyable.
+static bool subobjectsHaveMoveOrTrivialCopy(Sema &S, CXXRecordDecl *ClassDecl,
+                                            bool IsConstructor) {
+  for (CXXRecordDecl::base_class_iterator Base = ClassDecl->bases_begin(),
+                                          BaseEnd = ClassDecl->bases_end();
+       Base != BaseEnd; ++Base) {
+    if (Base->isVirtual())
+      continue;
+
+    if (!hasMoveOrIsTriviallyCopyable(S, Base->getType(), IsConstructor))
+      return false;
+  }
+
+  for (CXXRecordDecl::base_class_iterator Base = ClassDecl->vbases_begin(),
+                                          BaseEnd = ClassDecl->vbases_end();
+       Base != BaseEnd; ++Base) {
+    if (!hasMoveOrIsTriviallyCopyable(S, Base->getType(), IsConstructor))
+      return false;
+  }
+
+  for (CXXRecordDecl::field_iterator Field = ClassDecl->field_begin(),
+                                     FieldEnd = ClassDecl->field_end();
+       Field != FieldEnd; ++Field) {
+    if (!hasMoveOrIsTriviallyCopyable(S, (*Field)->getType(), IsConstructor))
+      return false;
+  }
+
+  return true;
+}
+
 CXXMethodDecl *Sema::DeclareImplicitMoveAssignment(CXXRecordDecl *ClassDecl) {
+  // C++11 [class.copy]p20:
+  //   If the definition of a class X does not explicitly declare a move
+  //   assignment operator, one will be implicitly declared as defaulted
+  //   if and only if:
+  //
+  //   - [first 4 bullets]
+  assert(ClassDecl->needsImplicitMoveAssignment());
+
+  // [Checked after we build the declaration]
+  //   - the move assignment operator would not be implicitly defined as
+  //     deleted,
+
+  // [DR1402]:
+  //   - X has no direct or indirect virtual base class with a non-trivial
+  //     move assignment operator, and
+  //   - each of X's non-static data members and direct or virtual base classes
+  //     has a type that either has a move assignment operator or is trivially
+  //     copyable.
+  if (hasVirtualBaseWithNonTrivialMoveAssignment(*this, ClassDecl) ||
+      !subobjectsHaveMoveOrTrivialCopy(*this, ClassDecl,/*Constructor*/false)) {
+    ClassDecl->setFailedImplicitMoveAssignment();
+    return 0;
+  }
+
   // Note: The following rules are largely analoguous to the move
   // constructor rules.
 
@@ -8203,7 +8281,7 @@ void Sema::DefineImplicitMoveAssignment(SourceLocation CurrentLocation,
   // ASTs.
   Expr *This = ActOnCXXThis(Loc).takeAs<Expr>();
   assert(This && "Reference to this cannot fail!");
-  
+
   // Assign base classes.
   bool Invalid = false;
   for (CXXRecordDecl::base_class_iterator Base = ClassDecl->bases_begin(),
@@ -8734,6 +8812,25 @@ Sema::ComputeDefaultedMoveCtorExceptionSpec(CXXRecordDecl *ClassDecl) {
 
 CXXConstructorDecl *Sema::DeclareImplicitMoveConstructor(
                                                     CXXRecordDecl *ClassDecl) {
+  // C++11 [class.copy]p9:
+  //   If the definition of a class X does not explicitly declare a move
+  //   constructor, one will be implicitly declared as defaulted if and only if:
+  //
+  //   - [first 4 bullets]
+  assert(ClassDecl->needsImplicitMoveConstructor());
+
+  // [Checked after we build the declaration]
+  //   - the move assignment operator would not be implicitly defined as
+  //     deleted,
+
+  // [DR1402]:
+  //   - each of X's non-static data members and direct or virtual base classes
+  //     has a type that either has a move constructor or is trivially copyable.
+  if (!subobjectsHaveMoveOrTrivialCopy(*this, ClassDecl, /*Constructor*/true)) {
+    ClassDecl->setFailedImplicitMoveConstructor();
+    return 0;
+  }
+
   ImplicitExceptionSpecification Spec(
       ComputeDefaultedMoveCtorExceptionSpec(ClassDecl));
 
diff --git a/test/CXX/special/class.copy/implicit-move.cpp b/test/CXX/special/class.copy/implicit-move.cpp
index 9ff6f48fad..9cb4215cd0 100644
--- a/test/CXX/special/class.copy/implicit-move.cpp
+++ b/test/CXX/special/class.copy/implicit-move.cpp
@@ -162,3 +162,75 @@ struct ContainsRValueRef {
 void test_contains_rref() {
   (ContainsRValueRef(ContainsRValueRef()));
 }
+
+
+namespace DR1402 {
+  struct NonTrivialCopyCtor {
+    NonTrivialCopyCtor(const NonTrivialCopyCtor &);
+  };
+  struct NonTrivialCopyAssign {
+    NonTrivialCopyAssign &operator=(const NonTrivialCopyAssign &);
+  };
+
+  struct NonTrivialCopyCtorVBase : virtual NonTrivialCopyCtor {
+    NonTrivialCopyCtorVBase(NonTrivialCopyCtorVBase &&);
+    NonTrivialCopyCtorVBase &operator=(NonTrivialCopyCtorVBase &&) = default;
+  };
+  struct NonTrivialCopyAssignVBase : virtual NonTrivialCopyAssign {
+    NonTrivialCopyAssignVBase(NonTrivialCopyAssignVBase &&);
+    NonTrivialCopyAssignVBase &operator=(NonTrivialCopyAssignVBase &&) = default;
+  };
+
+  struct NonTrivialMoveAssign {
+    NonTrivialMoveAssign(NonTrivialMoveAssign&&);
+    NonTrivialMoveAssign &operator=(NonTrivialMoveAssign &&);
+  };
+  struct NonTrivialMoveAssignVBase : virtual NonTrivialMoveAssign {
+    NonTrivialMoveAssignVBase(NonTrivialMoveAssignVBase &&);
+    NonTrivialMoveAssignVBase &operator=(NonTrivialMoveAssignVBase &&) = default;
+  };
+
+  // A non-movable, non-trivially-copyable class type as a subobject inhibits
+  // the declaration of a move operation.
+  struct NoMove1 { NonTrivialCopyCtor ntcc; }; // expected-note 2{{'const DR1402::NoMove1 &'}}
+  struct NoMove2 { NonTrivialCopyAssign ntcc; }; // expected-note 2{{'const DR1402::NoMove2 &'}}
+  struct NoMove3 : NonTrivialCopyCtor {}; // expected-note 2{{'const DR1402::NoMove3 &'}}
+  struct NoMove4 : NonTrivialCopyAssign {}; // expected-note 2{{'const DR1402::NoMove4 &'}}
+  struct NoMove5 : virtual NonTrivialCopyCtor {}; // expected-note 2{{'const DR1402::NoMove5 &'}}
+  struct NoMove6 : virtual NonTrivialCopyAssign {}; // expected-note 2{{'const DR1402::NoMove6 &'}}
+  struct NoMove7 : NonTrivialCopyCtorVBase {}; // expected-note 2{{'DR1402::NoMove7 &'}}
+  struct NoMove8 : NonTrivialCopyAssignVBase {}; // expected-note 2{{'DR1402::NoMove8 &'}}
+
+  // A non-trivially-move-assignable virtual base class inhibits the declaration
+  // of a move assignment (which might move-assign the base class multiple
+  // times).
+  struct NoMove9 : NonTrivialMoveAssign {};
+  struct NoMove10 : virtual NonTrivialMoveAssign {}; // expected-note {{'DR1402::NoMove10 &'}}
+  struct NoMove11 : NonTrivialMoveAssignVBase {}; // expected-note {{'DR1402::NoMove11 &'}}
+
+  struct Test {
+    friend NoMove1::NoMove1(NoMove1 &&); // expected-error {{no matching function}}
+    friend NoMove2::NoMove2(NoMove2 &&); // expected-error {{no matching function}}
+    friend NoMove3::NoMove3(NoMove3 &&); // expected-error {{no matching function}}
+    friend NoMove4::NoMove4(NoMove4 &&); // expected-error {{no matching function}}
+    friend NoMove5::NoMove5(NoMove5 &&); // expected-error {{no matching function}}
+    friend NoMove6::NoMove6(NoMove6 &&); // expected-error {{no matching function}}
+    friend NoMove7::NoMove7(NoMove7 &&); // expected-error {{no matching function}}
+    friend NoMove8::NoMove8(NoMove8 &&); // expected-error {{no matching function}}
+    friend NoMove9::NoMove9(NoMove9 &&);
+    friend NoMove10::NoMove10(NoMove10 &&);
+    friend NoMove11::NoMove11(NoMove11 &&);
+
+    friend NoMove1 &NoMove1::operator=(NoMove1 &&); // expected-error {{no matching function}}
+    friend NoMove2 &NoMove2::operator=(NoMove2 &&); // expected-error {{no matching function}}
+    friend NoMove3 &NoMove3::operator=(NoMove3 &&); // expected-error {{no matching function}}
+    friend NoMove4 &NoMove4::operator=(NoMove4 &&); // expected-error {{no matching function}}
+    friend NoMove5 &NoMove5::operator=(NoMove5 &&); // expected-error {{no matching function}}
+    friend NoMove6 &NoMove6::operator=(NoMove6 &&); // expected-error {{no matching function}}
+    friend NoMove7 &NoMove7::operator=(NoMove7 &&); // expected-error {{no matching function}}
+    friend NoMove8 &NoMove8::operator=(NoMove8 &&); // expected-error {{no matching function}}
+    friend NoMove9 &NoMove9::operator=(NoMove9 &&);
+    friend NoMove10 &NoMove10::operator=(NoMove10 &&); // expected-error {{no matching function}}
+    friend NoMove11 &NoMove11::operator=(NoMove11 &&); // expected-error {{no matching function}}
+  };
+}
diff --git a/test/CXX/special/class.copy/p11.0x.move.cpp b/test/CXX/special/class.copy/p11.0x.move.cpp
index 3e9cc6b662..ff9478be8b 100644
--- a/test/CXX/special/class.copy/p11.0x.move.cpp
+++ b/test/CXX/special/class.copy/p11.0x.move.cpp
@@ -123,7 +123,7 @@ struct NonMove {
   CopyOnly CO;
   NonMove(NonMove&&);
 };
-NonMove::NonMove(NonMove&&) = default; // expected-error{{would delete}}
+NonMove::NonMove(NonMove&&) = default; // ok under DR1402
 
 struct Moveable {
   Moveable();
@@ -135,3 +135,30 @@ struct HasMove {
   HasMove(HasMove&&);
 };
 HasMove::HasMove(HasMove&&) = default;
+
+namespace DR1402 {
+  struct member {
+    member();
+    member(const member&);
+    member& operator=(const member&);
+    ~member();
+  };
+
+  struct A {
+    member m_;
+
+    A() = default;
+    A(const A&) = default;
+    A& operator=(const A&) = default;
+    A(A&&) = default;
+    A& operator=(A&&) = default;
+    ~A() = default;
+  };
+
+  // ok, A's explicitly-defaulted move operations copy m_.
+  void f() {
+    A a, b(a), c(static_cast<A&&>(a));
+    a = b;
+    b = static_cast<A&&>(c);
+  }
+}