Re: [PING PATCH coroutines] Do not strip cleanup_point when promote temporaries out of current stmt

[Iain Sandoe]

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.

2/ @ Bin / Jun Ma

  - The problem is that the testcase seems to be fragile, perhaps it was a 
c-reduced one?

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?
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 ();