coroutines: Update lambda capture handling to n4849.

In the absence of specific comment on the handling of closures I'd
implemented something more than was intended (extending the lifetime
of lambda capture-by-copy vars to the duration of the coro).

After discussion at WG21 in February and by email, the correct handling
is to treat the closure "this" pointer the same way as for a regular one,
and thus it is the user's responsibility to ensure that the lambda capture
object has suitable lifetime for the coroutine.  It is noted that users
frequently get this wrong, so it would be a good thing to revisit for C++23.

This patch removes the additional copying behaviour for lambda capture-by-
copy vars.

gcc/cp/ChangeLog:

2020-03-02  Iain Sandoe  <iain@sandoe.co.uk>

	* coroutines.cc (struct local_var_info): Adjust to remove the
	reference to the captured var, and just to note that this is a
	lambda capture proxy.
	(transform_local_var_uses): Handle lambda captures specially.
	(struct param_frame_data): Add a visited set.
	(register_param_uses): Also check for param uses in lambda
	capture proxies.
	(struct local_vars_frame_data): Remove captures list.
	(register_local_var_uses): Handle lambda capture proxies by
	noting and bypassing them.
	(morph_fn_to_coro): Update to remove lifetime extension of
	lambda capture-by-copy vars.

gcc/testsuite/ChangeLog:

2020-03-02  Iain Sandoe  <iain@sandoe.co.uk>
	    Jun Ma <JunMa@linux.alibaba.com>

	* g++.dg/coroutines/torture/class-05-lambda-capture-copy-local.C:
	* g++.dg/coroutines/torture/lambda-09-init-captures.C: New test.
	* g++.dg/coroutines/torture/lambda-10-mutable.C: New test.

gcc/cp/ChangeLog

 2020-03-02  Iain Sandoe  <iain@sandoe.co.uk>
 
+	* coroutines.cc (struct local_var_info): Adjust to remove the
+	reference to the captured var, and just to note that this is a
+	lambda capture proxy.
+	(transform_local_var_uses): Handle lambda captures specially.
+	(struct param_frame_data): Add a visited set.
+	(register_param_uses): Also check for param uses in lambda
+	capture proxies.
+	(struct local_vars_frame_data): Remove captures list.
+	(register_local_var_uses): Handle lambda capture proxies by
+	noting and bypassing them.
+	(morph_fn_to_coro): Update to remove lifetime extension of
+	lambda capture-by-copy vars.
+
+2020-03-02  Iain Sandoe  <iain@sandoe.co.uk>
+
 	* coroutines.cc (build_co_await): Do not build frame
 	awaitable proxy vars when the co_await expression is
 	a function parameter or local var.

gcc/cp/coroutines.cc

@@ -1783,7 +1783,7 @@ struct local_var_info
   tree field_id;
   tree field_idx;
   tree frame_type;
-  tree captured;
+  bool is_lambda_capture;
   location_t def_loc;
 };
 
@@ -1828,6 +1828,14 @@ transform_local_var_uses (tree *stmt, int *do_subtree, void *d)
 	  cp_walk_tree (&DECL_SIZE_UNIT (lvar), transform_local_var_uses, d,
 			NULL);
 
+	/* For capture proxies, this could include the decl value expr.  */
+	if (local_var.is_lambda_capture)
+	  {
+	    tree ve = DECL_VALUE_EXPR (lvar);
+	    cp_walk_tree (&ve, transform_local_var_uses, d, NULL);
+	    continue; /* No frame entry for this.  */
+	  }
+
 	  /* TODO: implement selective generation of fields when vars are
 	     known not-used.  */
 	  if (local_var.field_id == NULL_TREE)
@@ -1842,8 +1850,9 @@ transform_local_var_uses (tree *stmt, int *do_subtree, void *d)
 	  local_var.field_idx = fld_idx;
 	}
       cp_walk_tree (&BIND_EXPR_BODY (*stmt), transform_local_var_uses, d, NULL);
