This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PR 35158 g++ does not compile valid C++ for loops with -fopenmp


2008/8/11 Jakub Jelinek <jakub@redhat.com>:
> On Thu, Aug 07, 2008 at 10:18:08AM +0200, Manuel López-Ibáñez wrote:
>> The following patch correctly diagnoses the use of parenthesized
>> initializers in a OpenMP for loop, which seems to be not allowed by
>> the standard.
>> Instead of a cascade of useless errors, it prints the reason to not
>> accept the code and what it expects.
>
> I'm not convinced it is safe to parse the expression definitely immediately
> (and diverging from cp_parse_condition where this originates),
> also you move unrelated code around (e.g. the real_decl = decl; assignment
> or cp_parser_declarator call formatting).

The code re-formatted or moved around was done to help readability. If
you don't like it I can revert those.

cp_parse_condition cannot parse definitely because it may be called
tentatively. Also this is not a condition, so I don't see why we have
to keep compatibility with it. Moreover, I see no way that at this
point we can have anything sensible apart from these two options:

* It is a declaration: so the first thing is a type specifier.
* It is not a declaration: so it must be an assignment.

We abuse parse tentatively far too much and that makes the parser
slow, complex, difficult to debug and when things fail, it results in
a cascade of unrelated errors because we have delayed reporting the
error until too late.

>
> I'd prefer:
>
> --- gcc/cp/parser.c.jj  2008-08-11 10:54:12.000000000 +0200
> +++ gcc/cp/parser.c     2008-08-11 16:11:21.000000000 +0200
> @@ -20872,7 +20872,9 @@ cp_parser_omp_for_loop (cp_parser *parse
>              attributes = cp_parser_attributes_opt (parser);
>              asm_specification = cp_parser_asm_specification_opt (parser);
>
> -             if (cp_lexer_next_token_is_not (parser->lexer, CPP_EQ))
> +             if (cp_lexer_next_token_is_not (parser->lexer, CPP_EQ)
> +                 && cp_lexer_next_token_is_not (parser->lexer,
> +                                                CPP_OPEN_PAREN))
>                cp_parser_require (parser, CPP_EQ, "%<=%>");
>              if (cp_parser_parse_definitely (parser))
>                {
> @@ -20883,8 +20885,15 @@ cp_parser_omp_for_loop (cp_parser *parse
>                                     /*prefix_attributes=*/NULL_TREE,
>                                     &pushed_scope);
>
> -                 if (CLASS_TYPE_P (TREE_TYPE (decl))
> -                     || type_dependent_expression_p (decl))
> +                 if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_PAREN))
> +                   {
> +                     error ("parenthesized initialization is not allowed "
> +                            "in OpenMP %<for%> loop");
> +                     cp_parser_skip_to_end_of_statement (parser);
> +                     init = error_mark_node;
> +                   }
> +                 else if (CLASS_TYPE_P (TREE_TYPE (decl))
> +                          || type_dependent_expression_p (decl))
>                    {
>                      bool is_direct_init, is_non_constant_init;
>
> instead, then the new error is the only one error reported, there are
> no further parse errors reported.

I can implement this and still parse definitvely as my patch does. I
introduced the extra (expected '=') on purpose to hint the user.
Again, parsing tentatively when it is not necessary makes the parser
really slow and difficult to follow.

Anyway, let me know what is your definitive answer and I would implement that.

Cheers,

Manuel.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]