| Message ID | 87125790-f1ee-6c4c-6489-700be72a25a3@linux.alibaba.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Coroutines] Handle type deduction of auto and decltype(auto) with indirect_ref expression | expand |
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.
在 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. > >
在 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. >> >> --- 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; +}
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
在 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 > --- 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; +}
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
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; +}