[PATCH Coroutines] Handle type deduction of auto and decltype(auto) with indirect_ref expression

[PATCH Coroutines] Handle type deduction of auto and decltype(auto) with indirect_ref expression

[JunMa]

[-- Attachment #1: Type: text/plain, Size: 1265 bytes --]

Hi
When test gcc with cppcoro, I find case like:

   int& awaitable::await_resume()
   auto x1 = co_await awaitable;
   decltype(auto) x2 = co_await awaitable;

Based on standard, typeof(x1) should be int, typeof(x2) is int&.
However, we get both x1 and x2 as int, this because we donot
consider indirect_ref which wrap await_resume call expression
(convert_from_reference), and it is invoked into type deduction
of auto and decltype(auto).

This patch wrap co_await expression with indirect_ref which should be
same with await_resume expression, and it sink await_resume expression
to real call_expr when replace co_await expression. it fix type deduction
of auto and decltype(auto) in coroutine.

Bootstrap and test on X86_64, is it OK?

Regards
JunMa

gcc/cp
2020-01-21  Jun Ma <JunMa@linux.alibaba.com>

         * coroutines.cc (build_co_await): Wrap co_await with
         indirect_ref when needed.
         (co_await_expander):  Sink to call_expr if await_resume
         is wrapped by indirect_ref.

gcc/testsuite
2020-01-21  Jun Ma <JunMa@linux.alibaba.com>

         * g++.dg/coroutines/co-await-14-return-ref-to-auto.C: Add label.

[-- Attachment #2: 0001-Handle-type-deduction-of-auto-and-decltype-auto-with.patch --]
[-- Type: text/plain, Size: 3017 bytes --]

---
 gcc/cp/coroutines.cc                          | 16 +++++--
 .../torture/co-await-14-return-ref-to-auto.C  | 45 +++++++++++++++++++
 2 files changed, 58 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/co-await-14-return-ref-to-auto.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index d25c1887c1e..0a8943d8348 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -743,9 +743,16 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind)
   TREE_VEC_ELT (awaiter_calls, 1) = awsp_call; /* await_suspend().  */
   TREE_VEC_ELT (awaiter_calls, 2) = awrs_call; /* await_resume().  */
 
-  return build5_loc (loc, CO_AWAIT_EXPR, TREE_TYPE (awrs_call), a,
-		     e_proxy, o, awaiter_calls,
-		     build_int_cst (integer_type_node, (int) suspend_kind));
+  tree await_expr = build5_loc (loc, CO_AWAIT_EXPR,
+				TREE_TYPE (TREE_TYPE (awrs_func)),
+				a, e_proxy, o, awaiter_calls,
+				build_int_cst (integer_type_node,
+					       (int) suspend_kind));
+  /* Wrap co_await_expr.  */
+  if (TREE_CODE (awrs_call) == INDIRECT_REF)
+    await_expr = build1_loc (loc, INDIRECT_REF, TREE_TYPE (awrs_call),
+			     await_expr);
+  return await_expr;
 }
 
 tree
@@ -1503,6 +1510,9 @@ co_await_expander (tree *stmt, int * /*do_subtree*/, void *d)
   /* This will produce the value (if one is provided) from the co_await
      expression.  */
   tree resume_call = TREE_VEC_ELT (awaiter_calls, 2); /* await_resume().  */
