[coroutines] Handle component_ref in captures_temporary

Message ID b07f02b0-3f35-aa1d-239f-9fee4a00b19b@linux.alibaba.com
State New
Headers show
Series
  • [coroutines] Handle component_ref in captures_temporary
Related show

Commit Message

JunMa Feb. 12, 2020, 7:23 a.m.
Hi
In captures_temporary, the current implementation fails to handle
component_ref. This causes ice with case co_await A while
operator co_await is defined in base class of A. Also it is necessary
to capture the object of base class as if it is temporary object.

This patch strips component_ref to its base object and check it as usual.

Bootstrap and test on X86_64, is it OK?

Regards
JunMa

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

         * coroutines.cc (captures_temporary): Strip component_ref
         to its base object.

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

         * g++.dg/coroutines/torture/co-await-15-capture-comp-ref.C: New 
test.
---
 gcc/cp/coroutines.cc                          | 15 ++-
 .../torture/co-await-15-capture-comp-ref.C    | 99 +++++++++++++++++++
 2 files changed, 113 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/co-await-15-capture-comp-ref.C

Comments

JunMa Feb. 27, 2020, 2:18 a.m. | #1
在 2020/2/12 下午3:23, JunMa 写道:
Kindly ping

Regards
JunMa
> Hi

> In captures_temporary, the current implementation fails to handle

> component_ref. This causes ice with case co_await A while

> operator co_await is defined in base class of A. Also it is necessary

> to capture the object of base class as if it is temporary object.

>

> This patch strips component_ref to its base object and check it as usual.

>

> Bootstrap and test on X86_64, is it OK?

>

> Regards

> JunMa

>

> gcc/cp

> 2020-02-12  Jun Ma <JunMa@linux.alibaba.com>

>

>         * coroutines.cc (captures_temporary): Strip component_ref

>         to its base object.

>

> gcc/testsuite

> 2020-02-12  Jun Ma <JunMa@linux.alibaba.com>

>

>         * g++.dg/coroutines/torture/co-await-15-capture-comp-ref.C: 

> New test.

>
Nathan Sidwell March 2, 2020, 2:49 p.m. | #2
On 2/12/20 2:23 AM, JunMa wrote:
> Hi

> In captures_temporary, the current implementation fails to handle

> component_ref. This causes ice with case co_await A while

> operator co_await is defined in base class of A. Also it is necessary

> to capture the object of base class as if it is temporary object.

> 

> This patch strips component_ref to its base object and check it as usual.

> 

> Bootstrap and test on X86_64, is it OK?

> 

> Regards

> JunMa

> 

> gcc/cp

> 2020-02-12  Jun Ma <JunMa@linux.alibaba.com>

> 

>          * coroutines.cc (captures_temporary): Strip component_ref

>          to its base object.

> 

> gcc/testsuite

> 2020-02-12  Jun Ma <JunMa@linux.alibaba.com>

> 

>          * g++.dg/coroutines/torture/co-await-15-capture-comp-ref.C: New 

> test.

> 

> +

> +      /* In case of component_ref, we need to capture the object of base

> +	 class as if it is temporary object.  There are two possibilities:

> +	 (*base).field and base->field.  */

> +      while (TREE_CODE (parm) == COMPONENT_REF)

> +	{

> +	  parm = TREE_OPERAND (parm, 0);

> +	  if (TREE_CODE (parm) == INDIRECT_REF)

> +	    parm = TREE_OPERAND (parm, 0);

> +	  while (TREE_CODE (parm) == NOP_EXPR)

> +	    parm = TREE_OPERAND (parm, 0);


Use STRIP_NOPS.

> +	}

> +

>        if (TREE_CODE (parm) == VAR_DECL && !DECL_ARTIFICIAL (parm))

>  	/* This isn't a temporary... */

>  	continue;

>  

> -      if (TREE_CODE (parm) == PARM_DECL)

> +      if (TREE_CODE (parm) == PARM_DECL  || TREE_CODE (parm) == NON_LVALUE_EXPR)

>  	/* .. nor is this... */

>  	continue;


Either a separate if, or merging both ifs (my preference) would be better.

nathan

-- 
Nathan Sidwell
JunMa March 3, 2020, 5:42 a.m. | #3
在 2020/3/2 下午10:49, Nathan Sidwell 写道:
> On 2/12/20 2:23 AM, JunMa wrote:

