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 <i...@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 <i...@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
