diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp --- a/clang/lib/Sema/SemaCoroutine.cpp +++ b/clang/lib/Sema/SemaCoroutine.cpp @@ -954,6 +954,82 @@ return BuildCoreturnStmt(Loc, E); } +// [class.copy.elision]p3: "If the expression in a [...] co_­return statement +// is a (possibly parenthesized) id-expression that names an implicitly +// movable entity declared in the body or parameter-declaration-clause of +// the innermost enclosing function or lambda-expression, [...] overload +// resolution to select [...] the return_­value overload to call is first +// performed as if the expression or operand were an rvalue. If the first +// overload resolution fails or was not performed, overload resolution is +// performed again, considering the expression or operand as an lvalue." +// +// Our implementation deviates from the standard in two ways: +// 1) We only consider cases where return_value is a function or function +// template, so we never try implicitly moving in the case of a function +// pointer or object of class type with operator() overload. +// 2) We don't fall back if overload resolution failed due to ambiguity, only +// if no candidate works. One could say that we do overload resolution on +// both (const) T& and T&& while treating candidates for T&& as better fit: +// so no candidate for T&& falls back to T&, but multiple equivalent +// candidates are an error. +static bool shouldMoveReturnExpr(Sema &S, RecordDecl *PromiseTypeDecl, + Expr *E) { + // Check if we're allowed to implicitly move. + VarDecl *ImplicitMoveCandidate = + S.getCopyElisionCandidate(E->getType(), E, Sema::CES_Default); + if (!ImplicitMoveCandidate) + return false; + if (ImplicitMoveCandidate->getType()->isLValueReferenceType()) + return false; + + ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, E->getType(), CK_NoOp, E, + VK_XValue, FPOptionsOverride()); + + // Lookup return_value methods in the promise type, and check if any accepts + // the moved expression. If it does, we do the move. + DeclarationNameInfo NameInfo(&S.PP.getIdentifierTable().get("return_value"), + SourceLocation{}); + LookupResult LR(S, NameInfo, Sema::LookupMemberName); + CXXScopeSpec SS; + S.LookupQualifiedName(LR, PromiseTypeDecl, SS); + for (NamedDecl *D : LR) { + if (const auto *USD = dyn_cast(D)) + D = USD->getTargetDecl(); + if (const auto *CMD = dyn_cast(D)) { + if (CMD->isDeleted()) + continue; + if (CMD->getNumParams() < 1) + continue; + for (unsigned param = 1; param < CMD->getNumParams(); ++param) + if (!CMD->getParamDecl(param)->hasDefaultArg()) + continue; + InitializedEntity Entity = InitializedEntity::InitializeParameter( + S.Context, CMD->getParamDecl(0)); + if (S.CanPerformCopyInitialization(Entity, &AsRvalue)) + return true; + } else if (auto *FTD = dyn_cast(D)) { + TemplateDeductionInfo Info(SourceLocation{}); + FunctionDecl *Specialization = nullptr; + Sema::TemplateDeductionResult Result = S.DeduceTemplateArguments( + FTD, {}, {&AsRvalue}, Specialization, Info, false, + [&](ArrayRef ParamTypes) { + QualType ParamType = ParamTypes[0]; + if (!ParamType->isDependentType()) { + InitializedEntity Entity = InitializedEntity::InitializeParameter( + S.Context, ParamType, false); + if (!S.CanPerformCopyInitialization(Entity, &AsRvalue)) + return true; + } + return false; + }); + if (Result == Sema::TDK_Success) + return true; + } + // TODO: handle VarDecls. They can be function pointers or function objects. + } + return false; +} + StmtResult Sema::BuildCoreturnStmt(SourceLocation Loc, Expr *E, bool IsImplicit) { auto *FSI = checkCoroutineContext(*this, Loc, "co_return", IsImplicit); @@ -967,26 +1043,15 @@ E = R.get(); } - // Move the return value if we can - if (E) { - auto NRVOCandidate = this->getCopyElisionCandidate(E->getType(), E, CES_AsIfByStdMove); - if (NRVOCandidate) { - InitializedEntity Entity = - InitializedEntity::InitializeResult(Loc, E->getType(), NRVOCandidate); - ExprResult MoveResult = this->PerformMoveOrCopyInitialization( - Entity, NRVOCandidate, E->getType(), E); - if (MoveResult.get()) - E = MoveResult.get(); - } - } - - // FIXME: If the operand is a reference to a variable that's about to go out - // of scope, we should treat the operand as an xvalue for this overload - // resolution. VarDecl *Promise = FSI->CoroutinePromise; ExprResult PC; if (E && (isa(E) || !E->getType()->isVoidType())) { - PC = buildPromiseCall(*this, Promise, Loc, "return_value", E); + Expr *Arg = E; + if (const RecordType *PromiseT = Promise->getType()->getAs()) + if (shouldMoveReturnExpr(*this, PromiseT->getDecl(), E)) + Arg = ImplicitCastExpr::Create(Context, E->getType(), CK_NoOp, E, + nullptr, VK_XValue, FPOptionsOverride()); + PC = buildPromiseCall(*this, Promise, Loc, "return_value", Arg); } else { E = MakeFullDiscardedValueExpr(E).get(); PC = buildPromiseCall(*this, Promise, Loc, "return_void", None); diff --git a/clang/test/SemaCXX/coroutine-rvo.cpp b/clang/test/SemaCXX/coroutine-rvo.cpp --- a/clang/test/SemaCXX/coroutine-rvo.cpp +++ b/clang/test/SemaCXX/coroutine-rvo.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -stdlib=libc++ -std=c++1z -fcoroutines-ts -fsyntax-only +// RUN: %clang_cc1 -verify -std=c++17 -fcoroutines-ts -fsyntax-only %s namespace std::experimental { template struct coroutine_handle { @@ -39,10 +39,14 @@ }; struct MoveOnly { - MoveOnly() {}; + MoveOnly() = default; MoveOnly(const MoveOnly&) = delete; - MoveOnly(MoveOnly&&) noexcept {}; - ~MoveOnly() {}; + MoveOnly(MoveOnly &&) = default; +}; + +struct NoCopyNoMove { + NoCopyNoMove() = default; + NoCopyNoMove(const NoCopyNoMove &) = delete; }; template @@ -52,18 +56,93 @@ auto final_suspend() noexcept { return suspend_never{}; } auto get_return_object() { return task{}; } static void unhandled_exception() {} - void return_value(T&& value) {} + void return_value(T &&value) {} // expected-note 2{{passing argument}} }; }; -task f() { - MoveOnly value; +task local2val() { + NoCopyNoMove value; + co_return value; +} + +task local2ref() { + NoCopyNoMove value; + co_return value; +} + +// We need the move constructor for construction of the coroutine. +task param2val(MoveOnly value) { + co_return value; +} + +task lvalue2val(NoCopyNoMove &value) { + co_return value; // expected-error{{rvalue reference to type 'NoCopyNoMove' cannot bind to lvalue of type 'NoCopyNoMove'}} +} + +task rvalue2val(NoCopyNoMove &&value) { + co_return value; +} + +task lvalue2ref(NoCopyNoMove &value) { co_return value; } -int main() { - f(); - return 0; +task rvalue2ref(NoCopyNoMove &&value) { + co_return value; +} + +struct To { + operator MoveOnly() &&; +}; +task conversion_operator() { + To t; + co_return t; +} + +struct Construct { + Construct(MoveOnly); +}; +task converting_constructor() { + MoveOnly w; + co_return w; +} + +struct Derived : MoveOnly {}; +task derived2base() { + Derived result; + co_return result; +} + +struct RetThis { + task foo() && { + co_return *this; // expected-error{{rvalue reference to type 'RetThis' cannot bind to lvalue of type 'RetThis'}} + } +}; + +template +struct is_same { static constexpr bool value = false; }; + +template +struct is_same { static constexpr bool value = true; }; + +template +struct template_return_task { + struct promise_type { + auto initial_suspend() { return suspend_never{}; } + auto final_suspend() noexcept { return suspend_never{}; } + auto get_return_object() { return template_return_task{}; } + static void unhandled_exception(); + template + void return_value(U &&value) { + static_assert(is_same::value); + } + }; +}; + +template_return_task param2template(MoveOnly value) { + co_return value; // We should deduce U = MoveOnly. } -// expected-no-diagnostics +template_return_task lvalue2template(NoCopyNoMove &value) { + co_return value; // We should deduce U = NoCopyNoMove&. +}