>> Hi

>> In captures_temporary, the current implementation fails to handle

>> component_ref. This causes ice with case co_await A while

>> operator co_await is defined in base class of A. Also it is necessary

>> to capture the object of base class as if it is temporary object.

>>

>> This patch strips component_ref to its base object and check it as 

>> usual.

>>

>> Bootstrap and test on X86_64, is it OK?

>>

>> Regards

>> JunMa

>>

>> gcc/cp

>> 2020-02-12  Jun Ma <JunMa@linux.alibaba.com>

>>

>>          * coroutines.cc (captures_temporary): Strip component_ref

>>          to its base object.

>>

>> gcc/testsuite

>> 2020-02-12  Jun Ma <JunMa@linux.alibaba.com>

>>

>>          * g++.dg/coroutines/torture/co-await-15-capture-comp-ref.C: 

>> New test.

>>

>> +

>> +      /* In case of component_ref, we need to capture the object of 

>> base

>> +     class as if it is temporary object.  There are two possibilities:

>> +     (*base).field and base->field.  */

>> +      while (TREE_CODE (parm) == COMPONENT_REF)

>> +    {

>> +      parm = TREE_OPERAND (parm, 0);

>> +      if (TREE_CODE (parm) == INDIRECT_REF)

>> +        parm = TREE_OPERAND (parm, 0);

>> +      while (TREE_CODE (parm) == NOP_EXPR)

>> +        parm = TREE_OPERAND (parm, 0);

>

> Use STRIP_NOPS.

>

>> +    }

>> +

>>        if (TREE_CODE (parm) == VAR_DECL && !DECL_ARTIFICIAL (parm))

>>      /* This isn't a temporary... */

>>      continue;

>>

>> -      if (TREE_CODE (parm) == PARM_DECL)

>> +      if (TREE_CODE (parm) == PARM_DECL  || TREE_CODE (parm) == 

>> NON_LVALUE_EXPR)

>>      /* .. nor is this... */

>>      continue;

>

> Either a separate if, or merging both ifs (my preference) would be 

> better.

>

> nathan

>

Hi nathan

Here is the updated patch

Regards
JunMa
---
 gcc/cp/coroutines.cc                          | 20 +++-
 .../torture/co-await-15-capture-comp-ref.C    | 99 +++++++++++++++++++
 2 files changed, 114 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/co-await-15-capture-comp-ref.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 966ec0583aa..2a54bcefc1e 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -2613,12 +2613,22 @@ captures_temporary (tree *stmt, int *do_subtree, void *d)
 	continue;
 
       parm = TREE_OPERAND (parm, 0);
-      if (TREE_CODE (parm) == VAR_DECL && !DECL_ARTIFICIAL (parm))
-	/* This isn't a temporary... */
-	continue;
 
