Message ID | 87626yupft.fsf@redhat.com |
---|---|
State | New |
Headers | show |
I am friendly pinging the patch below ... Dodji Seketeli <dodji@redhat.com> a écrit: > Hello, > > Consider this invalid example given in the PR, where T is not defined: > > 1 template<typename> > 2 struct X { > 3 using type = T; > 4 }; > > g++ yields the confusing diagnostics: > > test.cc:3:10: error: expected nested-name-specifier before 'type' > using type = T; > ^ > test.cc:3:10: error: using-declaration for non-member at class scope > test.cc:3:15: error: expected ';' before '=' token > using type = T; > ^ > test.cc:3:15: error: expected unqualified-id before '=' token > > I think this is because in cp_parser_member_declaration we tentatively > parse an alias declaration; we then have a somewhat meaningful > diagnostic which alas is not emitted because we are parsing > tentatively. As the parsing didn't succeed (because the input is > invalid) we try to parse a using declaration, which fails as well; but > then the diagnostic emitted is the one for the failed attempt at > parsing a using declaration, not an alias declaration. Oops. > > The idea of this patch is to detect in advance that we want to parse > an alias declaration, parse it non-tentatively, and then if an error > arises, emit it. > > I also changed cp_parser_alias_declaration to get out directly when it > detects that the type-id is invalid, rather than going on nonetheless > and emitting more (irrelevant) error diagnostics. > > We are now getting the following output: > > test.cc:3:18: erreur: expected type-specifier before ‘T’ > using type = T; > ^ > test.cc:3:18: erreur: ‘T’ does not name a type > > I don't really like the "before 'T'" there, but I think we maybe could > revisit the format of what cp_parser_error emits in general, now that > we have caret diagnostics; We could maybe do away with the "before T" > altogether? > > In the mean time, it seems to me that this patch brings an improvement > over what we already have in trunk, and the issue above could be > addressed separately. > > Tested on x86_64-unknown-linux-gnu against trunk. > > gcc/cp/ > > * parser.c (cp_parser_expecting_alias_declaration_p): New static > function. > (cp_parser_block_declaration): Use it. > (cp_parser_member_declaration): Likewise. Don't parse the using > declaration tentatively. > (cp_parser_alias_declaration): Get out if the type-id is invalid. > > gcc/testsuite/ > > * g++.dg/cpp0x/alias-decl-23.C: New test. > --- > gcc/cp/parser.c | 38 +++++++++++++++++++++++------- > gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C | 7 ++++++ > 2 files changed, 36 insertions(+), 9 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > index e8c0378..cab2d09 100644 > --- a/gcc/cp/parser.c > +++ b/gcc/cp/parser.c > @@ -1937,6 +1937,8 @@ static bool cp_parser_using_declaration > (cp_parser *, bool); > static void cp_parser_using_directive > (cp_parser *); > +static bool cp_parser_expecting_alias_declaration_p > + (cp_parser*); > static tree cp_parser_alias_declaration > (cp_parser *); > static void cp_parser_asm_definition > @@ -10292,11 +10294,7 @@ cp_parser_block_declaration (cp_parser *parser, > cp_parser_using_directive (parser); > /* If the second token after 'using' is '=', then we have an > alias-declaration. */ > - else if (cxx_dialect >= cxx0x > - && token2->type == CPP_NAME > - && ((cp_lexer_peek_nth_token (parser->lexer, 3)->type == CPP_EQ) > - || (cp_lexer_peek_nth_token (parser->lexer, 3)->keyword > - == RID_ATTRIBUTE))) > + else if (cp_parser_expecting_alias_declaration_p (parser)) > cp_parser_alias_declaration (parser); > /* Otherwise, it's a using-declaration. */ > else > @@ -15079,6 +15077,24 @@ cp_parser_using_declaration (cp_parser* parser, > return true; > } > > +/* Return TRUE if the coming tokens reasonably denote the beginning of > + an alias declaration. */ > + > +static bool > +cp_parser_expecting_alias_declaration_p (cp_parser* parser) > +{ > + if (cxx_dialect < cxx0x) > + return false; > + cp_parser_parse_tentatively (parser); > + cp_parser_require_keyword (parser, RID_USING, RT_USING); > + cp_parser_identifier (parser); > + cp_parser_attributes_opt (parser); > + cp_parser_require (parser, CPP_EQ, RT_EQ); > + bool is_ok = !cp_parser_error_occurred (parser); > + cp_parser_abort_tentative_parse (parser); > + return is_ok; > +} > + > /* Parse an alias-declaration. > > alias-declaration: > @@ -15141,6 +15157,9 @@ cp_parser_alias_declaration (cp_parser* parser) > if (parser->num_template_parameter_lists) > parser->type_definition_forbidden_message = saved_message; > > + if (type == error_mark_node) > + return error_mark_node; > + > cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON); > > if (cp_parser_error_occurred (parser)) > @@ -18849,10 +18868,11 @@ cp_parser_member_declaration (cp_parser* parser) > else > { > tree decl; > - cp_parser_parse_tentatively (parser); > - decl = cp_parser_alias_declaration (parser); > - if (cp_parser_parse_definitely (parser)) > - finish_member_declaration (decl); > + if (cp_parser_expecting_alias_declaration_p (parser)) > + { > + decl = cp_parser_alias_declaration (parser); > + finish_member_declaration (decl); > + } > else > cp_parser_using_declaration (parser, > /*access_declaration_p=*/false); > diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C > new file mode 100644 > index 0000000..086b5e5 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C > @@ -0,0 +1,7 @@ > +// Origin: PR c++/54401 > +// { dg-do compile { target c++11 } } > + > +template<typename> > +struct X { > + using type = T; // { dg-error "expected type-specifier|does not name a type" } > +}; > -- > 1.7.11.4
Would it work to just do a cp_parser_commit_to_tentative_parse after we see the '='? Jason
On Fri, Nov 16, 2012 at 8:51 AM, Jason Merrill <jason@redhat.com> wrote: > Would it work to just do a cp_parser_commit_to_tentative_parse after we see > the '='? I like that solution better. -- Gaby
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index e8c0378..cab2d09 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -1937,6 +1937,8 @@ static bool cp_parser_using_declaration (cp_parser *, bool); static void cp_parser_using_directive (cp_parser *); +static bool cp_parser_expecting_alias_declaration_p + (cp_parser*); static tree cp_parser_alias_declaration (cp_parser *); static void cp_parser_asm_definition @@ -10292,11 +10294,7 @@ cp_parser_block_declaration (cp_parser *parser, cp_parser_using_directive (parser); /* If the second token after 'using' is '=', then we have an alias-declaration. */ - else if (cxx_dialect >= cxx0x - && token2->type == CPP_NAME - && ((cp_lexer_peek_nth_token (parser->lexer, 3)->type == CPP_EQ) - || (cp_lexer_peek_nth_token (parser->lexer, 3)->keyword - == RID_ATTRIBUTE))) + else if (cp_parser_expecting_alias_declaration_p (parser)) cp_parser_alias_declaration (parser); /* Otherwise, it's a using-declaration. */ else @@ -15079,6 +15077,24 @@ cp_parser_using_declaration (cp_parser* parser, return true; } +/* Return TRUE if the coming tokens reasonably denote the beginning of + an alias declaration. */ + +static bool +cp_parser_expecting_alias_declaration_p (cp_parser* parser) +{ + if (cxx_dialect < cxx0x) + return false; + cp_parser_parse_tentatively (parser); + cp_parser_require_keyword (parser, RID_USING, RT_USING); + cp_parser_identifier (parser); + cp_parser_attributes_opt (parser); + cp_parser_require (parser, CPP_EQ, RT_EQ); + bool is_ok = !cp_parser_error_occurred (parser); + cp_parser_abort_tentative_parse (parser); + return is_ok; +} + /* Parse an alias-declaration. alias-declaration: @@ -15141,6 +15157,9 @@ cp_parser_alias_declaration (cp_parser* parser) if (parser->num_template_parameter_lists) parser->type_definition_forbidden_message = saved_message; + if (type == error_mark_node) + return error_mark_node; + cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON); if (cp_parser_error_occurred (parser)) @@ -18849,10 +18868,11 @@ cp_parser_member_declaration (cp_parser* parser) else { tree decl; - cp_parser_parse_tentatively (parser); - decl = cp_parser_alias_declaration (parser); - if (cp_parser_parse_definitely (parser)) - finish_member_declaration (decl); + if (cp_parser_expecting_alias_declaration_p (parser)) + { + decl = cp_parser_alias_declaration (parser); + finish_member_declaration (decl); + } else cp_parser_using_declaration (parser, /*access_declaration_p=*/false); diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C new file mode 100644 index 0000000..086b5e5 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C @@ -0,0 +1,7 @@ +// Origin: PR c++/54401 +// { dg-do compile { target c++11 } } + +template<typename> +struct X { + using type = T; // { dg-error "expected type-specifier|does not name a type" } +};