[PATCH] coroutines: Fix for uses of structured binding [PR94701]

[Iain Sandoe]

Hi,

As reported by Michał Dominiak, we are generating incorrect code
for structured binding of local vars.  Somewhere in the machinations
associated with lambda captures, I messed up the code handling
DECL_VALUE_EXPRs. 

tested so far on x86_64-darwin16,
OK for master if it passes regstrap on x86-64-Linux?
thanks
Iain

====

Structured binding makes use of the DECL_VALUE_EXPR fields
in local variables.  We need to recognise these and only amend
the expression values, retaining the 'alias' value intact.

gcc/cp/ChangeLog:

2020-04-21  Iain Sandoe  <iain@sandoe.co.uk>

	PR c++/94701
	* coroutines.cc (struct local_var_info): Add fields for static
	variables and those with DECL_VALUE_EXPR redirection.
	(transform_local_var_uses):  Skip past typedefs and static vars
	and then account for redirected variables.
	(register_local_var_uses): Likewise.

gcc/testsuite/ChangeLog:

2020-04-21  Iain Sandoe  <iain@sandoe.co.uk>

	PR c++/94701
	* g++.dg/coroutines/torture/local-var-06-structured-binding.C: New test.
---
 gcc/cp/coroutines.cc                          | 40 ++++++++++----
 .../torture/local-var-06-structured-binding.C | 55 +++++++++++++++++++
 2 files changed, 84 insertions(+), 11 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/local-var-06-structured-binding.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 30676eba6c2..5580247edfc 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1776,6 +1776,8 @@ struct local_var_info
   tree field_idx;
   tree frame_type;
   bool is_lambda_capture;
+  bool is_static;
+  bool has_value_expr_p;
   location_t def_loc;
 };
 
@@ -1821,7 +1823,7 @@ transform_local_var_uses (tree *stmt, int *do_subtree, void *d)
 			NULL);
 
 	/* For capture proxies, this could include the decl value expr.  */
-	if (local_var.is_lambda_capture)
+	if (local_var.is_lambda_capture || local_var.has_value_expr_p)
 	  {
 	    tree ve = DECL_VALUE_EXPR (lvar);
 	    cp_walk_tree (&ve, transform_local_var_uses, d, NULL);
@@ -1854,15 +1856,12 @@ transform_local_var_uses (tree *stmt, int *do_subtree, void *d)
 
 	  /* 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)
+	     deal with the rest.
+	     Likewise, variables with their value found elsewhere.
+	     Skip past unused ones too.  */
+	  if (local_var.is_lambda_capture
+	     || local_var.has_value_expr_p
+	     || local_var.field_id == NULL_TREE)
 	    {
 	      pvar = &DECL_CHAIN (*pvar);
 	      continue;
@@ -1896,10 +1895,13 @@ transform_local_var_uses (tree *stmt, int *do_subtree, void *d)
      for the promise and coroutine handle(s), to global vars or to compiler
      temporaries.  Skip past these, we will handle them later.  */
   local_var_info *local_var_i = lvd->local_var_uses->get (var_decl);
+
   if (local_var_i == NULL)
     return NULL_TREE;
 
-  if (local_var_i->is_lambda_capture)
+  if (local_var_i->is_lambda_capture
+      || local_var_i->is_static
+      || local_var_i->has_value_expr_p)
     return NULL_TREE;
 
   /* This is our revised 'local' i.e. a frame slot.  */
@@ -3021,6 +3023,16 @@ register_local_var_uses (tree *stmt, int *do_subtree, void *d)
 	  tree lvtype = TREE_TYPE (lvar);
 	  local_var.frame_type = lvtype;
 	  local_var.field_idx = local_var.field_id = NULL_TREE;
+
+	  /* Make sure that we only present vars to the tests below.  */
+	  if (TREE_CODE (lvar) == TYPE_DECL)
+	    continue;
+
+	  /* We don't move static vars into the frame. */
+	  local_var.is_static = TREE_STATIC (lvar);
+	  if (local_var.is_static)
+	    continue;
+
 	  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
@@ -3029,6 +3041,12 @@ register_local_var_uses (tree *stmt, int *do_subtree, void *d)
 	  if (local_var.is_lambda_capture)
 	    continue;
 
+	  /* If a variable has a value expression, then that's what needs
+	     to be processed.  */
+	  local_var.has_value_expr_p = DECL_HAS_VALUE_EXPR_P (lvar);
+	  if (local_var.has_value_expr_p)
+	    continue;
+
 	  /* Make names depth+index unique, so that we can support nested
 	     scopes with identically named locals.  */
 	  tree lvname = DECL_NAME (lvar);
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/local-var-06-structured-binding.C b/gcc/testsuite/g++.dg/coroutines/torture/local-var-06-structured-binding.C
new file mode 100644
index 00000000000..ef3ff47c16f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/local-var-06-structured-binding.C
@@ -0,0 +1,55 @@
+//  { dg-do run }
+
+#include "../coro.h"
+
+struct promise;
+
+struct future
+{
+  using promise_type = promise;
+};
+
+struct promise
+{
+  template<typename... Args>
+  promise (Args&... args) {}
+ 
+  coro::suspend_never initial_suspend() { return {}; }
+  coro::suspend_never final_suspend() { return {}; }
+
+  future get_return_object() { return {}; }
+
+  void return_value(int) {}
+  void unhandled_exception() {}
+};
+
+struct pair
+{
+  int i;
+};
+
+pair 
+something ()
+{
+  return { 1 };
+}
+
+future 
+my_coro ()
+{   
+  auto ret = something ();
+
+  if (ret.i != 1)
+    abort ();
+
+  auto [ i ] = something ();
+  if (i != 1)
+    abort ();
+
+  co_return 1;
+}
+
+int main ()
+{
+  my_coro ();
+}
-- 
2.24.1