+  if (TREE_CODE (resume_call) == INDIRECT_REF)
+    /* Sink to await_resume call_expr.  */
+    resume_call = TREE_OPERAND (resume_call, 0);
   switch (stmt_code)
     {
     default: /* not likely to work .. but... */
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/co-await-14-return-ref-to-auto.C b/gcc/testsuite/g++.dg/coroutines/torture/co-await-14-return-ref-to-auto.C
new file mode 100644
index 00000000000..0a7c035ef2a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/co-await-14-return-ref-to-auto.C
@@ -0,0 +1,45 @@
+//  { dg-do run }
+
+/* The simplest valued co_await we can do.  */
+
+#include "../coro.h"
+
+// boiler-plate for tests of codegen
+#include "../coro1-ret-int-yield-int.h"
+
+
+coro1
+f ()
+{
+  int t1 = 5;
+  int t2 = 5;
+  auto gX = co_await coro1::suspend_always_intrefprt{t1};
+  if (gX != t1)
+	  abort();
+  decltype(auto) gX1 = co_await coro1::suspend_always_intrefprt{t2};
+  if (&gX1 != &t2)
+	  abort();
+  co_return t1 + 10;
+}
+
+int main ()
+{
+  PRINT ("main: create coro1");
+  struct coro1 f_coro = f ();
+  if (f_coro.handle.done())
+    {
+      PRINT ("main: we should not be 'done' [1]");
+      abort ();
+    }
+  PRINT ("main: resuming [1] initial suspend");
+  while (!f_coro.handle.done())
+    f_coro.handle.resume();
+  /* we should now have returned with the co_return (15) */
+  if (!f_coro.handle.done())
+    {
+      PRINT ("main: we should be 'done' ");
+      abort ();
+    }
+  puts ("main: done");
+  return 0;
+}
-- 
2.19.1.3.ge56e4f7

Re: [PATCH Coroutines] Handle type deduction of auto and decltype(auto) with indirect_ref expression

[Nathan Sidwell]

On 1/21/20 6:21 AM, JunMa wrote:
> Hi
> When test gcc with cppcoro, I find case like:
> 
>    int& awaitable::await_resume()
>    auto x1 = co_await awaitable;
>    decltype(auto) x2 = co_await awaitable;
> 
> Based on standard, typeof(x1) should be int, typeof(x2) is int&.
> However, we get both x1 and x2 as int, this because we donot
> consider indirect_ref which wrap await_resume call expression
> (convert_from_reference), and it is invoked into type deduction
> of auto and decltype(auto).
> 
> This patch wrap co_await expression with indirect_ref which should be
> same with await_resume expression, and it sink await_resume expression
> to real call_expr when replace co_await expression. it fix type deduction
> of auto and decltype(auto) in coroutine.
> 
> Bootstrap and test on X86_64, is it OK?

+  /* Wrap co_await_expr.  */
+  if (TREE_CODE (awrs_call) == INDIRECT_REF)
+    await_expr = build1_loc (loc, INDIRECT_REF, TREE_TYPE (awrs_call),
+			     await_expr);

I think all you want here is:
   await_expr = convert_from_reference (await_expr);

> 
> Regards
> JunMa
> 
> gcc/cp
> 2020-01-21  Jun Ma <JunMa@linux.alibaba.com>
> 
>          * coroutines.cc (build_co_await): Wrap co_await with
>          indirect_ref when needed.
>          (co_await_expander):  Sink to call_expr if await_resume
>          is wrapped by indirect_ref.
> 
> gcc/testsuite
> 2020-01-21  Jun Ma <JunMa@linux.alibaba.com>
> 
>          * g++.dg/coroutines/co-await-14-return-ref-to-auto.C: Add label.


-- 
Nathan Sidwell

Re: [PATCH Coroutines] Handle type deduction of auto and decltype(auto) with indirect_ref expression

[JunMa]

在 2020/1/28 上午12:01, Nathan Sidwell 写道:
> On 1/21/20 6:21 AM, JunMa wrote:
>> Hi
>> When test gcc with cppcoro, I find case like:
>>
>>    int& awaitable::await_resume()
>>    auto x1 = co_await awaitable;
>>    decltype(auto) x2 = co_await awaitable;
>>
>> Based on standard, typeof(x1) should be int, typeof(x2) is int&.
>> However, we get both x1 and x2 as int, this because we donot
>> consider indirect_ref which wrap await_resume call expression
>> (convert_from_reference), and it is invoked into type deduction
>> of auto and decltype(auto).
>>
>> This patch wrap co_await expression with indirect_ref which should be
>> same with await_resume expression, and it sink await_resume expression
>> to real call_expr when replace co_await expression. it fix type 
>> deduction
>> of auto and decltype(auto) in coroutine.
>>
>> Bootstrap and test on X86_64, is it OK?
>
> +  /* Wrap co_await_expr.  */
> +  if (TREE_CODE (awrs_call) == INDIRECT_REF)
> +    await_expr = build1_loc (loc, INDIRECT_REF, TREE_TYPE (awrs_call),
> +                 await_expr);
>
> I think all you want here is:
>   await_expr = convert_from_reference (await_expr);
>
Thanks, I'll update it.

Regards
JunMa
>>
>> Regards
>> JunMa
>>
>> gcc/cp
>> 2020-01-21  Jun Ma <JunMa@linux.alibaba.com>
>>
>>          * coroutines.cc (build_co_await): Wrap co_await with
>>          indirect_ref when needed.
>>          (co_await_expander):  Sink to call_expr if await_resume
>>          is wrapped by indirect_ref.
>>
>> gcc/testsuite
>> 2020-01-21  Jun Ma <JunMa@linux.alibaba.com>
>>
>>          * g++.dg/coroutines/co-await-14-return-ref-to-auto.C: Add 
>> label.
>
>

Re: [PATCH Coroutines v1] Handle type deduction of auto and decltype(auto) with indirect_ref expression

[JunMa]

[-- Attachment #1: Type: text/plain, Size: 2676 bytes --]

在 2020/2/3 上午9:03, JunMa 写道:
> 在 2020/1/28 上午12:01, Nathan Sidwell 写道:
>> On 1/21/20 6:21 AM, JunMa wrote:
>>> Hi
>>> When test gcc with cppcoro, I find case like:
>>>
>>>    int& awaitable::await_resume()
>>>    auto x1 = co_await awaitable;
>>>    decltype(auto) x2 = co_await awaitable;
>>>
>>> Based on standard, typeof(x1) should be int, typeof(x2) is int&.
>>> However, we get both x1 and x2 as int, this because we donot
>>> consider indirect_ref which wrap await_resume call expression
>>> (convert_from_reference), and it is invoked into type deduction
>>> of auto and decltype(auto).
>>>
>>> This patch wrap co_await expression with indirect_ref which should be
>>> same with await_resume expression, and it sink await_resume expression
>>> to real call_expr when replace co_await expression. it fix type 
>>> deduction
>>> of auto and decltype(auto) in coroutine.
>>>
>>> Bootstrap and test on X86_64, is it OK?
>>
>> +  /* Wrap co_await_expr.  */
>> +  if (TREE_CODE (awrs_call) == INDIRECT_REF)
>> +    await_expr = build1_loc (loc, INDIRECT_REF, TREE_TYPE (awrs_call),
>> +                 await_expr);
>>
>> I think all you want here is:
>>   await_expr = convert_from_reference (await_expr);
>>
> Thanks, I'll update it.
>
> Regards
> JunMa
Hi nathan,
Here is the update.

Regards
JunMa

gcc/cp
2020-02-03  Jun Ma <JunMa@linux.alibaba.com>

          * coroutines.cc (build_co_await): Call convert_from_reference
          to wrap co_await_expr with indirect_ref which avoid
          reference/non-reference type confusion.

          (co_await_expander):  Sink to call_expr if await_resume
          is wrapped by indirect_ref.

gcc/testsuite
2020-02-03  Jun Ma <JunMa@linux.alibaba.com>

          * g++.dg/coroutines/co-await-14-return-ref-to-auto.C: New test.
>
>>>
>>> Regards
>>> JunMa
>>>
>>> gcc/cp
>>> 2020-01-21  Jun Ma <JunMa@linux.alibaba.com>
>>>
>>>          * coroutines.cc (build_co_await): Wrap co_await with
>>>          indirect_ref when needed.
>>>          (co_await_expander):  Sink to call_expr if await_resume
>>>          is wrapped by indirect_ref.
>>>
>>> gcc/testsuite
>>> 2020-01-21  Jun Ma <JunMa@linux.alibaba.com>
>>>
>>>          * g++.dg/coroutines/co-await-14-return-ref-to-auto.C: Add 
>>> label.
>>
>>


[-- Attachment #2: 0001-Handle-type-deduction-of-auto-and-decltype-auto-with.patch --]
[-- Type: text/plain, Size: 2870 bytes --]

---
 gcc/cp/coroutines.cc                          | 12 +++--
 .../torture/co-await-14-return-ref-to-auto.C  | 45 +++++++++++++++++++
 2 files changed, 54 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/co-await-14-return-ref-to-auto.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 62d92d9fc98..e7a150d257f 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -749,9 +749,12 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind)
   TREE_VEC_ELT (awaiter_calls, 1) = awsp_call; /* await_suspend().  */
   TREE_VEC_ELT (awaiter_calls, 2) = awrs_call; /* await_resume().  */
 
-  return build5_loc (loc, CO_AWAIT_EXPR, TREE_TYPE (awrs_call), a,
-		     e_proxy, o, awaiter_calls,
-		     build_int_cst (integer_type_node, (int) suspend_kind));
+  tree await_expr = build5_loc (loc, CO_AWAIT_EXPR,
+				TREE_TYPE (TREE_TYPE (awrs_func)),
+				a, e_proxy, o, awaiter_calls,
+				build_int_cst (integer_type_node,
+					       (int) suspend_kind));
+  return convert_from_reference (await_expr);
 }
 
 tree
@@ -1512,6 +1515,9 @@ co_await_expander (tree *stmt, int * /*do_subtree*/, void *d)
   /* This will produce the value (if one is provided) from the co_await
      expression.  */
   tree resume_call = TREE_VEC_ELT (awaiter_calls, 2); /* await_resume().  */
+  if (TREE_CODE (resume_call) == INDIRECT_REF)
+    /* Sink to await_resume call_expr.  */
+    resume_call = TREE_OPERAND (resume_call, 0);
   switch (stmt_code)
     {
     default: /* not likely to work .. but... */
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/co-await-14-return-ref-to-auto.C b/gcc/testsuite/g++.dg/coroutines/torture/co-await-14-return-ref-to-auto.C
new file mode 100644
index 00000000000..0a7c035ef2a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/co-await-14-return-ref-to-auto.C
@@ -0,0 +1,45 @@
+//  { dg-do run }
+
+/* The simplest valued co_await we can do.  */
+
+#include "../coro.h"
+
+// boiler-plate for tests of codegen
+#include "../coro1-ret-int-yield-int.h"
+
+
+coro1
+f ()
+{
+  int t1 = 5;
+  int t2 = 5;
+  auto gX = co_await coro1::suspend_always_intrefprt{t1};
+  if (gX != t1)
+	  abort();
+  decltype(auto) gX1 = co_await coro1::suspend_always_intrefprt{t2};
+  if (&gX1 != &t2)
+	  abort();
+  co_return t1 + 10;
+}
+
+int main ()
+{
+  PRINT ("main: create coro1");
+  struct coro1 f_coro = f ();
+  if (f_coro.handle.done())
+    {
+      PRINT ("main: we should not be 'done' [1]");
+      abort ();
+    }
+  PRINT ("main: resuming [1] initial suspend");
+  while (!f_coro.handle.done())
+    f_coro.handle.resume();
+  /* we should now have returned with the co_return (15) */
+  if (!f_coro.handle.done())
+    {
+      PRINT ("main: we should be 'done' ");
+      abort ();
+    }
+  puts ("main: done");
+  return 0;
+}
-- 
2.19.1.3.ge56e4f7

Re: [PATCH Coroutines v1] Handle type deduction of auto and decltype(auto) with indirect_ref expression

[Nathan Sidwell]

On 2/2/20 9:28 PM, JunMa wrote:
> 在 2020/2/3 上午9:03, JunMa 写道:

>>> I think all you want here is:
>>>   await_expr = convert_from_reference (await_expr);
>>>
>> Thanks, I'll update it.


>>
>> Regards
>> JunMa
> Hi nathan,
> Here is the update.

>    /* This will produce the value (if one is provided) from the co_await
>       expression.  */
>    tree resume_call = TREE_VEC_ELT (awaiter_calls, 2); /* await_resume().  */
> +  if (TREE_CODE (resume_call) == INDIRECT_REF)
> +    /* Sink to await_resume call_expr.  */
> +    resume_call = TREE_OPERAND (resume_call, 0);

sorry, I should have realized you still needed this bit.  This too is 
stripping a reference access, right?  I.e. TYPE_REF_P (TREE_TYPE 
(resume_call)) is true. You should that as the condition too.

the other part of the patch is fine

nathan

-- 
Nathan Sidwell

Re: [PATCH Coroutines v2] Handle type deduction of auto and decltype(auto) with indirect_ref expression

[JunMa]

[-- Attachment #1: Type: text/plain, Size: 1094 bytes --]

在 2020/2/3 下午8:53, Nathan Sidwell 写道:
> On 2/2/20 9:28 PM, JunMa wrote:
>> 在 2020/2/3 上午9:03, JunMa 写道:
>
>>>> I think all you want here is:
>>>>   await_expr = convert_from_reference (await_expr);
>>>>
>>> Thanks, I'll update it.
>
>
>>>
>>> Regards
>>> JunMa
>> Hi nathan,
>> Here is the update.
>
>>    /* This will produce the value (if one is provided) from the co_await
>>       expression.  */
>>    tree resume_call = TREE_VEC_ELT (awaiter_calls, 2); /* 
>> await_resume().  */
>> +  if (TREE_CODE (resume_call) == INDIRECT_REF)
>> +    /* Sink to await_resume call_expr.  */
>> +    resume_call = TREE_OPERAND (resume_call, 0);
>
> sorry, I should have realized you still needed this bit.  This too is 
> stripping a reference access, right?  I.e. TYPE_REF_P (TREE_TYPE 
> (resume_call)) is true. You should that as the condition too.
>
you mean REFERENCE_REF_P (resume_call) ?  here is the update.

Regards
JunMa
> the other part of the patch is fine
>
> nathan
>


[-- Attachment #2: 0001-Handle-type-deduction-of-auto-and-decltype-auto-with.patch --]
[-- Type: text/plain, Size: 2860 bytes --]

---
 gcc/cp/coroutines.cc                          | 12 +++--
 .../torture/co-await-14-return-ref-to-auto.C  | 45 +++++++++++++++++++
 2 files changed, 54 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/co-await-14-return-ref-to-auto.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 8a0ce384425..0c2ec32d7db 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -800,9 +800,12 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind)
   TREE_VEC_ELT (awaiter_calls, 1) = awsp_call; /* await_suspend().  */
   TREE_VEC_ELT (awaiter_calls, 2) = awrs_call; /* await_resume().  */
 
-  return build5_loc (loc, CO_AWAIT_EXPR, TREE_TYPE (awrs_call), a,
-		     e_proxy, o, awaiter_calls,
-		     build_int_cst (integer_type_node, (int) suspend_kind));
+  tree await_expr = build5_loc (loc, CO_AWAIT_EXPR,
+				TREE_TYPE (TREE_TYPE (awrs_func)),
+				a, e_proxy, o, awaiter_calls,
+				build_int_cst (integer_type_node,
+					       (int) suspend_kind));
+  return convert_from_reference (await_expr);
 }
 
 tree
@@ -1563,6 +1566,9 @@ co_await_expander (tree *stmt, int * /*do_subtree*/, void *d)
   /* This will produce the value (if one is provided) from the co_await
      expression.  */
   tree resume_call = TREE_VEC_ELT (awaiter_calls, 2); /* await_resume().  */
+  if (REFERENCE_REF_P (resume_call))
+    /* Sink to await_resume call_expr.  */
+    resume_call = TREE_OPERAND (resume_call, 0);
   switch (stmt_code)
     {
     default: /* not likely to work .. but... */
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/co-await-14-return-ref-to-auto.C b/gcc/testsuite/g++.dg/coroutines/torture/co-await-14-return-ref-to-auto.C
new file mode 100644
index 00000000000..0a7c035ef2a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/co-await-14-return-ref-to-auto.C
@@ -0,0 +1,45 @@
+//  { dg-do run }
+
+/* The simplest valued co_await we can do.  */
+
+#include "../coro.h"
+
+// boiler-plate for tests of codegen
+#include "../coro1-ret-int-yield-int.h"
+
+
+coro1
+f ()
+{
+  int t1 = 5;
+  int t2 = 5;
+  auto gX = co_await coro1::suspend_always_intrefprt{t1};
+  if (gX != t1)
+	  abort();
+  decltype(auto) gX1 = co_await coro1::suspend_always_intrefprt{t2};
+  if (&gX1 != &t2)
+	  abort();
+  co_return t1 + 10;
+}
+
+int main ()
+{
+  PRINT ("main: create coro1");
+  struct coro1 f_coro = f ();
+  if (f_coro.handle.done())
+    {
+      PRINT ("main: we should not be 'done' [1]");
+      abort ();
+    }
+  PRINT ("main: resuming [1] initial suspend");
+  while (!f_coro.handle.done())
+    f_coro.handle.resume();
+  /* we should now have returned with the co_return (15) */
+  if (!f_coro.handle.done())
+    {
+      PRINT ("main: we should be 'done' ");
+      abort ();
+    }
+  puts ("main: done");
+  return 0;
+}
-- 
2.19.1.3.ge56e4f7

Re: [PATCH Coroutines v2] Handle type deduction of auto and decltype(auto) with indirect_ref expression

[Nathan Sidwell]

On 2/3/20 8:47 PM, JunMa wrote:
> 在 2020/2/3 下午8:53, Nathan Sidwell 写道:

>> sorry, I should have realized you still needed this bit.  This too is 
>> stripping a reference access, right?  I.e. TYPE_REF_P (TREE_TYPE 
>> (resume_call)) is true. You should that as the condition too.
>>
> you mean REFERENCE_REF_P (resume_call) ?  here is the update.

ah, missed that predicate -- well found!

yes, this is all good, thanks.

nathan

-- 
Nathan Sidwell