Message ID | ZcKYmBgorieFShB4@tucnak |
---|---|
State | New |
Headers | show |
Series | c++: Disallow this specifier except for parameter declarations [PR113788] | expand |
On Tue, Feb 06, 2024 at 09:37:44PM +0100, Jakub Jelinek wrote: > Hi! > > The deducing this patchset added parsing of this specifier to > cp_parser_decl_specifier_seq unconditionally, but in the C++ grammar > this[opt] only appears in the parameter-declaration non-terminal, so > rather than checking in all the callers of cp_parser_decl_specifier_seq > except for cp_parser_parameter_declaration that this specifier didn't > appear I think it is far easier and closer to what the standard says > to only parse this specifier when called from > cp_parser_parameter_declaration. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? FWIW, the patch looks good to me. > 2024-02-06 Jakub Jelinek <jakub@redhat.com> > > PR c++/113788 > * parser.cc (CP_PARSER_FLAGS_PARAMETER): New enumerator. > (cp_parser_decl_specifier_seq): Parse RID_THIS only if > CP_PARSER_FLAGS_PARAMETER is set in flags. > (cp_parser_parameter_declaration): Or in CP_PARSER_FLAGS_PARAMETER > when calling cp_parser_decl_specifier_seq. > > * g++.dg/parse/pr113788.C: New test. > > --- gcc/cp/parser.cc.jj 2024-01-17 10:34:45.337660930 +0100 > +++ gcc/cp/parser.cc 2024-02-06 18:31:35.587193903 +0100 > @@ -2088,7 +2088,9 @@ enum > /* When parsing of the noexcept-specifier should be delayed. */ > CP_PARSER_FLAGS_DELAY_NOEXCEPT = 0x40, > /* When parsing a consteval declarator. */ > - CP_PARSER_FLAGS_CONSTEVAL = 0x80 > + CP_PARSER_FLAGS_CONSTEVAL = 0x80, > + /* When parsing a parameter declaration. */ > + CP_PARSER_FLAGS_PARAMETER = 0x100 > }; > > /* This type is used for parameters and variables which hold > @@ -16342,7 +16344,7 @@ cp_parser_decl_specifier_seq (cp_parser* > /* Special case for "this" specifier, indicating a parm is an xobj parm. > The "this" specifier must be the first specifier in the declaration, > after any attributes. */ > - if (token->keyword == RID_THIS) > + if (token->keyword == RID_THIS && (flags & CP_PARSER_FLAGS_PARAMETER)) > { > cp_lexer_consume_token (parser->lexer); > if (token != first_specifier) > @@ -25607,7 +25609,7 @@ cp_parser_parameter_declaration (cp_pars > /* Parse the declaration-specifiers. */ > cp_token *decl_spec_token_start = cp_lexer_peek_token (parser->lexer); > cp_parser_decl_specifier_seq (parser, > - flags, > + flags | CP_PARSER_FLAGS_PARAMETER, > &decl_specifiers, > &declares_class_or_enum); > > --- gcc/testsuite/g++.dg/parse/pr113788.C.jj 2024-02-06 18:40:29.553791028 +0100 > +++ gcc/testsuite/g++.dg/parse/pr113788.C 2024-02-06 18:41:23.326045703 +0100 > @@ -0,0 +1,20 @@ > +// PR c++/113788 > +// { dg-do compile { target c++11 } } > + > +struct S { int a, b; }; > +struct U { > + void foo () { this int g = 1; } // { dg-error "expected ';' before 'int'" } > +}; > +this auto h = 1; // { dg-error "expected unqualified-id before 'this'" } > + > +int > +main () > +{ > + S s = { 1, 2 }; > + short t[3] = { 3, 4, 5 }; > + this auto &[a, b] = s; // { dg-error "invalid use of 'this' in non-member function" } > + this auto &[c, d, e] = t; // { dg-error "invalid use of 'this' in non-member function" } > + this int f = 1; // { dg-error "invalid use of 'this' in non-member function" } > + for (this auto &i : t) // { dg-error "invalid use of 'this' in non-member function" } > + ; // { dg-error "expected" } > +} // { dg-error "expected" } > > Jakub > Marek
On 2/6/24 15:45, Marek Polacek wrote: > On Tue, Feb 06, 2024 at 09:37:44PM +0100, Jakub Jelinek wrote: >> Hi! >> >> The deducing this patchset added parsing of this specifier to >> cp_parser_decl_specifier_seq unconditionally, but in the C++ grammar >> this[opt] only appears in the parameter-declaration non-terminal, so >> rather than checking in all the callers of cp_parser_decl_specifier_seq >> except for cp_parser_parameter_declaration that this specifier didn't >> appear I think it is far easier and closer to what the standard says >> to only parse this specifier when called from >> cp_parser_parameter_declaration. >> >> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > FWIW, the patch looks good to me. Agreed, OK. Jason
--- gcc/cp/parser.cc.jj 2024-01-17 10:34:45.337660930 +0100 +++ gcc/cp/parser.cc 2024-02-06 18:31:35.587193903 +0100 @@ -2088,7 +2088,9 @@ enum /* When parsing of the noexcept-specifier should be delayed. */ CP_PARSER_FLAGS_DELAY_NOEXCEPT = 0x40, /* When parsing a consteval declarator. */ - CP_PARSER_FLAGS_CONSTEVAL = 0x80 + CP_PARSER_FLAGS_CONSTEVAL = 0x80, + /* When parsing a parameter declaration. */ + CP_PARSER_FLAGS_PARAMETER = 0x100 }; /* This type is used for parameters and variables which hold @@ -16342,7 +16344,7 @@ cp_parser_decl_specifier_seq (cp_parser* /* Special case for "this" specifier, indicating a parm is an xobj parm. The "this" specifier must be the first specifier in the declaration, after any attributes. */ - if (token->keyword == RID_THIS) + if (token->keyword == RID_THIS && (flags & CP_PARSER_FLAGS_PARAMETER)) { cp_lexer_consume_token (parser->lexer); if (token != first_specifier) @@ -25607,7 +25609,7 @@ cp_parser_parameter_declaration (cp_pars /* Parse the declaration-specifiers. */ cp_token *decl_spec_token_start = cp_lexer_peek_token (parser->lexer); cp_parser_decl_specifier_seq (parser, - flags, + flags | CP_PARSER_FLAGS_PARAMETER, &decl_specifiers, &declares_class_or_enum); --- gcc/testsuite/g++.dg/parse/pr113788.C.jj 2024-02-06 18:40:29.553791028 +0100 +++ gcc/testsuite/g++.dg/parse/pr113788.C 2024-02-06 18:41:23.326045703 +0100 @@ -0,0 +1,20 @@ +// PR c++/113788 +// { dg-do compile { target c++11 } } + +struct S { int a, b; }; +struct U { + void foo () { this int g = 1; } // { dg-error "expected ';' before 'int'" } +}; +this auto h = 1; // { dg-error "expected unqualified-id before 'this'" } + +int +main () +{ + S s = { 1, 2 }; + short t[3] = { 3, 4, 5 }; + this auto &[a, b] = s; // { dg-error "invalid use of 'this' in non-member function" } + this auto &[c, d, e] = t; // { dg-error "invalid use of 'this' in non-member function" } + this int f = 1; // { dg-error "invalid use of 'this' in non-member function" } + for (this auto &i : t) // { dg-error "invalid use of 'this' in non-member function" } + ; // { dg-error "expected" } +} // { dg-error "expected" }