+
       /* Now we have processed and removed references to the original vars,
-	 we can drop those from the bind.  */
+	 we can drop those from the bind - leaving capture proxies alone.  */
       for (tree *pvar = &BIND_EXPR_VARS (*stmt); *pvar != NULL;)
 	{
 	  bool existed;
@@ -1851,10 +1860,24 @@ transform_local_var_uses (tree *stmt, int *do_subtree, void *d)
 	    = lvd->local_var_uses->get_or_insert (*pvar, &existed);
 	  gcc_checking_assert (existed);
 
+	  /* Leave lambda closure captures alone, we replace the *this
+	     pointer with the frame version and let the normal process
+	     deal with the rest.  */
+	  if (local_var.is_lambda_capture)
+	    {
+	      pvar = &DECL_CHAIN (*pvar);
+	      continue;
+	    }
+
+	  /* It's not used, but we can let the optimizer deal with that.  */
 	  if (local_var.field_id == NULL_TREE)
-	    pvar = &DECL_CHAIN (*pvar); /* Wasn't used.  */
+	    {
+	      pvar = &DECL_CHAIN (*pvar);
+	      continue;
+	    }
 
-	  *pvar = DECL_CHAIN (*pvar); /* discard this one, we replaced it.  */
+	  /* Discard this one, we replaced it.  */
+	  *pvar = DECL_CHAIN (*pvar);
 	}
 
       *do_subtree = 0; /* We've done the body already.  */
@@ -1884,6 +1907,9 @@ transform_local_var_uses (tree *stmt, int *do_subtree, void *d)
   if (local_var_i == NULL)
     return NULL_TREE;
 
+  if (local_var_i->is_lambda_capture)
+    return NULL_TREE;
+
   /* This is our revised 'local' i.e. a frame slot.  */
   tree revised = local_var_i->field_idx;
   gcc_checking_assert (DECL_CONTEXT (var_decl) == lvd->context);
@@ -2877,6 +2903,7 @@ struct param_frame_data
 {
   tree *field_list;
   hash_map<tree, param_info> *param_uses;
+  hash_set<tree *> *visited;
   location_t loc;
   bool param_seen;
 };
