From: Iain Sandoe <iain@sandoe.co.uk>
To: Jason Merrill <jason@redhat.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH RFC] c++: co_await and move-only type [PR105406]
Date: Thu, 16 Mar 2023 10:23:41 +0000 [thread overview]
Message-ID: <AE242E14-0322-4D50-A727-3D52DCEED9C3@sandoe.co.uk> (raw)
In-Reply-To: <20230316030157.882778-1-jason@redhat.com>
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
>
prev parent reply other threads:[~2023-03-16 10:23 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-16 3:01 Jason Merrill
2023-03-16 10:23 ` Iain Sandoe [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=AE242E14-0322-4D50-A727-3D52DCEED9C3@sandoe.co.uk \
--to=iain@sandoe.co.uk \
--cc=gcc-patches@gcc.gnu.org \
--cc=jason@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).