-      if (TREE_CODE (parm) == PARM_DECL)
-	/* .. nor is this... */
+      /* In case of component_ref, we need to capture the object of base
+	 class as if it is temporary object.  There are two possibilities:
+	 (*base).field and base->field.  */
+      while (TREE_CODE (parm) == COMPONENT_REF)
+	{
+	  parm = TREE_OPERAND (parm, 0);
+	  if (TREE_CODE (parm) == INDIRECT_REF)
+	    parm = TREE_OPERAND (parm, 0);
+	  parm = STRIP_NOPS (parm);
+	}
+
+      /* This isn't a temporary or argument.  */
+      if ((TREE_CODE (parm) == VAR_DECL && !DECL_ARTIFICIAL (parm))
+	  || TREE_CODE (parm) == PARM_DECL
+	  || TREE_CODE (parm) == NON_LVALUE_EXPR)
 	continue;
 
       if (TREE_CODE (parm) == TARGET_EXPR)
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/co-await-15-capture-comp-ref.C b/gcc/testsuite/g++.dg/coroutines/torture/co-await-15-capture-comp-ref.C
new file mode 100644
index 00000000000..93a43fbd298
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/co-await-15-capture-comp-ref.C
@@ -0,0 +1,99 @@
+//  { dg-do run }
+
+#include "../coro.h"
+
+class resumable {
+public:
+  struct promise_type;
+  using coro_handle = std::coroutine_handle<promise_type>;
+  resumable(coro_handle handle) : handle_(handle) { }
+  resumable(resumable&) = delete;
+  resumable(resumable&&) = delete;
+  ~resumable() { handle_.destroy(); }
+  coro_handle handle_;
+};
+
+struct resumable::promise_type {
+  using coro_handle = std::coroutine_handle<promise_type>;
+  int used;
+  auto get_return_object() {
+    return coro_handle::from_promise(*this);
+  }
+  auto initial_suspend() { return std::suspend_never(); }
+  auto final_suspend() { return std::suspend_always(); }
+  void return_value(int x) {used = x;}
+  void unhandled_exception() {}
+
+  struct TestAwaiter {
+    int recent_test;
+    TestAwaiter(int test) : recent_test{test} {}
+    bool await_ready() { return false; }
+    void await_suspend(std::coroutine_handle<promise_type>) {}
+    int await_resume() {
+      return recent_test;
+    }
+    auto operator co_await() {
+      return *this;
+    }
+  };
+
+  struct TestAwaiterCH :TestAwaiter { 
+    TestAwaiterCH(int test) : TestAwaiter(test) {};
+  };
+
+  struct TestAwaiterCHCH :TestAwaiterCH {
+    TestAwaiterCHCH(int test) : TestAwaiterCH(test) {};
+
+    resumable foo(){
+    int x = co_await *this;
+    co_return x;
+    }
+  };
+};
+
+struct TestP {
+ resumable::promise_type::TestAwaiterCHCH  tp = resumable::promise_type::TestAwaiterCHCH(6);
+};
+
+resumable foo1(int t){
+  int x = co_await resumable::promise_type::TestAwaiterCH(t);
+  co_return x;
+}
+
+resumable foo2(){
+  struct TestP  TP;
+  int x = co_await TP.tp;
+  co_return x;
+}
+
+resumable foo3(){
+  int x = co_await TestP{}.tp;
+  co_return x;
+}
+
+int main(){
+  auto t = resumable::promise_type::TestAwaiterCHCH(4);
+  resumable res = t.foo();
+  while (!res.handle_.done())
+    res.handle_.resume();
+  if (res.handle_.promise().used != 4)
+    abort();
+
+  resumable res1 = foo1(5);
+  while (!res1.handle_.done())
+    res1.handle_.resume();
+  if (res1.handle_.promise().used != 5)
+    abort();
+
+  resumable res2 = foo2();
+  while (!res2.handle_.done())
+    res2.handle_.resume();
+  if (res2.handle_.promise().used != 6)
+    abort();
+  
+  resumable res3 = foo2();
+  while (!res3.handle_.done())
+    res3.handle_.resume();
+  if (res3.handle_.promise().used != 6)
+    abort();
+}
Nathan Sidwell March 3, 2020, 12:15 p.m. | #4
On 3/3/20 12:42 AM, JunMa wrote:
> 在 2020/3/2 下午10:49, Nathan Sidwell 写道:

>> On 2/12/20 2:23 AM, JunMa wrote:


> Hi nathan

> 

> Here is the updated patch


This is ok, with a correction in a comment:
+      /* This isn't a temporary or argument.  */
        /* This isn't a temporary.  */
  is sufficient.  Otherwise it reads as 'this is neither a temporary nor 
an argument' which isn't the case.

nathan

-- 
Nathan Sidwell
JunMa March 3, 2020, 1:47 p.m. | #5
在 2020/3/3 下午8:15, Nathan Sidwell 写道:
> On 3/3/20 12:42 AM, JunMa wrote:

>> 在 2020/3/2 下午10:49, Nathan Sidwell 写道:

>>> On 2/12/20 2:23 AM, JunMa wrote:

>

>> Hi nathan

>>

>> Here is the updated patch

>

> This is ok, with a correction in a comment:

> +      /* This isn't a temporary or argument.  */

>        /* This isn't a temporary.  */

>  is sufficient.  Otherwise it reads as 'this is neither a temporary 

> nor an argument' which isn't the case.

>

Thanks, will check in later.

Regards
JunMa
> nathan

>

Patch

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 1a77f5dbfce..a6adb946df1 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -2474,11 +2474,24 @@  captures_temporary (tree *stmt, int *do_subtree, void *d)
 	continue;
 
       parm = TREE_OPERAND (parm, 0);
