| Message ID | 51AE8778.8080901@oracle.com |
|---|---|
| State | New |
| Headers | show |
On 06/04/2013 08:34 PM, Paolo Carlini wrote: > /* After seeing a decl-specifier-seq, if the next token is not a > - "(", there is no possibility that the code is a valid > + "(" nor '...', there is no possibility that the code is a valid > expression. Therefore, if parsing tentatively, we commit at > this point. */ > > clearly in the case at issue of decl-specifier-seq followed by ellipsis > we may be parsing a perfectly valid declaration. Yes, but not an expression, which is why we are committing there: we know it *has* to be a declaration. The non-variadic case should hit that commit, so your change doesn't seem to make the variadic case more consistent. Jason
Hi, On 06/05/2013 02:53 PM, Jason Merrill wrote: > On 06/04/2013 08:34 PM, Paolo Carlini wrote: >> /* After seeing a decl-specifier-seq, if the next token is not a >> - "(", there is no possibility that the code is a valid >> + "(" nor '...', there is no possibility that the code is a valid >> expression. Therefore, if parsing tentatively, we commit at >> this point. */ >> >> clearly in the case at issue of decl-specifier-seq followed by ellipsis >> we may be parsing a perfectly valid declaration. > > Yes, but not an expression, which is why we are committing there: we > know it *has* to be a declaration. I see, I didn't notice the reference to *expressions* in the comment. > The non-variadic case should hit that commit, so your change doesn't > seem to make the variadic case more consistent. But the non-variadic case does *not* hit the commit, because is handled above: /* If the next token is a `)', `,', `=', `>', or `...', then there is no declarator. However, when variadic templates are enabled, there may be a declarator following `...'. */ if (token->type == CPP_CLOSE_PAREN || token->type == CPP_COMMA || token->type == CPP_EQ || token->type == CPP_GREATER) { declarator = NULL; if (parenthesized_p) *parenthesized_p = false; } Thus it seems to me that the variadic case should also somehow not-hit the commit, do you agree? Paolo.
On 06/05/2013 09:07 AM, Paolo Carlini wrote: > Hi, > > On 06/05/2013 02:53 PM, Jason Merrill wrote: >> On 06/04/2013 08:34 PM, Paolo Carlini wrote: >>> /* After seeing a decl-specifier-seq, if the next token is not a >>> - "(", there is no possibility that the code is a valid >>> + "(" nor '...', there is no possibility that the code is a valid >>> expression. Therefore, if parsing tentatively, we commit at >>> this point. */ >>> >>> clearly in the case at issue of decl-specifier-seq followed by ellipsis >>> we may be parsing a perfectly valid declaration. >> >> Yes, but not an expression, which is why we are committing there: we >> know it *has* to be a declaration. > I see, I didn't notice the reference to *expressions* in the comment. >> The non-variadic case should hit that commit, so your change doesn't >> seem to make the variadic case more consistent. > But the non-variadic case does *not* hit the commit, because is handled > above: > > /* If the next token is a `)', `,', `=', `>', or `...', then there > is no declarator. However, when variadic templates are enabled, > there may be a declarator following `...'. */ > if (token->type == CPP_CLOSE_PAREN > || token->type == CPP_COMMA > || token->type == CPP_EQ > || token->type == CPP_GREATER) > { > declarator = NULL; > if (parenthesized_p) > *parenthesized_p = false; > } > > Thus it seems to me that the variadic case should also somehow not-hit > the commit, do you agree? Well, the comment here is correct: (Args... args) would also be well-formed. But in the testcase there isn't a declarator, so the comment following the above, "Otherwise, there should be a declarator", is wrong. I guess the above should also check for ... followed by one of those tokens. In any case, the commit doesn't seem like the problem. I'm not sure why it's in the else block rather than before the if. Jason
.. true that my first try didn't fix this non-variadic variant, because
we have a declarator and again we hit the commit:
struct foo
{
template <typename Ret, typename Args>
operator decltype(static_cast<Ret(*)(Args args)>(0)) () const;
};
Paolo.
On 06/05/2013 09:19 AM, Jason Merrill wrote:
> In any case, the commit doesn't seem like the problem.
Oh, I see, of course it is. The problem is that it commits to all
levels, so that if we happen to be in a nested tentative parse we can't
commit to the inner one without committing to the outer one, which might
not be appropriate. This seems like a basic flaw in our tentative
parsing mechanism.
What happens if we disable that commit entirely?
Jason
Index: cp/parser.c =================================================================== --- cp/parser.c (revision 199675) +++ cp/parser.c (working copy) @@ -17887,7 +17887,7 @@ cp_parser_parameter_declaration (cp_parser *parser parser->default_arg_ok_p = false; /* After seeing a decl-specifier-seq, if the next token is not a - "(", there is no possibility that the code is a valid + "(" nor '...', there is no possibility that the code is a valid expression. Therefore, if parsing tentatively, we commit at this point. */ if (!parser->in_template_argument_list_p @@ -17901,7 +17901,8 @@ cp_parser_parameter_declaration (cp_parser *parser && !parser->in_type_id_in_expr_p && cp_parser_uncommitted_to_tentative_parse_p (parser) && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_BRACE) - && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_PAREN)) + && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_PAREN) + && cp_lexer_next_token_is_not (parser->lexer, CPP_ELLIPSIS)) cp_parser_commit_to_tentative_parse (parser); /* Parse the declarator. */ declarator_token_start = token; Index: testsuite/g++.dg/cpp0x/variadic144.C =================================================================== --- testsuite/g++.dg/cpp0x/variadic144.C (revision 0) +++ testsuite/g++.dg/cpp0x/variadic144.C (working copy) @@ -0,0 +1,8 @@ +// PR c++/51908 +// { dg-do compile { target c++11 } } + +struct foo +{ + template <typename Ret, typename... Args> + operator decltype(static_cast<Ret (*)(Args...)>(nullptr)) () const; +};
Hi, this ICE on valid happens in the cp_parser_abort_tentative_parse called at the end of cp_parser_decltype_expr. I started seriously looking into it when I noticed that a variant of the testcase not using variadic templates is fine, thus I concentrated on cp_parser_parameter_declaration and noticed an inconsistency in the following comment and the code implementing it: /* After seeing a decl-specifier-seq, if the next token is not a - "(", there is no possibility that the code is a valid + "(" nor '...', there is no possibility that the code is a valid expression. Therefore, if parsing tentatively, we commit at this point. */ clearly in the case at issue of decl-specifier-seq followed by ellipsis we may be parsing a perfectly valid declaration. Tested x86_64-linux, as usual. Thanks, Paolo. //////////////////////// /cp 2013-06-05 Paolo Carlini <paolo.carlini@oracle.com> PR c++/51908 * parser.c (cp_parser_parameter_declaration): Do not commit to tentative parse after a decl-specifier-seq followed by ellipsis. /testsuite 2013-06-05 Paolo Carlini <paolo.carlini@oracle.com> PR c++/51908 * g++.dg/cpp0x/variadic144.C: New.