Implement the preference for move-construction over copy-construction
when returning an NRVO candidate expression. For example, this
properly picks the move constructor when dealing with code such as
MoveOnlyType f() { MoveOnlyType mot; return mot; }
The previously-XFAIL'd rvalue-references test case now works, and has
been moved into the appropriate paragraph-specific test case.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@123992 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h
index 9dee1b5..ffa459e 100644
--- a/include/clang/AST/DeclCXX.h
+++ b/include/clang/AST/DeclCXX.h
@@ -1473,6 +1473,29 @@
return isCopyConstructor(TypeQuals);
}
+ /// \brief Determine whether this constructor is a move constructor
+ /// (C++0x [class.copy]p3), which can be used to move values of the class.
+ ///
+ /// \param TypeQuals If this constructor is a move constructor, will be set
+ /// to the type qualifiers on the referent of the first parameter's type.
+ bool isMoveConstructor(unsigned &TypeQuals) const;
+
+ /// \brief Determine whether this constructor is a move constructor
+ /// (C++0x [class.copy]p3), which can be used to move values of the class.
+ bool isMoveConstructor() const;
+
+ /// \brief Determine whether this is a copy or move constructor.
+ ///
+ /// \param TypeQuals Will be set to the type qualifiers on the reference
+ /// parameter, if in fact this is a copy or move constructor.
+ bool isCopyOrMoveConstructor(unsigned &TypeQuals) const;
+
+ /// \brief Determine whether this a copy or move constructor.
+ bool isCopyOrMoveConstructor() const {
+ unsigned Quals;
+ return isCopyOrMoveConstructor(Quals);
+ }
+
/// isConvertingConstructor - Whether this constructor is a
/// converting constructor (C++ [class.conv.ctor]), which can be
/// used for user-defined conversions.
diff --git a/lib/AST/DeclCXX.cpp b/lib/AST/DeclCXX.cpp
index 4548273..334ee79 100644
--- a/lib/AST/DeclCXX.cpp
+++ b/lib/AST/DeclCXX.cpp
@@ -1116,25 +1116,40 @@
bool
CXXConstructorDecl::isCopyConstructor(unsigned &TypeQuals) const {
+ return isCopyOrMoveConstructor(TypeQuals) &&
+ getParamDecl(0)->getType()->isLValueReferenceType();
+}
+
+bool CXXConstructorDecl::isMoveConstructor(unsigned &TypeQuals) const {
+ return isCopyOrMoveConstructor(TypeQuals) &&
+ getParamDecl(0)->getType()->isRValueReferenceType();
+}
+
+/// \brief Determine whether this is a copy or move constructor.
+bool CXXConstructorDecl::isCopyOrMoveConstructor(unsigned &TypeQuals) const {
// C++ [class.copy]p2:
// A non-template constructor for class X is a copy constructor
// if its first parameter is of type X&, const X&, volatile X& or
// const volatile X&, and either there are no other parameters
// or else all other parameters have default arguments (8.3.6).
+ // C++0x [class.copy]p3:
+ // A non-template constructor for class X is a move constructor if its
+ // first parameter is of type X&&, const X&&, volatile X&&, or
+ // const volatile X&&, and either there are no other parameters or else
+ // all other parameters have default arguments.
if ((getNumParams() < 1) ||
(getNumParams() > 1 && !getParamDecl(1)->hasDefaultArg()) ||
(getPrimaryTemplate() != 0) ||
(getDescribedFunctionTemplate() != 0))
return false;
-
+
const ParmVarDecl *Param = getParamDecl(0);
-
- // Do we have a reference type? Rvalue references don't count.
- const LValueReferenceType *ParamRefType =
- Param->getType()->getAs<LValueReferenceType>();
+
+ // Do we have a reference type?
+ const ReferenceType *ParamRefType = Param->getType()->getAs<ReferenceType>();
if (!ParamRefType)
return false;
-
+
// Is it a reference to our class type?
ASTContext &Context = getASTContext();
@@ -1144,12 +1159,12 @@
= Context.getCanonicalType(Context.getTagDeclType(getParent()));
if (PointeeType.getUnqualifiedType() != ClassTy)
return false;
-
+
// FIXME: other qualifiers?
-
- // We have a copy constructor.
+
+ // We have a copy or move constructor.
TypeQuals = PointeeType.getCVRQualifiers();
- return true;
+ return true;
}
bool CXXConstructorDecl::isConvertingConstructor(bool AllowExplicit) const {
diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp
index 9e8a157..00a13b5 100644
--- a/lib/Sema/SemaInit.cpp
+++ b/lib/Sema/SemaInit.cpp
@@ -3363,20 +3363,20 @@
if (S.RequireCompleteType(Loc, T, S.PDiag(diag::err_temp_copy_incomplete)))
return move(CurInit);
- // Perform overload resolution using the class's copy constructors.
+ // Perform overload resolution using the class's copy/move constructors.
DeclContext::lookup_iterator Con, ConEnd;
OverloadCandidateSet CandidateSet(Loc);
for (llvm::tie(Con, ConEnd) = S.LookupConstructors(Class);
Con != ConEnd; ++Con) {
- // Only consider copy constructors and constructor templates. Per
+ // Only consider copy/move constructors and constructor templates. Per
// C++0x [dcl.init]p16, second bullet to class types, this
// initialization is direct-initialization.
CXXConstructorDecl *Constructor = 0;
if ((Constructor = dyn_cast<CXXConstructorDecl>(*Con))) {
- // Handle copy constructors, only.
+ // Handle copy/moveconstructors, only.
if (!Constructor || Constructor->isInvalidDecl() ||
- !Constructor->isCopyConstructor() ||
+ !Constructor->isCopyOrMoveConstructor() ||
!Constructor->isConvertingConstructor(/*AllowExplicit=*/true))
continue;
diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp
index 7010d48..015dcbb 100644
--- a/lib/Sema/SemaStmt.cpp
+++ b/lib/Sema/SemaStmt.cpp
@@ -1137,6 +1137,86 @@
return 0;
}
+/// \brief Perform the initialization of a return value.
+///
+/// This routine implements C++0x [class.copy]p33, which attempts to treat
+/// returned lvalues as rvalues in certain cases (to prefer move construction),
+/// then falls back to treating them as lvalues if that failed.
+static ExprResult initializeReturnValue(Sema &S,
+ const VarDecl *NRVOCandidate,
+ SourceLocation ReturnLoc,
+ QualType ResultType,
+ Expr *RetValExp) {
+ // C++0x [class.copy]p33:
+ // When the criteria for elision of a copy operation are met or would
+ // be met save for the fact that the source object is a function
+ // parameter, and the object to be copied is designated by an lvalue,
+ // overload resolution to select the constructor for the copy is first
+ // performed as if the object were designated by an rvalue.
+ InitializedEntity Entity = InitializedEntity::InitializeResult(ReturnLoc,
+ ResultType,
+ NRVOCandidate != 0);
+
+ ExprResult Res = ExprError();
+ if (NRVOCandidate || S.getCopyElisionCandidate(ResultType, RetValExp, true)) {
+ ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack,
+ RetValExp->getType(), CK_LValueToRValue,
+ RetValExp, VK_XValue);
+
+ Expr *InitExpr = &AsRvalue;
+ InitializationKind Kind
+ = InitializationKind::CreateCopy(RetValExp->getLocStart(),
+ RetValExp->getLocStart());
+ InitializationSequence Seq(S, Entity, Kind, &InitExpr, 1);
+
+ // [...] If overload resolution fails, or if the type of the first
+ // parameter of the selected constructor is not an rvalue reference
+ // to the object’s type (possibly cv-qualified), overload resolution
+ // is performed again, considering the object as an lvalue.
+ if (Seq.getKind() != InitializationSequence::FailedSequence) {
+ for (InitializationSequence::step_iterator Step = Seq.step_begin(),
+ StepEnd = Seq.step_end();
+ Step != StepEnd; ++Step) {
+ if (Step->Kind
+ != InitializationSequence::SK_ConstructorInitialization)
+ continue;
+
+ CXXConstructorDecl *Constructor
+ = cast<CXXConstructorDecl>(Step->Function.Function);
+
+ const RValueReferenceType *RRefType
+ = Constructor->getParamDecl(0)->getType()
+ ->getAs<RValueReferenceType>();
+
+ // If we don't meet the criteria, break out now.
+ if (!RRefType ||
+ !S.Context.hasSameUnqualifiedType(RRefType->getPointeeType(),
+ ResultType))
+ break;
+
+ // Promote "AsRvalue" to the heap, since we now need this
+ // expression node to persist.
+ RetValExp = ImplicitCastExpr::Create(S.Context,
+ RetValExp->getType(),
+ CK_LValueToRValue,
+ RetValExp, 0, VK_XValue);
+
+ // Complete type-checking the initialization of the return type
+ // using the constructor we found.
+ Res = Seq.Perform(S, Entity, Kind, MultiExprArg(&RetValExp, 1));
+ }
+ }
+ }
+
+ // Either we didn't meet the criteria for treating an lvalue as an rvalue,
+ // above, or overload resolution failed. Either way, we need to try
+ // (again) now with the return value expression as written.
+ if (Res.isInvalid())
+ Res = S.PerformCopyInitialization(Entity, SourceLocation(), RetValExp);
+
+ return Res;
+}
+
/// ActOnBlockReturnStmt - Utility routine to figure out block's return type.
///
StmtResult
@@ -1193,12 +1273,8 @@
// In C++ the return statement is handled via a copy initialization.
// the C version of which boils down to CheckSingleAssignmentConstraints.
NRVOCandidate = getCopyElisionCandidate(FnRetType, RetValExp, false);
- ExprResult Res = PerformCopyInitialization(
- InitializedEntity::InitializeResult(ReturnLoc,
- FnRetType,
- NRVOCandidate != 0),
- SourceLocation(),
- Owned(RetValExp));
+ ExprResult Res = initializeReturnValue(*this, NRVOCandidate, ReturnLoc,
+ FnRetType, RetValExp);
if (Res.isInvalid()) {
// FIXME: Cleanup temporaries here, anyway?
return StmtError();
@@ -1291,12 +1367,8 @@
// In C++ the return statement is handled via a copy initialization.
// the C version of which boils down to CheckSingleAssignmentConstraints.
NRVOCandidate = getCopyElisionCandidate(FnRetType, RetValExp, false);
- ExprResult Res = PerformCopyInitialization(
- InitializedEntity::InitializeResult(ReturnLoc,
- FnRetType,
- NRVOCandidate != 0),
- SourceLocation(),
- Owned(RetValExp));
+ ExprResult Res = initializeReturnValue(*this, NRVOCandidate, ReturnLoc,
+ FnRetType, RetValExp);
if (Res.isInvalid()) {
// FIXME: Cleanup temporaries here, anyway?
return StmtError();
diff --git a/test/CXX/special/class.copy/p33-0x.cpp b/test/CXX/special/class.copy/p33-0x.cpp
new file mode 100644
index 0000000..a9ce589
--- /dev/null
+++ b/test/CXX/special/class.copy/p33-0x.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++0x -fsyntax-only -verify %s
+class X {
+ X(const X&);
+
+public:
+ X();
+ X(X&&);
+};
+
+X return_by_move(int i, X x) {
+ X x2;
+ if (i == 0)
+ return x;
+ else if (i == 1)
+ return x2;
+ else
+ return x;
+}
+
diff --git a/test/SemaCXX/rval-references-examples.cpp b/test/SemaCXX/rval-references-examples.cpp
index 1cde585..778924d 100644
--- a/test/SemaCXX/rval-references-examples.cpp
+++ b/test/SemaCXX/rval-references-examples.cpp
@@ -59,7 +59,7 @@
template<typename T> void accept_unique_ptr(unique_ptr<T>); // expected-note{{passing argument to parameter here}}
-void test_unique_ptr() {
+unique_ptr<int> test_unique_ptr() {
// Simple construction
unique_ptr<int> p;
unique_ptr<int> p1(new int);
@@ -85,4 +85,6 @@
// Implicit copies (failures);
accept_unique_ptr(p); // expected-error{{call to deleted constructor of 'unique_ptr<int>'}}
+
+ return p;
}
diff --git a/test/SemaCXX/rval-references-xfail.cpp b/test/SemaCXX/rval-references-xfail.cpp
deleted file mode 100644
index d41f8f1..0000000
--- a/test/SemaCXX/rval-references-xfail.cpp
+++ /dev/null
@@ -1,14 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++0x %s
-// XFAIL: *
-struct MoveOnly {
- MoveOnly();
- MoveOnly(const MoveOnly&) = delete; // expected-note {{candidate function}} \
- // expected-note 3{{explicitly marked deleted here}}
- MoveOnly(MoveOnly&&); // expected-note {{candidate function}}
- MoveOnly(int&&); // expected-note {{candidate function}}
-};
-
-MoveOnly returning() {
- MoveOnly mo;
- return mo;
-}