@@ -2886,9 +2913,20 @@ register_param_uses (tree *stmt, int *do_subtree ATTRIBUTE_UNUSED, void *d)
 {
   param_frame_data *data = (param_frame_data *) d;
 
+  /* For lambda closure content, we have to look specifically.  */
+  if (TREE_CODE (*stmt) == VAR_DECL && DECL_HAS_VALUE_EXPR_P (*stmt))
+    {
+      tree t = DECL_VALUE_EXPR (*stmt);
+      return cp_walk_tree (&t, register_param_uses, d, NULL);
+    }
+
   if (TREE_CODE (*stmt) != PARM_DECL)
     return NULL_TREE;
 
+  /* If we already saw the containing expression, then we're done.  */
+  if (data->visited->add (stmt))
+    return NULL_TREE;
+
   bool existed;
   param_info &parm = data->param_uses->get_or_insert (*stmt, &existed);
   gcc_checking_assert (existed);
@@ -2911,7 +2949,6 @@ struct local_vars_frame_data
 {
   tree *field_list;
   hash_map<tree, local_var_info> *local_var_uses;
-  vec<local_var_info> *captures;
   unsigned int nest_depth, bind_indx;
   location_t loc;
   bool saw_capture;
@@ -2940,45 +2977,33 @@ register_local_var_uses (tree *stmt, int *do_subtree, void *d)
 	  local_var_info &local_var
 	    = lvd->local_var_uses->get_or_insert (lvar, &existed);
 	  gcc_checking_assert (!existed);
+	  local_var.def_loc = DECL_SOURCE_LOCATION (lvar);
 	  tree lvtype = TREE_TYPE (lvar);
-	  tree lvname = DECL_NAME (lvar);
-	  bool captured = is_normal_capture_proxy (lvar);
+	  local_var.frame_type = lvtype;
+	  local_var.field_idx = local_var.field_id = NULL_TREE;
+	  lvd->local_var_seen = true;
+	  /* If this var is a lambda capture proxy, we want to leave it alone,
+	     and later rewrite the DECL_VALUE_EXPR to indirect through the
+	     frame copy of the pointer to the lambda closure object.  */
+	  local_var.is_lambda_capture = is_capture_proxy (lvar);
+	  if (local_var.is_lambda_capture)
+	    continue;
+
 	  /* Make names depth+index unique, so that we can support nested
 	     scopes with identically named locals.  */
+	  tree lvname = DECL_NAME (lvar);
 	  char *buf;
-	  size_t namsize = sizeof ("__lv...") + 18;
-	  const char *nm = (captured ? "cp" : "lv");
 	  if (lvname != NULL_TREE)
-	    {
-	      namsize += IDENTIFIER_LENGTH (lvname);
-	      buf = (char *) alloca (namsize);
-	      snprintf (buf, namsize, "__%s.%u.%u.%s", nm, lvd->bind_indx,
-			lvd->nest_depth, IDENTIFIER_POINTER (lvname));
-	    }
+	    buf = xasprintf ("__lv.%u.%u.%s", lvd->bind_indx, lvd->nest_depth,
+			     IDENTIFIER_POINTER (lvname));
 	  else
-	    {
-	      namsize += 10; /* 'D' followed by an unsigned.  */
-	      buf = (char *) alloca (namsize);
-	      snprintf (buf, namsize, "__%s.%u.%u.D%u", nm, lvd->bind_indx,
-			lvd->nest_depth, DECL_UID (lvar));
-	    }
+	    buf = xasprintf ("__lv.%u.%u.D%u", lvd->bind_indx, lvd->nest_depth,
+			     DECL_UID (lvar));
 	  /* TODO: Figure out if we should build a local type that has any
 	     excess alignment or size from the original decl.  */
 	  local_var.field_id
 	    = coro_make_frame_entry (lvd->field_list, buf, lvtype, lvd->loc);
-	  local_var.def_loc = DECL_SOURCE_LOCATION (lvar);
-	  local_var.frame_type = lvtype;
-	  local_var.field_idx = NULL_TREE;
-	  if (captured)
-	    {
-	      gcc_checking_assert (DECL_INITIAL (lvar) == NULL_TREE);
-	      local_var.captured = lvar;
-	      lvd->captures->safe_push (local_var);
-	      lvd->saw_capture = true;
-	    }
-	  else
-	    local_var.captured = NULL;
-	  lvd->local_var_seen = true;
+	  free (buf);
 	  /* We don't walk any of the local var sub-trees, they won't contain
 	     any bind exprs.  */
 	}
@@ -3032,7 +3057,6 @@ act_des_fn (tree orig, tree fn_type, tree coro_frame_ptr, const char* name)
   short __resume_at;
   handle_type self_handle;
   (maybe) parameter copies.
-  (maybe) lambda capture copies.
   coro1::suspend_never_prt __is;
   (maybe) handle_type i_hand;
   coro1::suspend_always_prt __fs;
@@ -3064,7 +3088,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
   location_t fn_start = DECL_SOURCE_LOCATION (orig);
   gcc_rich_location fn_start_loc (fn_start);
 
-  /* Initial processing of the captured body.
+  /* Initial processing of the function-body.
      If we have no expressions or just an error then punt.  */
   tree body_start = expr_first (fnbody);
   if (body_start == NULL_TREE || body_start == error_mark_node)
@@ -3223,10 +3247,12 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
 	  free (buf);
 	}
 
-      param_frame_data param_data
-	= {&field_list, param_uses, fn_start, false};
       /* We want to record every instance of param's use, so don't include
-	 a 'visited' hash_set.  */
+	 a 'visited' hash_set on the tree walk, but only record a containing
+	 expression once.  */
+      hash_set<tree *> visited;
+      param_frame_data param_data
+	= {&field_list, param_uses, &visited, fn_start, false};
       cp_walk_tree (&fnbody, register_param_uses, &param_data, NULL);
     }
 
@@ -3277,10 +3303,8 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
   /* 4. Now make space for local vars, this is conservative again, and we
      would expect to delete unused entries later.  */
   hash_map<tree, local_var_info> local_var_uses;
