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?

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

Reply via email to