On Tue, Apr 7, 2020 at 11:45 PM Iain Sandoe <i...@sandoe.co.uk> wrote: > > Bin.Cheng <amker.ch...@gmail.com> wrote: > > > On Mon, Mar 16, 2020 at 5:34 PM Bin.Cheng <amker.ch...@gmail.com> wrote: > >> On Wed, Mar 11, 2020 at 5:07 PM Iain Sandoe <i...@sandoe.co.uk> wrote: > >>> Bin.Cheng <amker.ch...@gmail.com> wrote: > >>> > >>>> On Thu, Mar 5, 2020 at 10:18 PM Iain Sandoe <i...@sandoe.co.uk> wrote: > > >>> If it helps, I could push a branch to users/iains/ on the FSF git server > >>> with this sequence. > >> Sorry for being slow replying. This is weird, were you testing against > >> trunk? > > 1/ @Jun Ma > > - I think your observation is correct, we should enusre that cleanups are > applied where needed > to any temps that remain after we’ve those captured by reference. > > However, I would prefer that we do this as required, rather than assuming > it will be needed so > I have an updated patch below. Sorry it's been quite long, I don't remember the detail of this change. Since this new version fixes the ICE too, please go ahead commit it. > > 2/ @ Bin / Jun Ma > > - The problem is that the testcase seems to be fragile, perhaps it was a > c-reduced one? It's strange, however, I don't see being c-reduced has impact here in this scenario. > > So… I do not propose to add this test-case at this point (perhaps we could > construct one that actually > tests the required behaviour - that cleanups are still run correctly for > temps that are not promoted by > capture)? > > Anyway to avoid further delay, I think we should apply the patch below (I > have other fixes on top of this > for open PRs) > > OK for master? Yes, please. Also one of approved patch is waiting for this one.
Thanks, bin > thanks > Iain > > coroutines: Add cleanups, where required, to statements with captured > references. > > When we promote captured temporaries to local variables, we also > remove their initializers from the relevant call expression. This > means that we should recompute the need for a cleanup expression > once the set of temporaries that remains becomes known. > > gcc/cp/ChangeLog: > > 2020-04-07 Iain Sandoe <i...@sandoe.co.uk> > Jun Ma <ju...@linux.alibaba.com> > > * coroutines.cc (maybe_promote_captured_temps): Add a > cleanup expression, if needed, to any call from which > we promoted temporaries captured by reference. > > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc > index 983fa650b55..936be06c336 100644 > --- a/gcc/cp/coroutines.cc > +++ b/gcc/cp/coroutines.cc > @@ -2798,11 +2798,13 @@ maybe_promote_captured_temps (tree *stmt, void *d) > location_t sloc = EXPR_LOCATION (*stmt); > tree aw_bind > = build3_loc (sloc, BIND_EXPR, void_type_node, NULL, NULL, NULL); > - tree aw_statement_current; > - if (TREE_CODE (*stmt) == CLEANUP_POINT_EXPR) > - aw_statement_current = TREE_OPERAND (*stmt, 0); > - else > - aw_statement_current = *stmt; > + > + /* Any cleanup point expression might no longer be necessary, since we > + are removing one or more temporaries. */ > + tree aw_statement_current = *stmt; > + if (TREE_CODE (aw_statement_current) == CLEANUP_POINT_EXPR) > + aw_statement_current = TREE_OPERAND (aw_statement_current, 0); > + > /* Collected the scope vars we need move the temps to regular. */ > tree aw_bind_body = push_stmt_list (); > tree varlist = NULL_TREE; > @@ -2843,8 +2845,12 @@ maybe_promote_captured_temps (tree *stmt, void *d) > /* Replace all instances of that temp in the original expr. */ > cp_walk_tree (&aw_statement_current, replace_proxy, &pr, NULL); > } > - /* What's left should be the original statement with any temporaries > - broken out. */ > + > + /* What's left should be the original statement with any co_await > + captured temporaries broken out. Other temporaries might remain > + so see if we need to wrap the revised statement in a cleanup. */ > + aw_statement_current = > + maybe_cleanup_point_expr_void (aw_statement_current); > add_stmt (aw_statement_current); > BIND_EXPR_BODY (aw_bind) = pop_stmt_list (aw_bind_body); > awpts->captured_temps.empty (); >
