[cfe-commits] r153883 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclCXX.cpp test/CXX/special/class.copy/implicit-move.cpp test/CXX/special/class.copy/p11.0x.move.cpp
Richard Smith
richard-llvm at metafoo.co.uk
Mon Apr 2 11:40:40 PDT 2012
Author: rsmith
Date: Mon Apr 2 13:40:40 2012
New Revision: 153883
URL: http://llvm.org/viewvc/llvm-project?rev=153883&view=rev
Log:
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.
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CXX/special/class.copy/implicit-move.cpp
cfe/trunk/test/CXX/special/class.copy/p11.0x.move.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=153883&r1=153882&r2=153883&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Apr 2 13:40:40 2012
@@ -2852,9 +2852,6 @@
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<
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=153883&r1=153882&r2=153883&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Apr 2 13:40:40 2012
@@ -4651,8 +4651,9 @@
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)
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=153883&r1=153882&r2=153883&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Apr 2 13:40:40 2012
@@ -4472,28 +4472,9 @@
// -- 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 @@
/// 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 @@
(!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 @@
DeclareImplicitCopyAssignment(ClassDecl);
}
- if (getLangOpts().CPlusPlus0x && ClassDecl->needsImplicitMoveAssignment()){
+ if (getLangOpts().CPlusPlus0x && ClassDecl->needsImplicitMoveAssignment()) {
++ASTContext::NumImplicitMoveAssignmentOperators;
// Likewise for the move assignment operator.
@@ -7396,8 +7366,8 @@
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 @@
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 @@
// 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 @@
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));
Modified: cfe/trunk/test/CXX/special/class.copy/implicit-move.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.copy/implicit-move.cpp?rev=153883&r1=153882&r2=153883&view=diff
==============================================================================
--- cfe/trunk/test/CXX/special/class.copy/implicit-move.cpp (original)
+++ cfe/trunk/test/CXX/special/class.copy/implicit-move.cpp Mon Apr 2 13:40:40 2012
@@ -162,3 +162,75 @@
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}}
+ };
+}
Modified: cfe/trunk/test/CXX/special/class.copy/p11.0x.move.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.copy/p11.0x.move.cpp?rev=153883&r1=153882&r2=153883&view=diff
==============================================================================
--- cfe/trunk/test/CXX/special/class.copy/p11.0x.move.cpp (original)
+++ cfe/trunk/test/CXX/special/class.copy/p11.0x.move.cpp Mon Apr 2 13:40:40 2012
@@ -123,7 +123,7 @@
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 @@
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);
+ }
+}
More information about the cfe-commits
mailing list