coroutines: Wrap co_await in a target expr where needed [PR95050]

Message ID 7C472946-5A11-45C3-871A-68616690A4D1@sandoe.co.uk
State New
Headers show
Series
  • coroutines: Wrap co_await in a target expr where needed [PR95050]
Related show

Commit Message

Iain Sandoe June 1, 2020, 7:55 a.m.
Hi,

Since the co_await expression is mostly opaque to the existing
machinery, we were hiding the details of the await_resume return
value.  If that needs to be wrapped in a target expression, then
emulate this with the whole co_await.  Similarly, if the await
expression we build in response to co_await p.yield_value (e)
is wrapped in a target expression, then we need to transfer that
wrapper to the resultant CO_YIELD_EXPR (which is, itself, just
a proxy for the underlying co_await).

tested on x86_64,powerpc64-linux, x86_64-darwin
OK for master?
OK for 10.2?
thanks
Iain

gcc/cp/ChangeLog:

	PR c++/95050
	* coroutines.cc (build_co_await): Wrap the co_await expression
	in a TARGET_EXPR, where needed.
	(finish_co_yield_expr): Likewise.

gcc/testsuite/ChangeLog:

	PR c++/95050
	* g++.dg/coroutines/pr95050.C: New test.
---
 gcc/cp/coroutines.cc                      | 29 +++++++++++++-
 gcc/testsuite/g++.dg/coroutines/pr95050.C | 49 +++++++++++++++++++++++
 2 files changed, 76 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95050.C

-- 
2.24.1

Comments

Nathan Sidwell June 1, 2020, 2:51 p.m. | #1
On 6/1/20 3:55 AM, Iain Sandoe wrote:
> Hi,

> 

> Since the co_await expression is mostly opaque to the existing

> machinery, we were hiding the details of the await_resume return

> value.  If that needs to be wrapped in a target expression, then

> emulate this with the whole co_await.  Similarly, if the await

> expression we build in response to co_await p.yield_value (e)

> is wrapped in a target expression, then we need to transfer that

> wrapper to the resultant CO_YIELD_EXPR (which is, itself, just

> a proxy for the underlying co_await).

> 

> tested on x86_64,powerpc64-linux, x86_64-darwin

> OK for master?

> OK for 10.2?

> thanks

> Iain


ok for both
> 

> gcc/cp/ChangeLog:

> 

> 	PR c++/95050

> 	* coroutines.cc (build_co_await): Wrap the co_await expression

> 	in a TARGET_EXPR, where needed.

> 	(finish_co_yield_expr): Likewise.

> 

> gcc/testsuite/ChangeLog:

> 

> 	PR c++/95050

> 	* g++.dg/coroutines/pr95050.C: New test.

> ---

>   gcc/cp/coroutines.cc                      | 29 +++++++++++++-

>   gcc/testsuite/g++.dg/coroutines/pr95050.C | 49 +++++++++++++++++++++++

>   2 files changed, 76 insertions(+), 2 deletions(-)

>   create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95050.C

> 

> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc

> index 8746927577a..cc685ca73b2 100644

> --- a/gcc/cp/coroutines.cc

> +++ b/gcc/cp/coroutines.cc

> @@ -816,6 +816,12 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind)

>     tree awaiter_calls = make_tree_vec (3);

>     TREE_VEC_ELT (awaiter_calls, 0) = awrd_call; /* await_ready().  */

>     TREE_VEC_ELT (awaiter_calls, 1) = awsp_call; /* await_suspend().  */

> +  tree te = NULL_TREE;

> +  if (TREE_CODE (awrs_call) == TARGET_EXPR)

> +    {

> +      te = awrs_call;

> +      awrs_call = TREE_OPERAND (awrs_call, 1);

> +    }

>     TREE_VEC_ELT (awaiter_calls, 2) = awrs_call; /* await_resume().  */

>   

>     tree await_expr = build5_loc (loc, CO_AWAIT_EXPR,

> @@ -823,7 +829,13 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind)

>   				a, e_proxy, o, awaiter_calls,

>   				build_int_cst (integer_type_node,

>   					       (int) suspend_kind));

> -  return convert_from_reference (await_expr);

> +  if (te)

> +    {

> +      TREE_OPERAND (te, 1) = await_expr;

> +      await_expr = te;

> +    }

> +  tree t = convert_from_reference (await_expr);

> +  return t;

>   }

>   

>   tree

> @@ -960,8 +972,21 @@ finish_co_yield_expr (location_t kw, tree expr)

>     tree op = build_co_await (kw, yield_call, CO_YIELD_SUSPEND_POINT);

>     if (op != error_mark_node)

>       {

> -      op = build2_loc (kw, CO_YIELD_EXPR, TREE_TYPE (op), expr, op);

> +      if (REFERENCE_REF_P (op))

> +	op = TREE_OPERAND (op, 0);

> +      /* If the await expression is wrapped in a TARGET_EXPR, then transfer

> +	 that wrapper to the CO_YIELD_EXPR, since this is just a proxy for

> +	 its contained await.  Otherwise, just build the CO_YIELD_EXPR.  */

> +      if (TREE_CODE (op) == TARGET_EXPR)

> +	{

> +	  tree t = TREE_OPERAND (op, 1);

> +	  t = build2_loc (kw, CO_YIELD_EXPR, TREE_TYPE (t), expr, t);

> +	  TREE_OPERAND (op, 1) = t;

> +	}

> +      else

> +	op = build2_loc (kw, CO_YIELD_EXPR, TREE_TYPE (op), expr, op);

>         TREE_SIDE_EFFECTS (op) = 1;

> +      op = convert_from_reference (op);

>       }

>   

>     return op;

