| Message ID | 20230316030157.882778-1-jason@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] c++: co_await and move-only type [PR105406] | expand |
Hi Jason > On 16 Mar 2023, at 03:01, Jason Merrill <jason@redhat.com> wrote: > > Tested x86_64-pc-linux-gnu. As with the array issue, I know you have WIP to > deal with larger issues, but this seems like a reasonable local fix. Does it > make sense to you? Yes, thanks for fixing it, I believe it will still be needed when pending WIP in this area is considered, since that only reduces the number of temporaries we might materialize. Iain > > -- 8< -- > > Here we were building a temporary MoveOnlyAwaitable to hold the result of > evaluating 'o', but since 'o' is an lvalue we should build a reference > instead. > > PR c++/105406 > > gcc/cp/ChangeLog: > > * coroutines.cc (build_co_await): Handle lvalue 'o'. > > gcc/testsuite/ChangeLog: > > * g++.dg/coroutines/co-await-moveonly1.C: New test. > --- > gcc/cp/coroutines.cc | 9 ++- > .../g++.dg/coroutines/co-await-moveonly1.C | 63 +++++++++++++++++++ > 2 files changed, 71 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/coroutines/co-await-moveonly1.C > > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc > index 7f8cbc3d95e..a2189e43db8 100644 > --- a/gcc/cp/coroutines.cc > +++ b/gcc/cp/coroutines.cc > @@ -1024,9 +1024,13 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind) > } > else > { > - e_proxy = get_awaitable_var (suspend_kind, o_type); > + tree p_type = o_type; > + if (glvalue_p (o)) > + p_type = cp_build_reference_type (p_type, !lvalue_p (o)); > + e_proxy = get_awaitable_var (suspend_kind, p_type); > o = cp_build_modify_expr (loc, e_proxy, INIT_EXPR, o, > tf_warning_or_error); > + e_proxy = convert_from_reference (e_proxy); > } > > /* I suppose we could check that this is contextually convertible to bool. */ > @@ -1120,6 +1124,9 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind) > } > TREE_VEC_ELT (awaiter_calls, 2) = awrs_call; /* await_resume(). */ > > + if (REFERENCE_REF_P (e_proxy)) > + e_proxy = TREE_OPERAND (e_proxy, 0); > + > tree await_expr = build5_loc (loc, CO_AWAIT_EXPR, > TREE_TYPE (TREE_TYPE (awrs_func)), > a, e_proxy, o, awaiter_calls, > diff --git a/gcc/testsuite/g++.dg/coroutines/co-await-moveonly1.C b/gcc/testsuite/g++.dg/coroutines/co-await-moveonly1.C > new file mode 100644 > index 00000000000..e2831c104bf > --- /dev/null > +++ b/gcc/testsuite/g++.dg/coroutines/co-await-moveonly1.C > @@ -0,0 +1,63 @@ > +// PR c++/105406 > +// { dg-do compile { target c++20 } } > + > +#include <coroutine> > +#include <exception> > + > +// A move-only awaitable > +class MoveOnlyAwaitable { > +public: > + MoveOnlyAwaitable() = default; > + MoveOnlyAwaitable(MoveOnlyAwaitable &&) = default; > + MoveOnlyAwaitable &operator=(MoveOnlyAwaitable &&) = default; > + > + MoveOnlyAwaitable(const MoveOnlyAwaitable &) = delete; > + MoveOnlyAwaitable &operator=(const MoveOnlyAwaitable &) = delete; > + > + bool await_ready() const noexcept { return false; } > + void await_suspend(std::coroutine_handle<>) noexcept {} > + void await_resume() {} > +}; > + > +struct task { > + struct promise_type { > + auto initial_suspend() const { return std::suspend_never{}; } > + auto final_suspend() const noexcept { return std::suspend_never(); } > + auto get_return_object() { return task{}; } > + void return_void() {} > + void unhandled_exception() {} > + > + template<typename T> > + T &&await_transform(T &&t) { > + return static_cast<T &&>(t); > + } > + > + > + }; > + > + bool await_ready() const { return false; } > + void await_suspend(std::coroutine_handle<> awaiter) {} > + void await_resume() {} > +}; > + > +task myCoroutine() { > + // GCC: OK > + // clang: OK > + { > + co_await MoveOnlyAwaitable(); > + } > + // GCC: OK > + // clang: OK > + { > + auto moveonly = MoveOnlyAwaitable(); > + co_await std::move(moveonly); > + } > + > + // GCC <= 11.2: OK > + // GCC 11.3:ERROR: error: use of deleted function 'MoveOnlyAwaitable::MoveOnlyAwaitable(const MoveOnlyAwaitable&) > + // clang: OK > + { > + auto moveonly = MoveOnlyAwaitable(); > + co_await moveonly; > + } > +} > > base-commit: ea4dd8f512979db247c54d6b41377bb73699bcd7 > -- > 2.31.1 >
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 7f8cbc3d95e..a2189e43db8 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -1024,9 +1024,13 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind) } else { - e_proxy = get_awaitable_var (suspend_kind, o_type); + tree p_type = o_type; + if (glvalue_p (o)) + p_type = cp_build_reference_type (p_type, !lvalue_p (o)); + e_proxy = get_awaitable_var (suspend_kind, p_type); o = cp_build_modify_expr (loc, e_proxy, INIT_EXPR, o, tf_warning_or_error); + e_proxy = convert_from_reference (e_proxy); } /* I suppose we could check that this is contextually convertible to bool. */ @@ -1120,6 +1124,9 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind) } TREE_VEC_ELT (awaiter_calls, 2) = awrs_call; /* await_resume(). */ + if (REFERENCE_REF_P (e_proxy)) + e_proxy = TREE_OPERAND (e_proxy, 0); + tree await_expr = build5_loc (loc, CO_AWAIT_EXPR, TREE_TYPE (TREE_TYPE (awrs_func)), a, e_proxy, o, awaiter_calls, diff --git a/gcc/testsuite/g++.dg/coroutines/co-await-moveonly1.C b/gcc/testsuite/g++.dg/coroutines/co-await-moveonly1.C new file mode 100644 index 00000000000..e2831c104bf --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/co-await-moveonly1.C @@ -0,0 +1,63 @@ +// PR c++/105406 +// { dg-do compile { target c++20 } } + +#include <coroutine> +#include <exception> + +// A move-only awaitable +class MoveOnlyAwaitable { +public: + MoveOnlyAwaitable() = default; + MoveOnlyAwaitable(MoveOnlyAwaitable &&) = default; + MoveOnlyAwaitable &operator=(MoveOnlyAwaitable &&) = default; + + MoveOnlyAwaitable(const MoveOnlyAwaitable &) = delete; + MoveOnlyAwaitable &operator=(const MoveOnlyAwaitable &) = delete; + + bool await_ready() const noexcept { return false; } + void await_suspend(std::coroutine_handle<>) noexcept {} + void await_resume() {} +}; + +struct task { + struct promise_type { + auto initial_suspend() const { return std::suspend_never{}; } + auto final_suspend() const noexcept { return std::suspend_never(); } + auto get_return_object() { return task{}; } + void return_void() {} + void unhandled_exception() {} + + template<typename T> + T &&await_transform(T &&t) { + return static_cast<T &&>(t); + } + + + }; + + bool await_ready() const { return false; } + void await_suspend(std::coroutine_handle<> awaiter) {} + void await_resume() {} +}; + +task myCoroutine() { + // GCC: OK + // clang: OK + { + co_await MoveOnlyAwaitable(); + } + // GCC: OK + // clang: OK + { + auto moveonly = MoveOnlyAwaitable(); + co_await std::move(moveonly); + } + + // GCC <= 11.2: OK + // GCC 11.3:ERROR: error: use of deleted function 'MoveOnlyAwaitable::MoveOnlyAwaitable(const MoveOnlyAwaitable&) + // clang: OK + { + auto moveonly = MoveOnlyAwaitable(); + co_await moveonly; + } +}