On Tue, Jul 3, 2018 at 9:49 AM Jakub Jelinek <ja...@redhat.com> wrote:
>
> Hi!
>
> While working on OpenMP range-for support, I ran into the weird thing that
> the C++ FE accepts
> for (auto &i : a)
> if (i != *__for_begin || &i == __for_end || &__for_range[0] != &a[0])
> __builtin_abort ();
> outside of templates, but doesn't inside of templates.
> I think we shouldn't let people do this at any time, it will just lead to
> non-portable code, and when it works only outside of templates, it isn't a
> well defined extension.
>
> Now, we could just use create_tmp_var_name ("__for_range") instead of
> get_identifier ("__for_range") etc. and the symbols would be non-accessible
> to users (usually; if assembler doesn't support dots nor dollars in labels,
> it is still a symbol with just alphanumeric chars in it, but there is a hard
> to predict number suffix at least), or just NULL DECL_NAME.
> But my understanding is that the intent was that in the debugger one could
> use __for_range, __for_begin and __for_end to make it easier to debug
> range fors. In that case, the following patch solves it by using a name
> not accessible for the user when parsing the body (with space in it) and
> correcting the names when the var gets out of scope.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Can we make them DECL_ARTIFICIAL and/or make name-lookup never
lookup DECL_ARTIFICIAL vars instead?
Richard.
> 2018-07-02 Jakub Jelinek <ja...@redhat.com>
>
> PR c++/85515
> * parser.c (build_range_temp): Use "__for range" instead of
> "__for_range".
> (cp_convert_range_for): Use "__for begin" and "__for end" instead of
> "__for_begin" and "__for_end".
> * semantics.c (finish_for_stmt): Rename "__for {range,begin,end}"
> local symbols to "__for_{range,begin,end}".
>
> * g++.dg/pr85515-2.C: Add expected dg-error.
> * g++.dg/cpp0x/range-for36.C: New test.
>
> --- gcc/cp/parser.c.jj 2018-07-02 23:04:00.869370436 +0200
> +++ gcc/cp/parser.c 2018-07-02 23:08:44.572618486 +0200
> @@ -11953,7 +11953,7 @@ build_range_temp (tree range_expr)
>
> /* Create the __range variable. */
> range_temp = build_decl (input_location, VAR_DECL,
> - get_identifier ("__for_range"), range_type);
> + get_identifier ("__for range"), range_type);
> TREE_USED (range_temp) = 1;
> DECL_ARTIFICIAL (range_temp) = 1;
>
> @@ -12061,7 +12061,7 @@ cp_convert_range_for (tree statement, tr
>
> /* The new for initialization statement. */
> begin = build_decl (input_location, VAR_DECL,
> - get_identifier ("__for_begin"), iter_type);
> + get_identifier ("__for begin"), iter_type);
> TREE_USED (begin) = 1;
> DECL_ARTIFICIAL (begin) = 1;
> pushdecl (begin);
> @@ -12072,7 +12072,7 @@ cp_convert_range_for (tree statement, tr
> if (cxx_dialect >= cxx17)
> iter_type = cv_unqualified (TREE_TYPE (end_expr));
> end = build_decl (input_location, VAR_DECL,
> - get_identifier ("__for_end"), iter_type);
> + get_identifier ("__for end"), iter_type);
> TREE_USED (end) = 1;
> DECL_ARTIFICIAL (end) = 1;
> pushdecl (end);
> --- gcc/cp/semantics.c.jj 2018-06-25 14:51:23.096989196 +0200
> +++ gcc/cp/semantics.c 2018-07-02 23:23:40.784400542 +0200
> @@ -1060,7 +1060,31 @@ finish_for_stmt (tree for_stmt)
> : &FOR_SCOPE (for_stmt));
> tree scope = *scope_ptr;
> *scope_ptr = NULL;
> +
> + /* During parsing of the body, range for uses "__for {range,begin,end}"
> + decl names to make those unaccessible by code in the body.
> + Change it to ones with underscore instead of space, so that it can
> + be inspected in the debugger. */
> + tree range_for_decl[3] = { NULL_TREE, NULL_TREE, NULL_TREE };
> + for (int i = 0; i < 3; i++)
> + {
> + tree id
> + = get_identifier ("__for range\0__for begin\0__for end" + 12 * i);
> + if (IDENTIFIER_BINDING (id)
> + && IDENTIFIER_BINDING (id)->scope == current_binding_level)
> + {
> + range_for_decl[i] = IDENTIFIER_BINDING (id)->value;
> + gcc_assert (VAR_P (range_for_decl[i])
> + && DECL_ARTIFICIAL (range_for_decl[i]));
> + }
> + }
> +
> add_stmt (do_poplevel (scope));
> +
> + for (int i = 0; i < 3; i++)
> + if (range_for_decl[i])
> + DECL_NAME (range_for_decl[i])
> + = get_identifier ("__for_range\0__for_begin\0__for_end" + 12 * i);
> }
>
> /* Begin a range-for-statement. Returns a new RANGE_FOR_STMT.
> --- gcc/testsuite/g++.dg/pr85515-2.C.jj 2018-04-27 22:28:02.889462532 +0200
> +++ gcc/testsuite/g++.dg/pr85515-2.C 2018-07-02 23:33:50.638930364 +0200
> @@ -15,8 +15,7 @@ int test_2 ()
> int sum = 0;
> for (const auto v: arr) {
> sum += v;
> - // TODO: should we issue an error for the following assignment?
> - __for_begin = __for_end;
> + __for_begin = __for_end; // { dg-error "was not declared in this
> scope" }
> }
> return sum;
> }
> --- gcc/testsuite/g++.dg/cpp0x/range-for36.C.jj 2018-07-02 23:35:12.796000382
> +0200
> +++ gcc/testsuite/g++.dg/cpp0x/range-for36.C 2018-07-02 23:35:04.018992900
> +0200
> @@ -0,0 +1,32 @@
> +// PR c++/85515
> +// { dg-do compile { target c++11 } }
> +
> +int a[10];
> +
> +void
> +foo ()
> +{
> + for (auto &i : a)
> + if (i != *__for_begin // { dg-error "was not declared in
> this scope" }
> + || &i == __for_end // { dg-error "was not declared in
> this scope" }
> + || &__for_range[0] != &a[0]) // { dg-error "was not declared in
> this scope" }
> + __builtin_abort ();
> +}
> +
> +template <int N>
> +void
> +bar ()
> +{
> + for (auto &i : a)
> + if (i != *__for_begin // { dg-error "was not declared in
> this scope" }
> + || &i == __for_end // { dg-error "was not declared in
> this scope" }
> + || &__for_range[0] != &a[0]) // { dg-error "was not declared in
> this scope" }
> + __builtin_abort ();
> +}
> +
> +void
> +baz ()
> +{
> + foo ();
> + bar <0> ();
> +}
>
> Jakub
