[PATCH RFC] c++: co_await and move-only type [PR105406]

[PATCH RFC] c++: co_await and move-only type [PR105406]

[Jason Merrill]

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?

-- 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

Re: [PATCH RFC] c++: co_await and move-only type [PR105406]

[Iain Sandoe]

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
>