> diff --git a/gcc/testsuite/g++.dg/coroutines/pr95050.C b/gcc/testsuite/g++.dg/coroutines/pr95050.C

> new file mode 100644

> index 00000000000..fd1516d32f0

> --- /dev/null

> +++ b/gcc/testsuite/g++.dg/coroutines/pr95050.C

> @@ -0,0 +1,49 @@

> +#if __has_include (<coroutine>)

> +#include <coroutine>

> +using namespace std;

> +#elif defined (__clang__) && __has_include (<experimental/coroutine>)

> +#include <experimental/coroutine>

> +using namespace std::experimental;

> +#endif

> +#include <utility>

> +

> +struct ret_type

> +{

> +  ret_type () = default;

> +  ret_type (const ret_type&) = delete;

> +  //ret_type (ret_type&&) = default;

> +  ~ret_type() {}

> +};

> +

> +struct task

> +{

> +  struct promise_type

> +  {

> +    auto get_return_object () -> task  { return {}; }

> +    auto initial_suspend () -> suspend_always { return {}; }

> +    auto final_suspend () -> suspend_always { return {}; }

> +    void return_void () {}

> +    void unhandled_exception () { }

> +    void thing (ret_type x) {}

> +  };

> +};

> +

> +struct awaiter

> +{

> +  bool await_ready() const { return true; }

> +  void await_suspend (coroutine_handle<>) {}

> +  ret_type await_resume() { return {}; }

> +};

> +

> +task

> +my_coro ()

> +{

> +  ret_type r2{co_await awaiter{}};

> +  //ret_type r3 (std::move(r2));

> +}

> +

> +int main()

> +{

> + auto x = my_coro ();

> + return 0;

> +}

> 



-- 
Nathan Sidwell

Patch

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 8746927577a..cc685ca73b2 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -816,6 +816,12 @@  build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind)
   tree awaiter_calls = make_tree_vec (3);
   TREE_VEC_ELT (awaiter_calls, 0) = awrd_call; /* await_ready().  */
   TREE_VEC_ELT (awaiter_calls, 1) = awsp_call; /* await_suspend().  */
+  tree te = NULL_TREE;
+  if (TREE_CODE (awrs_call) == TARGET_EXPR)
+    {
+      te = awrs_call;
+      awrs_call = TREE_OPERAND (awrs_call, 1);
+    }
   TREE_VEC_ELT (awaiter_calls, 2) = awrs_call; /* await_resume().  */
 
   tree await_expr = build5_loc (loc, CO_AWAIT_EXPR,
@@ -823,7 +829,13 @@  build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind)
 				a, e_proxy, o, awaiter_calls,
 				build_int_cst (integer_type_node,
 					       (int) suspend_kind));
-  return convert_from_reference (await_expr);
+  if (te)
+    {
+      TREE_OPERAND (te, 1) = await_expr;
+      await_expr = te;
+    }
+  tree t = convert_from_reference (await_expr);
+  return t;
 }
 
 tree
@@ -960,8 +972,21 @@  finish_co_yield_expr (location_t kw, tree expr)
   tree op = build_co_await (kw, yield_call, CO_YIELD_SUSPEND_POINT);
   if (op != error_mark_node)
     {
-      op = build2_loc (kw, CO_YIELD_EXPR, TREE_TYPE (op), expr, op);
+      if (REFERENCE_REF_P (op))
+	op = TREE_OPERAND (op, 0);
+      /* If the await expression is wrapped in a TARGET_EXPR, then transfer
+	 that wrapper to the CO_YIELD_EXPR, since this is just a proxy for
+	 its contained await.  Otherwise, just build the CO_YIELD_EXPR.  */
+      if (TREE_CODE (op) == TARGET_EXPR)
+	{
+	  tree t = TREE_OPERAND (op, 1);
+	  t = build2_loc (kw, CO_YIELD_EXPR, TREE_TYPE (t), expr, t);
+	  TREE_OPERAND (op, 1) = t;
+	}
+      else
+	op = build2_loc (kw, CO_YIELD_EXPR, TREE_TYPE (op), expr, op);
       TREE_SIDE_EFFECTS (op) = 1;
+      op = convert_from_reference (op);
     }
 
   return op;
diff --git a/gcc/testsuite/g++.dg/coroutines/pr95050.C b/gcc/testsuite/g++.dg/coroutines/pr95050.C
new file mode 100644
index 00000000000..fd1516d32f0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr95050.C
@@ -0,0 +1,49 @@ 
+#if __has_include (<coroutine>)
+#include <coroutine>
+using namespace std;
+#elif defined (__clang__) && __has_include (<experimental/coroutine>)
+#include <experimental/coroutine>
+using namespace std::experimental;
+#endif
+#include <utility>
+
+struct ret_type 
+{
+  ret_type () = default;
+  ret_type (const ret_type&) = delete;
+  //ret_type (ret_type&&) = default;
+  ~ret_type() {}
+};
+
+struct task
+{
+  struct promise_type
+  {
+    auto get_return_object () -> task  { return {}; }
+    auto initial_suspend () -> suspend_always { return {}; }
+    auto final_suspend () -> suspend_always { return {}; }
+    void return_void () {} 
+    void unhandled_exception () { }
+    void thing (ret_type x) {} 
+  };
+};
+
+struct awaiter
+{
+  bool await_ready() const { return true; }
+  void await_suspend (coroutine_handle<>) {}
+  ret_type await_resume() { return {}; }
+};
+
+task
+my_coro ()
+{
+  ret_type r2{co_await awaiter{}};
+  //ret_type r3 (std::move(r2));
+}
+
+int main()
+{
+ auto x = my_coro ();
+ return 0;
+}