C++ PATCH to implement P0614R1, Range-based for statements with initializer (take 2)

Jason Merrill jason@redhat.com
Wed May 23 16:42:00 GMT 2018


OK.

On Wed, May 23, 2018 at 10:45 AM, Marek Polacek <polacek@redhat.com> wrote:
> On Tue, May 22, 2018 at 09:46:10PM -0400, Jason Merrill wrote:
>> On Tue, May 22, 2018 at 7:25 PM, Marek Polacek <polacek@redhat.com> wrote:
>> > On Mon, May 21, 2018 at 09:51:44PM -0400, Jason Merrill wrote:
>> >> On Mon, May 21, 2018 at 7:34 PM, Marek Polacek <polacek@redhat.com> wrote:
>> >> > The previous version of this patch got confused by
>> >> >
>> >> >   for (int i = 0; n > 0 ? true : false; i++)
>> >> >     // ...
>> >> >
>> >> > because even though we see a ; followed by a :, it's not a range-based for with
>> >> > an initializer.  I find it very strange that this didn't show up during the
>> >> > regtest.
>> >> >
>> >> > To fix this, I had to uglify range_based_for_with_init_p to also check for a ?.
>> >> > Yuck.
>> >>
>> >> Perhaps cp_parser_skip_to_closing_parenthesis_1 should handle balanced
>> >> ?: like ()/[]/{}.
>> >
>> > Good point.  Clearly there's a difference between ?: and e.g. () because : can
>> > stand alone--e.g. in asm (: "whatever"), labels, goacc arrays like a[0:N], and
>> > so on.  The following seems to work well, and is certainly less ugly than the
>> > previous version.
>> >
>> > +       case CPP_QUERY:
>> > +         if (!brace_depth)
>> > +           ++condop_depth;
>> > +         break;
>> > +
>> > +       case CPP_COLON:
>> > +         if (!brace_depth && condop_depth > 0)
>> > +           condop_depth--;
>> > +         break;
>>
>> Since, as you say, colons can appear in more places, maybe we only
>> want to adjust condop_depth when all the other depths are 0, not just
>> brace_depth.
>
> Yeah, I meant to do it but apparently I didn't :(.  Fixed below.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2018-05-23  Marek Polacek  <polacek@redhat.com>
>
>         Implement P0614R1, Range-based for statements with initializer.
>         * parser.c (cp_parser_range_based_for_with_init_p): New.
>         (cp_parser_init_statement): Use it.  Parse the optional init-statement
>         for a range-based for loop.
>         (cp_parser_skip_to_closing_parenthesis_1): Handle balancing ?:.
>
>         * g++.dg/cpp2a/range-for1.C: New test.
>         * g++.dg/cpp2a/range-for2.C: New test.
>         * g++.dg/cpp2a/range-for3.C: New test.
>         * g++.dg/cpp2a/range-for4.C: New test.
>         * g++.dg/cpp2a/range-for5.C: New test.
>         * g++.dg/cpp2a/range-for6.C: New test.
>         * g++.dg/cpp2a/range-for7.C: New test.
>
> diff --git gcc/cp/parser.c gcc/cp/parser.c
> index 6f51f03f47c..d3e73488e84 100644
> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -3493,6 +3493,7 @@ cp_parser_skip_to_closing_parenthesis_1 (cp_parser *parser,
>    unsigned paren_depth = 0;
>    unsigned brace_depth = 0;
>    unsigned square_depth = 0;
> +  unsigned condop_depth = 0;
>
>    if (recovering && or_ttype == CPP_EOF
>        && cp_parser_uncommitted_to_tentative_parse_p (parser))
> @@ -3504,7 +3505,7 @@ cp_parser_skip_to_closing_parenthesis_1 (cp_parser *parser,
>
>        /* Have we found what we're looking for before the closing paren?  */
>        if (token->type == or_ttype && or_ttype != CPP_EOF
> -         && !brace_depth && !paren_depth && !square_depth)
> +         && !brace_depth && !paren_depth && !square_depth && !condop_depth)
>         return -1;
>
>        switch (token->type)
> @@ -3551,6 +3552,16 @@ cp_parser_skip_to_closing_parenthesis_1 (cp_parser *parser,
>             }
>           break;
>
> +       case CPP_QUERY:
> +         if (!brace_depth && !paren_depth && !square_depth)
> +           ++condop_depth;
> +         break;
> +
> +       case CPP_COLON:
> +         if (!brace_depth && !paren_depth && !square_depth && condop_depth > 0)
> +           condop_depth--;
> +         break;
> +
>         default:
>           break;
>         }
> @@ -11255,6 +11266,40 @@ cp_parser_statement_seq_opt (cp_parser* parser, tree in_statement_expr)
>      }
>  }
>
> +/* Return true if this is the C++20 version of range-based-for with
> +   init-statement.  */
> +
> +static bool
> +cp_parser_range_based_for_with_init_p (cp_parser *parser)
> +{
> +  bool r = false;
> +
> +  /* Save tokens so that we can put them back.  */
> +  cp_lexer_save_tokens (parser->lexer);
> +
> +  /* There has to be an unnested ; followed by an unnested :.  */
> +  if (cp_parser_skip_to_closing_parenthesis_1 (parser,
> +                                              /*recovering=*/false,
> +                                              CPP_SEMICOLON,
> +                                              /*consume_paren=*/false) != -1)
> +    goto out;
> +
> +  /* We found the semicolon, eat it now.  */
> +  cp_lexer_consume_token (parser->lexer);
> +
> +  /* Now look for ':' that is not nested in () or {}.  */
> +  r = (cp_parser_skip_to_closing_parenthesis_1 (parser,
> +                                               /*recovering=*/false,
> +                                               CPP_COLON,
> +                                               /*consume_paren=*/false) == -1);
> +
> +out:
> +  /* Roll back the tokens we skipped.  */
> +  cp_lexer_rollback_tokens (parser->lexer);
> +
> +  return r;
> +}
> +
>  /* Return true if we're looking at (init; cond), false otherwise.  */
>
>  static bool
> @@ -12299,7 +12344,7 @@ cp_parser_iteration_statement (cp_parser* parser, bool *if_p, bool ivdep,
>       simple-declaration  */
>
>  static bool
> -cp_parser_init_statement (cp_parser* parser, tree *decl)
> +cp_parser_init_statement (cp_parser *parser, tree *decl)
>  {
>    /* If the next token is a `;', then we have an empty
>       expression-statement.  Grammatically, this is also a
> @@ -12312,6 +12357,29 @@ cp_parser_init_statement (cp_parser* parser, tree *decl)
>        bool is_range_for = false;
>        bool saved_colon_corrects_to_scope_p = parser->colon_corrects_to_scope_p;
>
> +      /* Try to parse the init-statement.  */
> +      if (cp_parser_range_based_for_with_init_p (parser))
> +       {
> +         tree dummy;
> +         cp_parser_parse_tentatively (parser);
> +         /* Parse the declaration.  */
> +         cp_parser_simple_declaration (parser,
> +                                       /*function_definition_allowed_p=*/false,
> +                                       &dummy);
> +         cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
> +         if (!cp_parser_parse_definitely (parser))
> +           /* That didn't work, try to parse it as an expression-statement.  */
> +           cp_parser_expression_statement (parser, NULL_TREE);
> +
> +         if (cxx_dialect < cxx2a)
> +           {
> +             pedwarn (cp_lexer_peek_token (parser->lexer)->location, 0,
> +                      "range-based %<for%> loops with initializer only "
> +                      "available with -std=c++2a or -std=gnu++2a");
> +             *decl = error_mark_node;
> +           }
> +       }
> +
>        /* A colon is used in range-based for.  */
>        parser->colon_corrects_to_scope_p = false;
>
> @@ -12325,7 +12393,7 @@ cp_parser_init_statement (cp_parser* parser, tree *decl)
>        parser->colon_corrects_to_scope_p = saved_colon_corrects_to_scope_p;
>        if (cp_lexer_next_token_is (parser->lexer, CPP_COLON))
>         {
> -         /* It is a range-for, consume the ':' */
> +         /* It is a range-for, consume the ':'.  */
>           cp_lexer_consume_token (parser->lexer);
>           is_range_for = true;
>           if (cxx_dialect < cxx11)
> @@ -12337,9 +12405,9 @@ cp_parser_init_statement (cp_parser* parser, tree *decl)
>             }
>         }
>        else
> -         /* The ';' is not consumed yet because we told
> -            cp_parser_simple_declaration not to.  */
> -         cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
> +       /* The ';' is not consumed yet because we told
> +          cp_parser_simple_declaration not to.  */
> +       cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
>
>        if (cp_parser_parse_definitely (parser))
>         return is_range_for;
> diff --git gcc/testsuite/g++.dg/cpp2a/range-for1.C gcc/testsuite/g++.dg/cpp2a/range-for1.C
> index e69de29bb2d..3a5523585a1 100644
> --- gcc/testsuite/g++.dg/cpp2a/range-for1.C
> +++ gcc/testsuite/g++.dg/cpp2a/range-for1.C
> @@ -0,0 +1,16 @@
> +// P0614R1
> +// { dg-do compile { target c++11 } }
> +// { dg-options "" }
> +
> +void
> +fn1 ()
> +{
> +  int a[] = { 1, 2, 3, 4, 5 };
> +
> +  for (int i = 0; auto x : a) // { dg-warning "range-based .for. loops with initializer only available with" "" { target c++17_down } }
> +    ++i;
> +
> +  int i;
> +  for (i = 0; auto x : a) // { dg-warning "range-based .for. loops with initializer only available with" "" { target c++17_down } }
> +    ++i;
> +}
> diff --git gcc/testsuite/g++.dg/cpp2a/range-for2.C gcc/testsuite/g++.dg/cpp2a/range-for2.C
> index e69de29bb2d..acb16c57d1c 100644
> --- gcc/testsuite/g++.dg/cpp2a/range-for2.C
> +++ gcc/testsuite/g++.dg/cpp2a/range-for2.C
> @@ -0,0 +1,16 @@
> +// P0614R1
> +// { dg-do compile }
> +// { dg-options "-std=c++2a" }
> +
> +void
> +fn1 ()
> +{
> +  int a[] = { 1, 2, 3, 4, 5 };
> +
> +  for (int i = 0; auto x : a)
> +    ++i;
> +
> +  int i;
> +  for (i = 0; auto x : a)
> +    ++i;
> +}
> diff --git gcc/testsuite/g++.dg/cpp2a/range-for3.C gcc/testsuite/g++.dg/cpp2a/range-for3.C
> index e69de29bb2d..291e605b92f 100644
> --- gcc/testsuite/g++.dg/cpp2a/range-for3.C
> +++ gcc/testsuite/g++.dg/cpp2a/range-for3.C
> @@ -0,0 +1,26 @@
> +// P0614R1
> +// { 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;
> +}
> +
> +void
> +fn1 (int 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);
> +}
> diff --git gcc/testsuite/g++.dg/cpp2a/range-for4.C gcc/testsuite/g++.dg/cpp2a/range-for4.C
> index e69de29bb2d..6ba783f46cb 100644
> --- gcc/testsuite/g++.dg/cpp2a/range-for4.C
> +++ gcc/testsuite/g++.dg/cpp2a/range-for4.C
> @@ -0,0 +1,27 @@
> +// P0614R1
> +// { dg-do run }
> +// { dg-options "-std=c++2a" }
> +
> +int
> +main ()
> +{
> +  int a[] = { 1, 2, 3, 4, 5 };
> +
> +  for (int i = 1; auto x : a)
> +    if (i++ != x)
> +      __builtin_abort ();
> +
> +  int 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 (int s[] = { 1, 1, 1 }; auto x : s)
> +    if (x != 1)
> +      __builtin_abort ();
> +}
> diff --git gcc/testsuite/g++.dg/cpp2a/range-for5.C gcc/testsuite/g++.dg/cpp2a/range-for5.C
> index e69de29bb2d..62f1c2f04e1 100644
> --- gcc/testsuite/g++.dg/cpp2a/range-for5.C
> +++ gcc/testsuite/g++.dg/cpp2a/range-for5.C
> @@ -0,0 +1,46 @@
> +// P0614R1
> +// { dg-do compile }
> +// { dg-options "-std=c++2a" }
> +
> +void
> +fn1 ()
> +{
> +  int a[] = { 1, 2, 3, 4, 5 };
> +
> +  for (int i = 0; auto x : a)
> +    ++i;
> +
> +  i = 0; // { dg-error "not declared" }
> +
> +  for (int i = 0; auto x : a)
> +    {
> +      for (int j = 0; auto x : a)
> +       {
> +         for (int k = 0; auto x : a)
> +           k++;
> +         k++; // { dg-error "not declared" }
> +       }
> +      j++; // { dg-error "not declared" }
> +    }
> +}
> +
> +void
> +fn2 ()
> +{
> +  int a[] = { 1, 2, 3, 4, 5 };
> +  for (int i = 0; auto x : a)
> +    int i = 3; // { dg-error "redeclaration" }
> +}
> +void
> +fn3 ()
> +{
> +  int a[] = { 1, 2, 3, 4, 5 };
> +
> +  for (;:) // { dg-error "expected" }
> +    {
> +    }
> +
> +  for (;;:) // { dg-error "expected" }
> +    {
> +    }
> +}
> diff --git gcc/testsuite/g++.dg/cpp2a/range-for6.C gcc/testsuite/g++.dg/cpp2a/range-for6.C
> index e69de29bb2d..4cee60a839e 100644
> --- gcc/testsuite/g++.dg/cpp2a/range-for6.C
> +++ gcc/testsuite/g++.dg/cpp2a/range-for6.C
> @@ -0,0 +1,17 @@
> +// P0614R1
> +// { dg-do run }
> +// { dg-options "-std=c++2a" }
> +
> +int
> +main ()
> +{
> +  int a[] = { 1, 2, 3, 4, 5 };
> +
> +  for (int i = []{ return 3; }(); auto x : a)
> +    if (i != 3)
> +      __builtin_abort ();
> +
> +  for (int i = ({ 3; }); auto x : a)
> +    if (i != 3)
> +      __builtin_abort ();
> +}
> diff --git gcc/testsuite/g++.dg/cpp2a/range-for7.C gcc/testsuite/g++.dg/cpp2a/range-for7.C
> index e69de29bb2d..5a3a89c1c25 100644
> --- gcc/testsuite/g++.dg/cpp2a/range-for7.C
> +++ gcc/testsuite/g++.dg/cpp2a/range-for7.C
> @@ -0,0 +1,45 @@
> +// P0614R1
> +// { dg-do compile }
> +// { dg-options "-std=c++2a" }
> +
> +extern void bar (int);
> +
> +void
> +fn0 (int n)
> +{
> +  int a[] = { 1, 2, 3, 4, 5 };
> +
> +  /* Don't get confused by the colon here.  */
> +  for (int i = 0; n > 0 ? true : false; i++)
> +    bar (i);
> +
> +  for (int i = n ? 3 : 4; auto x : a)
> +    bar (x);
> +
> +  for (int i = n ? ({ a: 3; }) : 4; i < 10; i++)
> +    bar (i);
> +
> +  for (int i = n ? ({ L: 3; }) : 4; auto x : a)
> +    bar (x);
> +
> +  for (int i = n; auto x : a)
> +    bar (x);
> +
> +  for (int i = n ? n ? n : 3 : 3; auto x : a)
> +    bar (x);
> +
> +  for (int i = n ? n ? 3 : n ? 3 : 3 : 3; auto x : a)
> +    bar (x);
> +
> +  for (int i = [=]{ return n ? 1 : 2; }(); auto x : a)
> +    bar (x);
> +
> +  for (int i = [=]{ L2: if (!n) goto L2; else return 2; }(); auto x : a)
> +    bar (x);
> +
> +  for (auto x = n ? 1 : 2 : a) // { dg-error "initializer" }
> +    bar (x);
> +
> +  for (int i = 1; auto x = n ? 1 : 2 : a) // { dg-error "initializer" }
> +    bar (x);
> +}



More information about the Gcc-patches mailing list