OK
On Fri, Sep 7, 2018 at 4:29 AM, Marek Polacek <pola...@redhat.com> wrote:
> Turned out our range-based for loops with initializer are completely broken
> in templates, because I neglected to add the init-statement to RANGE_FOR_STMT.
> range-for18.C should verify we put it into the right scope.
>
> tsubst_expr then needs to recurse on it to avoid the ICE.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2018-09-06 Marek Polacek <pola...@redhat.com>
>
> PR c++/87152 - range-based for loops with initializer broken in
> templates.
> * constexpr.c (potential_constant_expression_1) <case RANGE_FOR_STMT>:
> Recur into RANGE_FOR_INIT_STMT.
> * cp-tree.def: Add RANGE_FOR_INIT_STMT to RANGE_FOR_STMT.
> * cp-tree.h (RANGE_FOR_INIT_STMT): Define.
> * dump.c (cp_dump_tree) <case RANGE_FOR_STMT>: Also dump
> RANGE_FOR_INIT_STMT.
> * pt.c (tsubst_expr) <case RANGE_FOR_STMT>: Recur into
> RANGE_FOR_INIT_STMT.
> * semantics.c (begin_range_for_stmt): Adjust call to build_stmt.
> Do put the init statement in RANGE_FOR_INIT_STMT.
> (finish_range_for_decl): Pop it for templates.
>
> * g++.dg/cpp2a/range-for11.C: New test.
> * g++.dg/cpp2a/range-for12.C: New test.
> * g++.dg/cpp2a/range-for13.C: New test.
> * g++.dg/cpp2a/range-for14.C: New test.
> * g++.dg/cpp2a/range-for15.C: New test.
> * g++.dg/cpp2a/range-for16.C: New test.
> * g++.dg/cpp2a/range-for17.C: New test.
> * g++.dg/cpp2a/range-for18.C: New test.
> * g++.dg/parse/error61.C (foo): Adjust dg-error.
>
> diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
> index f646519135f..6c2689064f3 100644
> --- gcc/cp/constexpr.c
> +++ gcc/cp/constexpr.c
> @@ -5767,6 +5767,8 @@ potential_constant_expression_1 (tree t, bool
> want_rval, bool strict, bool now,
> return true;
>
> case RANGE_FOR_STMT:
> + if (!RECUR (RANGE_FOR_INIT_STMT (t), any))
> + return false;
> if (!RECUR (RANGE_FOR_EXPR (t), any))
> return false;
> if (!RECUR (RANGE_FOR_BODY (t), any))
> diff --git gcc/cp/cp-tree.def gcc/cp/cp-tree.def
> index 1b0326f4e81..c64225ded6f 100644
> --- gcc/cp/cp-tree.def
> +++ gcc/cp/cp-tree.def
> @@ -301,9 +301,10 @@ DEFTREECODE (IF_STMT, "if_stmt", tcc_statement, 4)
> DEFTREECODE (FOR_STMT, "for_stmt", tcc_statement, 5)
>
> /* Used to represent a range-based `for' statement. The operands are
> - RANGE_FOR_DECL, RANGE_FOR_EXPR, RANGE_FOR_BODY, and RANGE_FOR_SCOPE,
> - RANGE_FOR_UNROLL respectively. Only used in templates. */
> -DEFTREECODE (RANGE_FOR_STMT, "range_for_stmt", tcc_statement, 5)
> + RANGE_FOR_DECL, RANGE_FOR_EXPR, RANGE_FOR_BODY, RANGE_FOR_SCOPE,
> + RANGE_FOR_UNROLL, and RANGE_FOR_INIT_STMT, respectively. Only used in
> + templates. */
> +DEFTREECODE (RANGE_FOR_STMT, "range_for_stmt", tcc_statement, 6)
>
> /* Used to represent a 'while' statement. The operands are WHILE_COND
> and WHILE_BODY, respectively. */
> diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
> index df441fca304..b78e9eb252b 100644
> --- gcc/cp/cp-tree.h
> +++ gcc/cp/cp-tree.h
> @@ -4923,6 +4923,7 @@ more_aggr_init_expr_args_p (const
> aggr_init_expr_arg_iterator *iter)
> #define RANGE_FOR_BODY(NODE) TREE_OPERAND (RANGE_FOR_STMT_CHECK (NODE), 2)
> #define RANGE_FOR_SCOPE(NODE) TREE_OPERAND (RANGE_FOR_STMT_CHECK (NODE), 3)
> #define RANGE_FOR_UNROLL(NODE) TREE_OPERAND (RANGE_FOR_STMT_CHECK (NODE), 4)
> +#define RANGE_FOR_INIT_STMT(NODE) TREE_OPERAND (RANGE_FOR_STMT_CHECK (NODE),
> 5)
> #define RANGE_FOR_IVDEP(NODE) TREE_LANG_FLAG_6 (RANGE_FOR_STMT_CHECK (NODE))
>
> #define SWITCH_STMT_COND(NODE) TREE_OPERAND (SWITCH_STMT_CHECK (NODE), 0)
> diff --git gcc/cp/dump.c gcc/cp/dump.c
> index 9c1e5fc781a..d9b868bfaef 100644
> --- gcc/cp/dump.c
> +++ gcc/cp/dump.c
> @@ -301,6 +301,7 @@ cp_dump_tree (void* dump_info, tree t)
>
> case RANGE_FOR_STMT:
> dump_stmt (di, t);
> + dump_child ("init", RANGE_FOR_INIT_STMT (t));
> dump_child ("decl", RANGE_FOR_DECL (t));
> dump_child ("expr", RANGE_FOR_EXPR (t));
> dump_child ("body", RANGE_FOR_BODY (t));
> diff --git gcc/cp/pt.c gcc/cp/pt.c
> index 0a618a5447d..892a387cbc5 100644
> --- gcc/cp/pt.c
> +++ gcc/cp/pt.c
> @@ -16815,6 +16815,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t
> complain, tree in_decl,
> stmt = (processing_template_decl
> ? begin_range_for_stmt (NULL_TREE, NULL_TREE)
> : begin_for_stmt (NULL_TREE, NULL_TREE));
> + RECUR (RANGE_FOR_INIT_STMT (t));
> decl = RANGE_FOR_DECL (t);
> decl = tsubst (decl, args, complain, in_decl);
> maybe_push_decl (decl);
> diff --git gcc/cp/semantics.c gcc/cp/semantics.c
> index 676de011868..f3e5d83b1ef 100644
> --- gcc/cp/semantics.c
> +++ gcc/cp/semantics.c
> @@ -1101,8 +1101,8 @@ begin_range_for_stmt (tree scope, tree init)
> {
> begin_maybe_infinite_loop (boolean_false_node);
>
> - tree r = build_stmt (input_location, RANGE_FOR_STMT,
> - NULL_TREE, NULL_TREE, NULL_TREE, NULL_TREE, NULL_TREE);
> + tree r = build_stmt (input_location, RANGE_FOR_STMT, NULL_TREE, NULL_TREE,
> + NULL_TREE, NULL_TREE, NULL_TREE, NULL_TREE);
>
> if (scope == NULL_TREE)
> {
> @@ -1110,22 +1110,23 @@ begin_range_for_stmt (tree scope, tree init)
> scope = begin_for_scope (&init);
> }
>
> - /* RANGE_FOR_STMTs do not use nor save the init tree, so we
> - pop it now. */
> - if (init)
> - pop_stmt_list (init);
> + /* Since C++20, RANGE_FOR_STMTs can use the init tree, so save it. */
> + RANGE_FOR_INIT_STMT (r) = init;
> RANGE_FOR_SCOPE (r) = scope;
>
> return r;
> }
>
> /* Finish the head of a range-based for statement, which may
> - be given by RANGE_FOR_STMT. DECL must be the declaration
> + be given by RANGE_FOR_STMT. DECL must be the declaration
> and EXPR must be the loop expression. */
>
> void
> finish_range_for_decl (tree range_for_stmt, tree decl, tree expr)
> {
> + if (processing_template_decl)
> + RANGE_FOR_INIT_STMT (range_for_stmt)
> + = pop_stmt_list (RANGE_FOR_INIT_STMT (range_for_stmt));
> RANGE_FOR_DECL (range_for_stmt) = decl;
> RANGE_FOR_EXPR (range_for_stmt) = expr;
> add_stmt (range_for_stmt);
> diff --git gcc/testsuite/g++.dg/cpp2a/range-for11.C
> gcc/testsuite/g++.dg/cpp2a/range-for11.C
> index e69de29bb2d..5f0a80318ab 100644
> --- gcc/testsuite/g++.dg/cpp2a/range-for11.C
> +++ gcc/testsuite/g++.dg/cpp2a/range-for11.C
> @@ -0,0 +1,22 @@
> +// PR c++/87152
> +// { dg-do run }
> +// { dg-options "-std=c++2a" }
> +
> +template<typename>
> +int foo ()
> +{
> + int a[] = { 1, 2, 3, 4, 5 };
> + int j = 0;
> + for (int i = 0; auto x : a)
> + j += i++;
> +
> + return j;
> +}
> +
> +int
> +main ()
> +{
> + int j = foo<int>();
> + if (j != 10)
> + __builtin_abort ();
> +}
> diff --git gcc/testsuite/g++.dg/cpp2a/range-for12.C
> gcc/testsuite/g++.dg/cpp2a/range-for12.C
> index e69de29bb2d..09558341bb9 100644
> --- gcc/testsuite/g++.dg/cpp2a/range-for12.C
> +++ gcc/testsuite/g++.dg/cpp2a/range-for12.C
> @@ -0,0 +1,33 @@
> +// PR c++/87152
> +// { dg-do compile }
> +// { dg-options "-std=c++2a" }
> +
> +static const int a[] = { 1, 2, 3, 4, 5 };
> +extern void foo (int);
> +extern void bar (int, int);
> +
> +constexpr int
> +baz ()
> +{
> + return 6;
> +}
> +
> +template<typename T>
> +void
> +fn1 (T i)
> +{
> + for ((i += 2); auto x : a)
> + foo (i);
> +
> + for (auto j = 0, k = 0; auto x : a)
> + bar (j + k, x);
> +
> + for (constexpr int j = baz (); auto x : a)
> + bar (x, j);
> +}
> +
> +void
> +do_fn1 ()
> +{
> + fn1<int>(10);
> +}
> diff --git gcc/testsuite/g++.dg/cpp2a/range-for13.C
> gcc/testsuite/g++.dg/cpp2a/range-for13.C
> index e69de29bb2d..fb1ff285529 100644
> --- gcc/testsuite/g++.dg/cpp2a/range-for13.C
> +++ gcc/testsuite/g++.dg/cpp2a/range-for13.C
> @@ -0,0 +1,33 @@
> +// PR c++/87152
> +// { dg-do run }
> +// { dg-options "-std=c++2a" }
> +
> +template<typename T>
> +void foo ()
> +{
> + int a[] = { 1, 2, 3, 4, 5 };
> +
> + for (T i = 1; auto x : a)
> + if (i++ != x)
> + __builtin_abort ();
> +
> + T i;
> + for (i = 1; auto x : a)
> + if (i++ != x)
> + __builtin_abort ();
> +
> + i = 0;
> + for (i++; auto x : a)
> + if (i != 1)
> + __builtin_abort ();
> +
> + for (T s[] = { 1, 1, 1 }; auto x : s)
> + if (x != 1)
> + __builtin_abort ();
> +}
> +
> +int
> +main ()
> +{
> + foo<int>();
> +}
> diff --git gcc/testsuite/g++.dg/cpp2a/range-for14.C
> gcc/testsuite/g++.dg/cpp2a/range-for14.C
> index e69de29bb2d..94ff3c19153 100644
> --- gcc/testsuite/g++.dg/cpp2a/range-for14.C
> +++ gcc/testsuite/g++.dg/cpp2a/range-for14.C
> @@ -0,0 +1,24 @@
> +// PR c++/87152
> +// { dg-do run }
> +// { dg-options "-std=c++2a" }
> +
> +template<typename T>
> +void
> +fn ()
> +{
> + T a[] = { 1, 2, 3, 4, 5 };
> +
> + for (T i = []{ return 3; }(); auto x : a)
> + if (i != 3)
> + __builtin_abort ();
> +
> + for (T i = ({ 3; }); auto x : a)
> + if (i != 3)
> + __builtin_abort ();
> +}
> +
> +int
> +main ()
> +{
> + fn<int>();
> +}
> diff --git gcc/testsuite/g++.dg/cpp2a/range-for15.C
> gcc/testsuite/g++.dg/cpp2a/range-for15.C
> index e69de29bb2d..532b7689459 100644
> --- gcc/testsuite/g++.dg/cpp2a/range-for15.C
> +++ gcc/testsuite/g++.dg/cpp2a/range-for15.C
> @@ -0,0 +1,43 @@
> +// PR c++/87152
> +// { dg-do run }
> +// { dg-options "-std=c++2a" }
> +
> +struct A { int i; long long j; } a[64];
> +
> +template<typename T>
> +void foo ()
> +{
> + for (T i = 0; auto &x : a)
> + {
> + x.i = i;
> + x.j = 2 * i++;
> + }
> + for (auto & [ x, y ] : a)
> + {
> + x += 2;
> + y += 3;
> + }
> + for (T i = 0; const auto [ u, v ] : a)
> + {
> + if (u != i + 2 || v != 2 * i++ + 3)
> + __builtin_abort ();
> + }
> + for (T i = 0; auto [ x, y ] : a)
> + {
> + x += 4;
> + y += 5;
> + if (x != i + 6 || y != 2 * i++ + 8)
> + __builtin_abort ();
> + }
> + for (T i = 0; const auto x : a)
> + {
> + if (x.i != i + 2 || x.j != 2 * i++ + 3)
> + __builtin_abort ();
> + }
> +}
> +
> +int
> +main ()
> +{
> + foo<int>();
> +}
> diff --git gcc/testsuite/g++.dg/cpp2a/range-for16.C
> gcc/testsuite/g++.dg/cpp2a/range-for16.C
> index e69de29bb2d..cbfd9873387 100644
> --- gcc/testsuite/g++.dg/cpp2a/range-for16.C
> +++ gcc/testsuite/g++.dg/cpp2a/range-for16.C
> @@ -0,0 +1,36 @@
> +// PR c++/87152
> +// { dg-do run }
> +// { dg-options "-std=c++2a" }
> +
> +struct A { int i, j; };
> +
> +template<typename T>
> +void foo ()
> +{
> + A a = { .i = 2, .j = 3 };
> + T arr[] = { 1, 1, 1 };
> +
> + for (auto & [ x, y ] = a; auto z : arr)
> + if (x + z != 3 || y + z != 4)
> + __builtin_abort ();
> +
> + for (T d = 1; auto &z : arr)
> + z += d;
> +
> + for (const auto [ x, y ] = a; auto z : arr)
> + if (x + z != 4 || y + z != 5)
> + __builtin_abort ();
> +
> + for (T d = 1; auto &z : arr)
> + z += d;
> +
> + for (auto [ x, y ] = a; auto z : arr)
> + if (x + z != 5 || y + z != 6)
> + __builtin_abort ();
> +}
> +
> +int
> +main ()
> +{
> + foo<int>();
> +}
> diff --git gcc/testsuite/g++.dg/cpp2a/range-for17.C
> gcc/testsuite/g++.dg/cpp2a/range-for17.C
> index e69de29bb2d..2e8734b795d 100644
> --- gcc/testsuite/g++.dg/cpp2a/range-for17.C
> +++ gcc/testsuite/g++.dg/cpp2a/range-for17.C
> @@ -0,0 +1,30 @@
> +// PR c++/87152
> +// { dg-do run }
> +// { dg-options "-std=c++2a" }
> +
> +struct A { int i; long long j; } a[64];
> +
> +template<typename>
> +void foo ()
> +{
> + A b = { 1, 2 };
> + for (auto & [ u, v ] : a)
> + {
> + u = 2;
> + v = 3;
> + }
> +
> + for (auto [x, y] = b; auto [ u, v ] : a)
> + if (y + u != x + v)
> + __builtin_abort ();
> +
> + for (auto [x, y] = b; auto & [ u, v ] : a)
> + if (y + u != x + v)
> + __builtin_abort ();
> +}
> +
> +int
> +main ()
> +{
> + foo<int>();
> +}
> diff --git gcc/testsuite/g++.dg/cpp2a/range-for18.C
> gcc/testsuite/g++.dg/cpp2a/range-for18.C
> index e69de29bb2d..e38b41cc30d 100644
> --- gcc/testsuite/g++.dg/cpp2a/range-for18.C
> +++ gcc/testsuite/g++.dg/cpp2a/range-for18.C
> @@ -0,0 +1,16 @@
> +// PR c++/87152
> +// { dg-do compile }
> +// { dg-options "-std=c++2a" }
> +
> +template<int> void foo()
> +{
> + int a[] = { 1, 1, 1 };
> + for (int i = 0; auto x : a);
> + int i;
> +}
> +
> +void
> +bar ()
> +{
> + foo<0>();
> +}
> diff --git gcc/testsuite/g++.dg/parse/error61.C
> gcc/testsuite/g++.dg/parse/error61.C
> index 199e1aa721c..272626bd734 100644
> --- gcc/testsuite/g++.dg/parse/error61.C
> +++ gcc/testsuite/g++.dg/parse/error61.C
> @@ -4,8 +4,8 @@
> template<int> void foo()
> {
> int x[8];
> - for (int& i, j : x) // { dg-error "multiple" }
> - i = 0; // { dg-error "local variable" }
> + for (int& i, j : x) // { dg-error "multiple|reference" }
> + i = 0;
> }
>
> void bar()
