Re: [PATCH v3] c++: fix parsing with auto(x) [PR112410]
On 11/15/23 17:24, Marek Polacek wrote: On Tue, Nov 14, 2023 at 05:27:03PM -0500, Jason Merrill wrote: On 11/14/23 10:58, Marek Polacek wrote: On Mon, Nov 13, 2023 at 09:26:41PM -0500, Jason Merrill wrote: On 11/10/23 20:13, Marek Polacek wrote: On Thu, Nov 09, 2023 at 07:07:03PM -0500, Jason Merrill wrote: On 11/9/23 14:58, Marek Polacek wrote: Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- Here we are wrongly parsing int y(auto(42)); which uses the C++23 cast-to-prvalue feature, and initializes y to 42. However, we were treating the auto as an implicit template parameter. Fixing the auto{42} case is easy, but when auto is followed by a (, I found the fix to be much more involved. For instance, we cannot use cp_parser_expression, because that can give hard errors. It's also necessary to disambiguate 'auto(i)' as 'auto i', not a cast. auto(), auto(int), auto(f)(int), auto(*), auto(i[]), auto(...), etc. are all function declarations. We have to look at more than one token to decide. Yeah, this is a most vexing parse problem. The code is synthesizing template parameters before we've resolved whether the auto is a decl-specifier or not. In this fix, I'm (ab)using cp_parser_declarator, with member_p=false so that it doesn't commit. But it handles even more complicated cases as int fn (auto (*const **)(int) -> char); But it doesn't seem to handle the extremely vexing struct A { A(int,int); }; int main() { int a; A b(auto(a), 42); } Argh. This test should indeed be accepted and is currently rejected, but it's a different problem: 'b' is at block scope and you can't have a template there. But when I put it into a namespace scope, it shows that my patch doesn't work correctly. I've added auto-fncast14.C for the latter and opened c++/112482 for the block-scope problem. I think we need to stop synthesizing immediately when we see RID_AUTO, and instead go back after we successfully parse a declaration and synthesize for any autos we saw along the way. :/ That seems very complicated :(. I had a different idea though; how about the following patch? The idea is that if we see that parsing the parameter-declaration-list didn't work, we undo what synthesize_ did, and let cp_parser_initializer parse "(auto(42))", which should succeed. I checked that after cp_finish_decl y is initialized to 42. Nice, that's much simpler. Do you also still need the changes to cp_parser_simple_type_specifier? I do, otherwise we parse int f (auto{42}); just as if it had been int f (auto); because the {42} is consumed in the cp_parser_simple_type_specifier/RID_AUTO loop. :/ It isn't consumed there, that loop is just scanning forward to see if there's a ->. The { is still the next token when we expect it to be a closing ) in cp_parser_direct_declarator: Ok, the tokens are rolled back after consuming so we can... /* Parse the parameter-declaration-clause. */ params = cp_parser_parameter_declaration_clause (parser, flags); const location_t parens_end = cp_lexer_peek_token (parser->lexer)->location; /* Consume the `)'. */ parens.require_close (parser); Maybe we want to abort_fully_implicit_template here rather than in cp_parser_parameter_declaration_clause? ...do this instead. Much better. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? OK, thanks. -- >8 -- Here we are wrongly parsing int y(auto(42)); which uses the C++23 cast-to-prvalue feature, and initializes y to 42. However, we were treating the auto as an implicit template parameter. Fixing the auto{42} case is easy, but when auto is followed by a (, I found the fix to be much more involved. For instance, we cannot use cp_parser_expression, because that can give hard errors. It's also necessary to disambiguate 'auto(i)' as 'auto i', not a cast. auto(), auto(int), auto(f)(int), auto(*), auto(i[]), auto(...), etc. are all function declarations. This patch rectifies that by undoing the implicit function template modification. In the test above, we should notice that the parameter list is ill-formed, and since we've synthesized an implicit template parameter, we undo it by calling abort_fully_implicit_template. Then, we'll parse the "(auto(42))" as an initializer. PR c++/112410 gcc/cp/ChangeLog: * parser.cc (cp_parser_direct_declarator): Maybe call abort_fully_implicit_template if it turned out the parameter list was ill-formed. gcc/testsuite/ChangeLog: * g++.dg/cpp23/auto-fncast13.C: New test. * g++.dg/cpp23/auto-fncast14.C: New test. --- gcc/cp/parser.cc | 13 + gcc/testsuite/g++.dg/cpp23/auto-fncast13.C | 61 ++ gcc/testsuite/g++.dg/cpp23/auto-fncast14.C | 9 3 files changed, 83 insertions(+) create mode 100644
[PATCH v3] c++: fix parsing with auto(x) [PR112410]
On Tue, Nov 14, 2023 at 05:27:03PM -0500, Jason Merrill wrote: > On 11/14/23 10:58, Marek Polacek wrote: > > On Mon, Nov 13, 2023 at 09:26:41PM -0500, Jason Merrill wrote: > > > On 11/10/23 20:13, Marek Polacek wrote: > > > > On Thu, Nov 09, 2023 at 07:07:03PM -0500, Jason Merrill wrote: > > > > > On 11/9/23 14:58, Marek Polacek wrote: > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > > > > > > > > > -- >8 -- > > > > > > Here we are wrongly parsing > > > > > > > > > > > > int y(auto(42)); > > > > > > > > > > > > which uses the C++23 cast-to-prvalue feature, and initializes y to > > > > > > 42. > > > > > > However, we were treating the auto as an implicit template > > > > > > parameter. > > > > > > > > > > > > Fixing the auto{42} case is easy, but when auto is followed by a (, > > > > > > I found the fix to be much more involved. For instance, we cannot > > > > > > use cp_parser_expression, because that can give hard errors. It's > > > > > > also necessary to disambiguate 'auto(i)' as 'auto i', not a cast. > > > > > > auto(), auto(int), auto(f)(int), auto(*), auto(i[]), auto(...), etc. > > > > > > are all function declarations. We have to look at more than one > > > > > > token to decide. > > > > > > > > > > Yeah, this is a most vexing parse problem. The code is synthesizing > > > > > template parameters before we've resolved whether the auto is a > > > > > decl-specifier or not. > > > > > > > > > > > In this fix, I'm (ab)using cp_parser_declarator, with member_p=false > > > > > > so that it doesn't commit. But it handles even more complicated > > > > > > cases as > > > > > > > > > > > > int fn (auto (*const **)(int) -> char); > > > > > > > > > > But it doesn't seem to handle the extremely vexing > > > > > > > > > > struct A { > > > > > A(int,int); > > > > > }; > > > > > > > > > > int main() > > > > > { > > > > > int a; > > > > > A b(auto(a), 42); > > > > > } > > > > > > > > Argh. This test should indeed be accepted and is currently rejected, > > > > but it's a different problem: 'b' is at block scope and you can't > > > > have a template there. But when I put it into a namespace scope, > > > > it shows that my patch doesn't work correctly. I've added > > > > auto-fncast14.C > > > > for the latter and opened c++/112482 for the block-scope problem. > > > > > I think we need to stop synthesizing immediately when we see > > > > > RID_AUTO, and > > > > > instead go back after we successfully parse a declaration and > > > > > synthesize for > > > > > any autos we saw along the way. :/ > > > > > > > > That seems very complicated :(. I had a different idea though; how > > > > about the following patch? The idea is that if we see that parsing > > > > the parameter-declaration-list didn't work, we undo what synthesize_ > > > > did, and let cp_parser_initializer parse "(auto(42))", which should > > > > succeed. I checked that after cp_finish_decl y is initialized to 42. > > > > > > Nice, that's much simpler. Do you also still need the changes to > > > cp_parser_simple_type_specifier? > > > > I do, otherwise we parse > > > >int f (auto{42}); > > > > just as if it had been > > > >int f (auto); > > > > because the {42} is consumed in the cp_parser_simple_type_specifier/RID_AUTO > > loop. :/ > > It isn't consumed there, that loop is just scanning forward to see if > there's a ->. The { is still the next token when we expect it to be a > closing ) in cp_parser_direct_declarator: Ok, the tokens are rolled back after consuming so we can... > > /* Parse the parameter-declaration-clause. */ > > params > > = cp_parser_parameter_declaration_clause (parser, flags); > > const location_t parens_end > > = cp_lexer_peek_token (parser->lexer)->location; > > > > /* Consume the `)'. */ > > parens.require_close (parser); > > Maybe we want to abort_fully_implicit_template here rather than in > cp_parser_parameter_declaration_clause? ...do this instead. Much better. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- Here we are wrongly parsing int y(auto(42)); which uses the C++23 cast-to-prvalue feature, and initializes y to 42. However, we were treating the auto as an implicit template parameter. Fixing the auto{42} case is easy, but when auto is followed by a (, I found the fix to be much more involved. For instance, we cannot use cp_parser_expression, because that can give hard errors. It's also necessary to disambiguate 'auto(i)' as 'auto i', not a cast. auto(), auto(int), auto(f)(int), auto(*), auto(i[]), auto(...), etc. are all function declarations. This patch rectifies that by undoing the implicit function template modification. In the test above, we should notice that the parameter list is ill-formed, and since we've synthesized an implicit template parameter, we undo it by calling