+
+      /* In case of component_ref, we need to capture the object of base
+	 class as if it is temporary object.  There are two possibilities:
+	 (*base).field and base->field.  */
+      while (TREE_CODE (parm) == COMPONENT_REF)
+	{
+	  parm = TREE_OPERAND (parm, 0);
+	  if (TREE_CODE (parm) == INDIRECT_REF)
+	    parm = TREE_OPERAND (parm, 0);
+	  while (TREE_CODE (parm) == NOP_EXPR)
+	    parm = TREE_OPERAND (parm, 0);
+	}
+
       if (TREE_CODE (parm) == VAR_DECL && !DECL_ARTIFICIAL (parm))
 	/* This isn't a temporary... */
 	continue;
 
-      if (TREE_CODE (parm) == PARM_DECL)
+      if (TREE_CODE (parm) == PARM_DECL  || TREE_CODE (parm) == NON_LVALUE_EXPR)
 	/* .. nor is this... */
 	continue;
 
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/co-await-15-capture-comp-ref.C b/gcc/testsuite/g++.dg/coroutines/torture/co-await-15-capture-comp-ref.C
new file mode 100644
index 00000000000..93a43fbd298
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/co-await-15-capture-comp-ref.C
@@ -0,0 +1,99 @@ 
+//  { dg-do run }
+
+#include "../coro.h"
+
+class resumable {
+public:
+  struct promise_type;
+  using coro_handle = std::coroutine_handle<promise_type>;
+  resumable(coro_handle handle) : handle_(handle) { }
+  resumable(resumable&) = delete;
+  resumable(resumable&&) = delete;
+  ~resumable() { handle_.destroy(); }
+  coro_handle handle_;
+};
+
+struct resumable::promise_type {
+  using coro_handle = std::coroutine_handle<promise_type>;
+  int used;
+  auto get_return_object() {
+    return coro_handle::from_promise(*this);
+  }
+  auto initial_suspend() { return std::suspend_never(); }
+  auto final_suspend() { return std::suspend_always(); }
+  void return_value(int x) {used = x;}
+  void unhandled_exception() {}
+
+  struct TestAwaiter {
+    int recent_test;
+    TestAwaiter(int test) : recent_test{test} {}
+    bool await_ready() { return false; }
+    void await_suspend(std::coroutine_handle<promise_type>) {}
+    int await_resume() {
+      return recent_test;
+    }
+    auto operator co_await() {
+      return *this;
+    }
+  };
+
+  struct TestAwaiterCH :TestAwaiter { 
+    TestAwaiterCH(int test) : TestAwaiter(test) {};
+  };
+
+  struct TestAwaiterCHCH :TestAwaiterCH {
+    TestAwaiterCHCH(int test) : TestAwaiterCH(test) {};
+
+    resumable foo(){
+    int x = co_await *this;
+    co_return x;
+    }
+  };
+};
+
+struct TestP {
+ resumable::promise_type::TestAwaiterCHCH  tp = resumable::promise_type::TestAwaiterCHCH(6);
+};
+
+resumable foo1(int t){
+  int x = co_await resumable::promise_type::TestAwaiterCH(t);
+  co_return x;
+}
+
+resumable foo2(){
+  struct TestP  TP;
+  int x = co_await TP.tp;
+  co_return x;
+}
+
+resumable foo3(){
+  int x = co_await TestP{}.tp;
+  co_return x;
+}
+
+int main(){
+  auto t = resumable::promise_type::TestAwaiterCHCH(4);
+  resumable res = t.foo();
+  while (!res.handle_.done())
+    res.handle_.resume();
+  if (res.handle_.promise().used != 4)
+    abort();
+
+  resumable res1 = foo1(5);
+  while (!res1.handle_.done())
+    res1.handle_.resume();
+  if (res1.handle_.promise().used != 5)
+    abort();
+
+  resumable res2 = foo2();
+  while (!res2.handle_.done())
+    res2.handle_.resume();
+  if (res2.handle_.promise().used != 6)
+    abort();
+  
+  resumable res3 = foo2();
+  while (!res3.handle_.done())
+    res3.handle_.resume();
+  if (res3.handle_.promise().used != 6)
+    abort();
+}