aaronpuchert created this revision.
aaronpuchert added reviewers: GorNishanov, modocache, Quuxplusone.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
aaronpuchert updated this revision to Diff 224513.
aaronpuchert added a comment.
Also remove FIXME comment.
The implementation of return value optimization in D51741
<https://reviews.llvm.org/D51741> turns
co_return value;
which is essentially
__promise.return_value(value);
into
__promise.return_value(decltype<value>(std::move(value)));
instead of
__promise.return_value(std::move(value));
Instead of doing a copy/move initialization, which is only required for
regular return statements, we just cast to an xvalue reference when we
can consume an object.
Also simplifies the test: some flags aren't needed, neither is main.
Instead we now check that no move constructor is emitted in certain
cases. (That is, we actually delete the move constructor.)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D68845
Files:
clang/lib/Sema/SemaCoroutine.cpp
clang/test/SemaCXX/coroutine-rvo.cpp
Index: clang/test/SemaCXX/coroutine-rvo.cpp
===================================================================
--- clang/test/SemaCXX/coroutine-rvo.cpp
+++ 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 <class Promise = void> struct coroutine_handle {
@@ -38,11 +38,17 @@
void await_resume() noexcept;
};
+struct Default {};
+
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 <typename T>
@@ -52,18 +58,31 @@
auto final_suspend() { 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<MoveOnly> f() {
- MoveOnly value;
+task<NoCopyNoMove> local2val() {
+ NoCopyNoMove value;
co_return value;
}
-int main() {
- f();
- return 0;
+task<MoveOnly> param2val(MoveOnly value) {
+ co_return value;
}
-// expected-no-diagnostics
+task<Default> lvalue2val(Default& value) {
+ co_return value; // expected-error{{rvalue reference to type 'Default'
cannot bind to lvalue of type 'Default'}}
+}
+
+task<NoCopyNoMove> rvalue2val(NoCopyNoMove&& value) {
+ co_return value;
+}
+
+task<NoCopyNoMove&> lvalue2ref(NoCopyNoMove& value) {
+ co_return value;
+}
+
+task<Default&> rvalue2ref(Default&& value) {
+ co_return value; // expected-error{{non-const lvalue reference to type
'Default' cannot bind to a temporary of type 'Default'}}
+}
Index: clang/lib/Sema/SemaCoroutine.cpp
===================================================================
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -866,20 +866,13 @@
// 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();
- }
+ VarDecl *NRVOCandidate =
+ getCopyElisionCandidate(E->getType(), E, CES_Default);
+ if (NRVOCandidate && !NRVOCandidate->getType()->isLValueReferenceType())
+ E = ImplicitCastExpr::Create(Context, E->getType(), CK_NoOp, E, {},
+ VK_XValue);
}
- // 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<InitListExpr>(E) || !E->getType()->isVoidType())) {
Index: clang/test/SemaCXX/coroutine-rvo.cpp
===================================================================
--- clang/test/SemaCXX/coroutine-rvo.cpp
+++ 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 <class Promise = void> struct coroutine_handle {
@@ -38,11 +38,17 @@
void await_resume() noexcept;
};
+struct Default {};
+
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 <typename T>
@@ -52,18 +58,31 @@
auto final_suspend() { 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<MoveOnly> f() {
- MoveOnly value;
+task<NoCopyNoMove> local2val() {
+ NoCopyNoMove value;
co_return value;
}
-int main() {
- f();
- return 0;
+task<MoveOnly> param2val(MoveOnly value) {
+ co_return value;
}
-// expected-no-diagnostics
+task<Default> lvalue2val(Default& value) {
+ co_return value; // expected-error{{rvalue reference to type 'Default' cannot bind to lvalue of type 'Default'}}
+}
+
+task<NoCopyNoMove> rvalue2val(NoCopyNoMove&& value) {
+ co_return value;
+}
+
+task<NoCopyNoMove&> lvalue2ref(NoCopyNoMove& value) {
+ co_return value;
+}
+
+task<Default&> rvalue2ref(Default&& value) {
+ co_return value; // expected-error{{non-const lvalue reference to type 'Default' cannot bind to a temporary of type 'Default'}}
+}
Index: clang/lib/Sema/SemaCoroutine.cpp
===================================================================
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -866,20 +866,13 @@
// 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();
- }
+ VarDecl *NRVOCandidate =
+ getCopyElisionCandidate(E->getType(), E, CES_Default);
+ if (NRVOCandidate && !NRVOCandidate->getType()->isLValueReferenceType())
+ E = ImplicitCastExpr::Create(Context, E->getType(), CK_NoOp, E, {},
+ VK_XValue);
}
- // 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<InitListExpr>(E) || !E->getType()->isVoidType())) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