-  auto_vec<local_var_info> captures;
-
   local_vars_frame_data local_vars_data
-    = {&field_list, &local_var_uses, &captures, 0, 0, fn_start, false, false};
+    = {&field_list, &local_var_uses, 0, 0, fn_start, false, false};
   cp_walk_tree (&fnbody, register_local_var_uses, &local_vars_data, NULL);
 
   /* Tie off the struct for now, so that we can build offsets to the
@@ -3302,16 +3326,6 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
   tree coro_fp = build_lang_decl (VAR_DECL, get_identifier ("coro.frameptr"),
 				  coro_frame_ptr);
   tree varlist = coro_fp;
-  local_var_info *cap;
-  if (!captures.is_empty())
-    for (int ix = 0; captures.iterate (ix, &cap); ix++)
-      {
-	if (cap->field_id == NULL_TREE)
-	  continue;
-	tree t = cap->captured;
-	DECL_CHAIN (t) = varlist;
-	varlist = t;
-      }
 
   /* Collected the scope vars we need ... only one for now. */
   BIND_EXPR_VARS (ramp_bind) = nreverse (varlist);
@@ -3668,62 +3682,6 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
       add_stmt (r);
     }
 
-  vec<tree, va_gc> *captures_dtor_list = NULL;
-  while (!captures.is_empty())
-    {
-      local_var_info cap = captures.pop();
-      if (cap.field_id == NULL_TREE)
-	continue;
-
-      tree fld_ref = lookup_member (coro_frame_type, cap.field_id,
-				    /*protect=*/1, /*want_type=*/0,
-				    tf_warning_or_error);
-      tree fld_idx
-	= build_class_member_access_expr (deref_fp, fld_ref, NULL_TREE,
-					  false, tf_warning_or_error);
-
-      tree cap_type = cap.frame_type;
-
-      /* When we have a reference, we do not want to change the referenced
-	 item, but actually to set the reference to the proxy var.  */
-      if (REFERENCE_REF_P (fld_idx))
-	fld_idx = TREE_OPERAND (fld_idx, 0);
-
-      if (TYPE_NEEDS_CONSTRUCTING (cap_type))
-	{
-	  vec<tree, va_gc> *p_in;
-	      if (TYPE_REF_P (cap_type)
-		  && (CLASSTYPE_LAZY_MOVE_CTOR (cap_type)
-		      || CLASSTYPE_LAZY_MOVE_ASSIGN (cap_type)
-		      || classtype_has_move_assign_or_move_ctor_p
-			    (cap_type, /*user_declared=*/true)))
-	    p_in = make_tree_vector_single (rvalue (cap.captured));
-	  else
-	    p_in = make_tree_vector_single (cap.captured);
-	  /* Construct in place or move as relevant.  */
-	  r = build_special_member_call (fld_idx, complete_ctor_identifier,
-					 &p_in, cap_type, LOOKUP_NORMAL,
-					 tf_warning_or_error);
-	  release_tree_vector (p_in);
-	  if (captures_dtor_list == NULL)
-	    captures_dtor_list = make_tree_vector ();
-	  vec_safe_push (captures_dtor_list, cap.field_id);
-	}
-      else
-	{
-	  if (!same_type_p (cap_type, TREE_TYPE (cap.captured)))
-	    r = build1_loc (DECL_SOURCE_LOCATION (cap.captured), CONVERT_EXPR,
-			    cap_type, cap.captured);
-	  else
-	    r = cap.captured;
-	  r = build_modify_expr (fn_start, fld_idx, cap_type,
-				 INIT_EXPR, DECL_SOURCE_LOCATION (cap.captured),
-				 r, TREE_TYPE (r));
-	}
-      r = coro_build_cvt_void_expr_stmt (r, fn_start);
-      add_stmt (r);
-    }
-
   /* Set up a new bind context for the GRO.  */
   tree gro_context_bind = build3 (BIND_EXPR, void_type_node, NULL, NULL, NULL);
   /* Make and connect the scope blocks.  */

