From: Erik Pilkington Date: Thu, 30 Jun 2016 23:09:13 +0000 (+0000) Subject: [Sema] Implement C++14's DR1579: Prefer returning by converting move constructor X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1b983cb0af73b792c5cdc14bed5394082709a8fe;p=clang [Sema] Implement C++14's DR1579: Prefer returning by converting move constructor Fixes PR28096. Differential Revision: http://reviews.llvm.org/D21619 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@274291 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Sema/Initialization.h b/include/clang/Sema/Initialization.h index 6ae20f1291..a0bf78bf16 100644 --- a/include/clang/Sema/Initialization.h +++ b/include/clang/Sema/Initialization.h @@ -952,6 +952,9 @@ public: step_iterator step_begin() const { return Steps.begin(); } step_iterator step_end() const { return Steps.end(); } + typedef llvm::iterator_range step_range; + step_range steps() const { return {step_begin(), step_end()}; } + /// \brief Determine whether this initialization is a direct reference /// binding (C++ [dcl.init.ref]). bool isDirectReferenceBinding() const; diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 8d469a0313..eab1188f0b 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -3476,9 +3476,9 @@ public: SourceLocation Loc, unsigned NumParams); VarDecl *getCopyElisionCandidate(QualType ReturnType, Expr *E, - bool AllowFunctionParameters); + bool AllowParamOrMoveConstructible); bool isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD, - bool AllowFunctionParameters); + bool AllowParamOrMoveConstructible); StmtResult ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp, Scope *CurScope); diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index 61ef8f3a81..ed80b08a49 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/lib/Sema/SemaStmt.cpp @@ -2697,16 +2697,16 @@ Sema::ActOnBreakStmt(SourceLocation BreakLoc, Scope *CurScope) { /// \param E The expression being returned from the function or block, or /// being thrown. /// -/// \param AllowFunctionParameter Whether we allow function parameters to -/// be considered NRVO candidates. C++ prohibits this for NRVO itself, but -/// we re-use this logic to determine whether we should try to move as part of -/// a return or throw (which does allow function parameters). +/// \param AllowParamOrMoveConstructible Whether we allow function parameters or +/// id-expressions that could be moved out of the function to be considered NRVO +/// candidates. C++ prohibits these for NRVO itself, but we re-use this logic to +/// determine whether we should try to move as part of a return or throw (which +/// does allow function parameters). /// /// \returns The NRVO candidate variable, if the return statement may use the /// NRVO, or NULL if there is no such candidate. -VarDecl *Sema::getCopyElisionCandidate(QualType ReturnType, - Expr *E, - bool AllowFunctionParameter) { +VarDecl *Sema::getCopyElisionCandidate(QualType ReturnType, Expr *E, + bool AllowParamOrMoveConstructible) { if (!getLangOpts().CPlusPlus) return nullptr; @@ -2719,13 +2719,13 @@ VarDecl *Sema::getCopyElisionCandidate(QualType ReturnType, if (!VD) return nullptr; - if (isCopyElisionCandidate(ReturnType, VD, AllowFunctionParameter)) + if (isCopyElisionCandidate(ReturnType, VD, AllowParamOrMoveConstructible)) return VD; return nullptr; } bool Sema::isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD, - bool AllowFunctionParameter) { + bool AllowParamOrMoveConstructible) { QualType VDType = VD->getType(); // - in a return statement in a function with ... // ... a class return type ... @@ -2733,20 +2733,24 @@ bool Sema::isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD, if (!ReturnType->isRecordType()) return false; // ... the same cv-unqualified type as the function return type ... - if (!VDType->isDependentType() && + // When considering moving this expression out, allow dissimilar types. + if (!AllowParamOrMoveConstructible && !VDType->isDependentType() && !Context.hasSameUnqualifiedType(ReturnType, VDType)) return false; } // ...object (other than a function or catch-clause parameter)... if (VD->getKind() != Decl::Var && - !(AllowFunctionParameter && VD->getKind() == Decl::ParmVar)) + !(AllowParamOrMoveConstructible && VD->getKind() == Decl::ParmVar)) return false; if (VD->isExceptionVariable()) return false; // ...automatic... if (!VD->hasLocalStorage()) return false; + if (AllowParamOrMoveConstructible) + return true; + // ...non-volatile... if (VD->getType().isVolatileQualified()) return false; @@ -2765,7 +2769,7 @@ bool Sema::isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD, /// \brief Perform the initialization of a potentially-movable value, which /// is the result of return value. /// -/// This routine implements C++0x [class.copy]p33, which attempts to treat +/// This routine implements C++14 [class.copy]p32, 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. ExprResult @@ -2774,52 +2778,59 @@ Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity, QualType ResultType, Expr *Value, bool AllowNRVO) { - // 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. + // C++14 [class.copy]p32: + // When the criteria for elision of a copy/move operation are met, but not for + // an exception-declaration, and the object to be copied is designated by an + // lvalue, or when the expression in a return statement is a (possibly + // parenthesized) id-expression that names an object with automatic storage + // duration declared in the body or parameter-declaration-clause of the + // innermost enclosing function or lambda-expression, overload resolution to + // select the constructor for the copy is first performed as if the object + // were designated by an rvalue. ExprResult Res = ExprError(); - if (AllowNRVO && - (NRVOCandidate || getCopyElisionCandidate(ResultType, Value, true))) { - ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, - Value->getType(), CK_NoOp, Value, VK_XValue); + + if (AllowNRVO && !NRVOCandidate) + NRVOCandidate = getCopyElisionCandidate(ResultType, Value, true); + + if (AllowNRVO && NRVOCandidate) { + ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(), + CK_NoOp, Value, VK_XValue); Expr *InitExpr = &AsRvalue; - InitializationKind Kind - = InitializationKind::CreateCopy(Value->getLocStart(), - Value->getLocStart()); - InitializationSequence Seq(*this, Entity, Kind, InitExpr); - // [...] 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. + InitializationKind Kind = InitializationKind::CreateCopy( + Value->getLocStart(), Value->getLocStart()); + + InitializationSequence Seq(*this, Entity, Kind, InitExpr); if (Seq) { - for (InitializationSequence::step_iterator Step = Seq.step_begin(), - StepEnd = Seq.step_end(); - Step != StepEnd; ++Step) { - if (Step->Kind != InitializationSequence::SK_ConstructorInitialization) + for (const InitializationSequence::Step &Step : Seq.steps()) { + if (!(Step.Kind == + InitializationSequence::SK_ConstructorInitialization || + (Step.Kind == InitializationSequence::SK_UserConversion && + isa(Step.Function.Function)))) continue; - CXXConstructorDecl *Constructor - = cast(Step->Function.Function); + CXXConstructorDecl *Constructor = + cast(Step.Function.Function); const RValueReferenceType *RRefType = Constructor->getParamDecl(0)->getType() ->getAs(); - // If we don't meet the criteria, break out now. + // [...] If the first overload resolution fails or was not performed, 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 (!RRefType || !Context.hasSameUnqualifiedType(RRefType->getPointeeType(), - Context.getTypeDeclType(Constructor->getParent()))) + NRVOCandidate->getType())) break; // Promote "AsRvalue" to the heap, since we now need this // expression node to persist. - Value = ImplicitCastExpr::Create(Context, Value->getType(), - CK_NoOp, Value, nullptr, VK_XValue); + Value = ImplicitCastExpr::Create(Context, Value->getType(), CK_NoOp, + Value, nullptr, VK_XValue); // Complete type-checking the initialization of the return type // using the constructor we found. diff --git a/test/CXX/drs/dr15xx.cpp b/test/CXX/drs/dr15xx.cpp index 439823410a..5f85a196fd 100644 --- a/test/CXX/drs/dr15xx.cpp +++ b/test/CXX/drs/dr15xx.cpp @@ -87,6 +87,62 @@ namespace std { } // std +namespace dr1579 { // dr1579: 3.9 +template +struct GenericMoveOnly { + GenericMoveOnly(); + template GenericMoveOnly(const GenericMoveOnly &) = delete; // expected-note 5 {{marked deleted here}} + GenericMoveOnly(const int &) = delete; // expected-note 2 {{marked deleted here}} + template GenericMoveOnly(GenericMoveOnly &&); + GenericMoveOnly(int &&); +}; + +GenericMoveOnly DR1579_Eligible(GenericMoveOnly CharMO) { + int i; + GenericMoveOnly GMO; + + if (0) + return i; + else if (0) + return GMO; + else if (0) + return ((GMO)); + else + return CharMO; +} + +GenericMoveOnly GlobalMO; + +GenericMoveOnly DR1579_Ineligible(int &AnInt, + GenericMoveOnly &CharMO) { + static GenericMoveOnly StaticMove; + extern GenericMoveOnly ExternMove; + + if (0) + return AnInt; // expected-error{{invokes a deleted function}} + else if (0) + return GlobalMO; // expected-error{{invokes a deleted function}} + else if (0) + return StaticMove; // expected-error{{invokes a deleted function}} + else if (0) + return ExternMove; // expected-error{{invokes a deleted function}} + else if (0) + return AnInt; // expected-error{{invokes a deleted function}} + else + return CharMO; // expected-error{{invokes a deleted function}} +} + +auto DR1579_lambda_valid = [](GenericMoveOnly mo) -> + GenericMoveOnly { + return mo; +}; + +auto DR1579_lambda_invalid = []() -> GenericMoveOnly { + static GenericMoveOnly mo; + return mo; // expected-error{{invokes a deleted function}} +}; +} // end namespace dr1579 + namespace dr1589 { // dr1589: 3.7 c++11 // Ambiguous ranking of list-initialization sequences diff --git a/test/SemaCXX/rval-references.cpp b/test/SemaCXX/rval-references.cpp index 9c79ad7b0b..4c2050494b 100644 --- a/test/SemaCXX/rval-references.cpp +++ b/test/SemaCXX/rval-references.cpp @@ -72,23 +72,17 @@ int&& should_not_warn(int&& i) { // But GCC 4.4 does // Test the return dance. This also tests IsReturnCopyElidable. struct MoveOnly { MoveOnly(); - MoveOnly(const MoveOnly&) = delete; // expected-note {{candidate constructor}} \ - // expected-note 3{{explicitly marked deleted here}} - MoveOnly(MoveOnly&&); // expected-note {{candidate constructor}} - MoveOnly(int&&); // expected-note {{candidate constructor}} + MoveOnly(const MoveOnly&) = delete; // expected-note 3{{explicitly marked deleted here}} }; MoveOnly gmo; MoveOnly returningNonEligible() { - int i; static MoveOnly mo; MoveOnly &r = mo; if (0) // Copy from global can't be elided return gmo; // expected-error {{call to deleted constructor}} else if (0) // Copy from local static can't be elided return mo; // expected-error {{call to deleted constructor}} - else if (0) // Copy from reference can't be elided + else // Copy from reference can't be elided return r; // expected-error {{call to deleted constructor}} - else // Construction from different type can't be elided - return i; // expected-error {{no viable conversion from returned value of type 'int' to function return type 'MoveOnly'}} }