| Message ID | b07f02b0-3f35-aa1d-239f-9fee4a00b19b@linux.alibaba.com |
|---|---|
| State | New |
| Headers | show |
| Series | [coroutines] Handle component_ref in captures_temporary | expand |
在 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. >
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
在 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(); +}
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
在 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 >
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(); +}