gcc/testsuite/ChangeLog

+2020-03-02  Iain Sandoe  <iain@sandoe.co.uk>
+	    Jun Ma <JunMa@linux.alibaba.com>
+
+	* g++.dg/coroutines/torture/class-05-lambda-capture-copy-local.C:
+	* g++.dg/coroutines/torture/lambda-09-init-captures.C: New test.
+	* g++.dg/coroutines/torture/lambda-10-mutable.C: New test.
+
 2020-03-02  Uroš Bizjak  <ubizjak@gmail.com>
 
 	PR target/93997

gcc/testsuite/g++.dg/coroutines/torture/class-05-lambda-capture-copy-local.C

@@ -18,7 +18,7 @@ class foo
       auto l = [=](T y) -> coro1
       {
 	T x = y;
-	co_return co_await x + local;
+	co_return co_await x + y + local;
       };
       return l;
     }
@@ -43,7 +43,7 @@ int main ()
 
   /* Now we should have the co_returned value.  */
   int y = x.handle.promise().get_value();
-  if ( y != 20 )
+  if ( y != 37 )
     {
       PRINTF ("main: wrong result (%d).", y);
       abort ();

gcc/testsuite/g++.dg/coroutines/torture/lambda-09-init-captures.C

+//  { dg-do run }
+
+// lambda with initialized captures
+
+#include "../coro.h"
+
+// boiler-plate for tests of codegen
+#include "../coro1-ret-int-yield-int.h"
+
+int main ()
+{
+  int a_copy = 20;
+
+  auto f = [&a_ref = a_copy, a_copy = a_copy + 10]() -> coro1
+  {
+    a_ref += 20;
+    co_return a_ref + a_copy;
+  };
+
+  {
+    coro1 A = f ();
+    A.handle.resume();
+    if (a_copy != 40)
+      {
+        PRINT ("main: [a_copy = 40]");
+	abort ();
+      }
+  
+    int y = A.handle.promise().get_value();
+    if (y != 70)
+      {
+	PRINTF ("main: A co-ret = %d, should be 70\n", y);
+	abort ();
+      }
+  }
+
+  a_copy = 5;
+
+  coro1 B = f ();
+  B.handle.resume();
+  if (a_copy != 25)
+    {
+      PRINT ("main: [a_copy = 25]");
+      abort ();
+    }
+
+  int y = B.handle.promise().get_value();
+  if (y != 55)
+    {
+      PRINTF ("main: B co-ret = %d, should be 55\n", y);
+      abort ();
+    }
+  
+  return 0;
+}

gcc/testsuite/g++.dg/coroutines/torture/lambda-10-mutable.C

+//  { dg-do run }
+
+// lambda with mutable closure object.
+
+#include "../coro.h"
+
+// boiler-plate for tests of codegen
+#include "../coro1-ret-int-yield-int.h"
+
+/* Creates a coro lambda with a mutable closure and
+   suspend-always initial suspend.  */
+
+auto make_co_lambda ()
+{
+  return [i = 1] () mutable -> coro1 { co_return i++; };
+}
+
+/* We make this behave sequentially for the purposes of testing.  */
+int main()
+{
+  auto co_l = make_co_lambda ();
+  auto v1 = co_l ();
+  auto v2 = co_l ();
+  auto v3 = co_l ();
+
+  v3.handle.resume();
+  v2.handle.resume();
+  v1.handle.resume();
+
+  int res1 = v1.handle.promise().get_value ();
+  int res2 = v2.handle.promise().get_value ();
+  int res3 = v3.handle.promise().get_value ();
+  PRINTF ("main: co-lambda %d, %d, %d\n",res1, res2, res3);
+  if ( res1 != 3 || res2 != 2 || res3 != 1)
+    {
+      PRINT ("main: bad return value.");
+      abort ();
+    }
+
+  if (!v1.handle.done() || !v2.handle.done() || !v3.handle.done())
+    {
+      PRINT ("main: apparently something was not done...");
+      abort ();
+    }
+
+  PRINT ("main: done.");
+}
+