From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1734) id C4D333858016; Thu, 5 May 2022 21:34:50 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C4D333858016 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Marek Polacek To: gcc-cvs@gcc.gnu.org Subject: [gcc r13-146] c++: wrong error with MVP and pushdecl [PR64679] X-Act-Checkin: gcc X-Git-Author: Marek Polacek X-Git-Refname: refs/heads/trunk X-Git-Oldrev: a48be2e513d9ddb2fa8800a158f18502a0923aec X-Git-Newrev: ee9128189623ce43343ce326caa8d5eec2eccfec Message-Id: <20220505213450.C4D333858016@sourceware.org> Date: Thu, 5 May 2022 21:34:50 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 May 2022 21:34:50 -0000 https://gcc.gnu.org/g:ee9128189623ce43343ce326caa8d5eec2eccfec commit r13-146-gee9128189623ce43343ce326caa8d5eec2eccfec Author: Marek Polacek Date: Tue May 3 19:01:19 2022 -0400 c++: wrong error with MVP and pushdecl [PR64679] This patch fixes the second half of 64679. Here we issue a wrong "redefinition of 'int x'" for the following: struct Bar { Bar(int, int, int); }; int x = 1; Bar bar(int(x), int(x), int{x}); // #1 cp_parser_parameter_declaration_list does pushdecl every time it sees a named parameter, so the second "int(x)" causes the error. That's premature, since this turns out to be a constructor call after the third argument! If the first parameter is parenthesized, we can't push until we've established we're looking at a function declaration. Therefore this could be fixed by some kind of lookahead. I thought about introducing a lightweight variant of cp_parser_parameter_declaration_list that would not have any side effects and would return as soon as it figures out whether it's looking at a declaration or expression. Since that would require fairly nontrivial changes, I wanted something simpler. Something like delaying the pushdecl until we've reached the ')' following the parameter-declaration-clause. But we must push the parameters before processing a default argument, as in: Bar bar(int(a), int(b), int c = sizeof(a)); // valid Moreover, this code should still be accepted Bar f(int(i), decltype(i) j = 42); so this patch stashes parameters into a vector when parsing tentatively only when pushdecl-ing a parameter would result in a clash and an error about redefinition/redeclaration. The stashed parameters are pushed at the end of a parameter-declaration-clause if it's followed by a ')', so that we still diagnose redefining a parameter. PR c++/64679 gcc/cp/ChangeLog: * parser.cc (cp_parser_parameter_declaration_clause): Maintain a vector of parameters that haven't been pushed yet. Push them at the end of a valid parameter-declaration-clause. (cp_parser_parameter_declaration_list): Take a new auto_vec parameter. Do not pushdecl while parsing tentatively when pushdecl-ing a parameter would result in a hard error. (cp_parser_cache_defarg): Adjust the call to cp_parser_parameter_declaration_list. gcc/testsuite/ChangeLog: * g++.dg/parse/ambig11.C: New test. * g++.dg/parse/ambig12.C: New test. * g++.dg/parse/ambig13.C: New test. * g++.dg/parse/ambig14.C: New test. Diff: --- gcc/cp/parser.cc | 57 ++++++++++++++++++++++++++++++++---- gcc/testsuite/g++.dg/parse/ambig11.C | 39 ++++++++++++++++++++++++ gcc/testsuite/g++.dg/parse/ambig12.C | 12 ++++++++ gcc/testsuite/g++.dg/parse/ambig13.C | 32 ++++++++++++++++++++ gcc/testsuite/g++.dg/parse/ambig14.C | 21 +++++++++++++ 5 files changed, 156 insertions(+), 5 deletions(-) diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index b52cbe18b9a..3ebaa414a3d 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -2391,7 +2391,7 @@ static void cp_parser_type_specifier_seq static tree cp_parser_parameter_declaration_clause (cp_parser *, cp_parser_flags); static tree cp_parser_parameter_declaration_list - (cp_parser *, cp_parser_flags); + (cp_parser *, cp_parser_flags, auto_vec *); static cp_parameter_declarator *cp_parser_parameter_declaration (cp_parser *, cp_parser_flags, bool, bool *); static tree cp_parser_default_argument @@ -24517,8 +24517,12 @@ cp_parser_parameter_declaration_clause (cp_parser* parser, return explicit_void_list_node; } + /* A vector of parameters that haven't been pushed yet. */ + auto_vec pending_decls; + /* Parse the parameter-declaration-list. */ - parameters = cp_parser_parameter_declaration_list (parser, flags); + parameters = cp_parser_parameter_declaration_list (parser, flags, + &pending_decls); /* If a parse error occurred while parsing the parameter-declaration-list, then the entire parameter-declaration-clause is erroneous. */ @@ -24548,6 +24552,15 @@ cp_parser_parameter_declaration_clause (cp_parser* parser, else ellipsis_p = false; + /* A valid parameter-declaration-clause can only be followed by a ')'. + So it's time to push all the parameters we have seen now that we + know we have a valid declaration. Note that here we may not have + committed yet, nor should we. Pushing here will detect the error + of redefining a parameter. */ + if (cp_lexer_next_token_is (parser->lexer, CPP_CLOSE_PAREN)) + for (tree p : pending_decls) + pushdecl (p); + /* Finish the parameter list. */ if (!ellipsis_p) parameters = chainon (parameters, void_list_node); @@ -24562,13 +24575,16 @@ cp_parser_parameter_declaration_clause (cp_parser* parser, parameter-declaration-list , parameter-declaration The parser flags FLAGS is used to control type-specifier parsing. + PENDING_DECLS is a vector of parameters that haven't been pushed yet. Returns a representation of the parameter-declaration-list, as for cp_parser_parameter_declaration_clause. However, the `void_list_node' is never appended to the list. */ static tree -cp_parser_parameter_declaration_list (cp_parser* parser, cp_parser_flags flags) +cp_parser_parameter_declaration_list (cp_parser* parser, + cp_parser_flags flags, + auto_vec *pending_decls) { tree parameters = NULL_TREE; tree *tail = ¶meters; @@ -24625,7 +24641,37 @@ cp_parser_parameter_declaration_list (cp_parser* parser, cp_parser_flags flags) parameter->decl_specifiers.attributes, 0); if (DECL_NAME (decl)) - decl = pushdecl (decl); + { + /* We cannot always pushdecl while parsing tentatively because + it may have side effects and we can't be sure yet if we're + parsing a declaration, e.g.: + + S foo(int(x), int(x), int{x}); + + where it's not clear if we're dealing with a constructor call + or a function declaration until we've seen the last argument + which breaks it up. + It's safe to pushdecl so long as it doesn't result in a clash + with an already-pushed parameter. But we don't delay pushing + different parameters to handle + + S foo(int(i), decltype(i) j = 42); + + which is valid. */ + if (pending_decls + && cp_parser_uncommitted_to_tentative_parse_p (parser) + /* See if PARAMETERS already contains a parameter with the same + DECL_NAME as DECL. */ + && [parameters, decl] { + for (tree p = parameters; p; p = TREE_CHAIN (p)) + if (DECL_NAME (decl) == DECL_NAME (TREE_VALUE (p))) + return true; + return false; + }()) + pending_decls->safe_push (decl); + else + decl = pushdecl (decl); + } if (decl != error_mark_node) { @@ -34072,7 +34118,8 @@ cp_parser_cache_defarg (cp_parser *parser, bool nsdmi) cp_lexer_consume_token (parser->lexer); begin_scope (sk_function_parms, NULL_TREE); tree t = cp_parser_parameter_declaration_list - (parser, CP_PARSER_FLAGS_NONE); + (parser, CP_PARSER_FLAGS_NONE, + /*pending_decls*/nullptr); if (t == error_mark_node) error = true; pop_bindings_and_leave_scope (); diff --git a/gcc/testsuite/g++.dg/parse/ambig11.C b/gcc/testsuite/g++.dg/parse/ambig11.C new file mode 100644 index 00000000000..51a586f40b0 --- /dev/null +++ b/gcc/testsuite/g++.dg/parse/ambig11.C @@ -0,0 +1,39 @@ +// PR c++/64679 +// { dg-do run { target c++11 } } + +struct Bar { + int a, b, c; + Bar(int a, int b, int c) : a(a), b(b), c(c) { } +}; + +void +f () +{ + Bar fn1(int(a), int(b), int c = sizeof(a)); + Bar fn2(int(x), int(y), int(z)); // { dg-warning "function declaration" } + Bar fn3(int(x), int(y), int); + Bar fn4(int (*p)(int(x), int(y))); // { dg-warning "function declaration" } + Bar fn5(int (x), int (*p)(int(x), int(y)), int); +} + +int +main () +{ + int x = 1; + // This ain't a decl. + Bar v1(int(x), int(x), int{x}); + if (v1.a != 1 || v1.b != v1.a || v1.c != v1.a) + __builtin_abort (); + Bar v2(int(x), int(x), 1); + if (v2.a != 1 || v2.b != v2.a || v2.c != 1) + __builtin_abort (); + Bar v3(int(x), int(x), int(1)); + if (v3.a != 1 || v3.b != v3.a || v3.c != 1) + __builtin_abort (); + Bar v4(int(1), int(x), int{x}); + if (v4.a != 1 || v4.b != 1 || v4.c != 1) + __builtin_abort (); + Bar v5(int{x}, int(x), int{x}); + if (v5.a != 1 || v5.b != v5.a || v5.c != v5.a) + __builtin_abort (); +} diff --git a/gcc/testsuite/g++.dg/parse/ambig12.C b/gcc/testsuite/g++.dg/parse/ambig12.C new file mode 100644 index 00000000000..981f35f32a2 --- /dev/null +++ b/gcc/testsuite/g++.dg/parse/ambig12.C @@ -0,0 +1,12 @@ +// PR c++/64679 + +struct Bar { + Bar (int, int, int); +}; + +void +g () +{ + Bar e1(int(x), int(x), int); // { dg-error "redefinition" } + Bar e2(int (*p)(int(x), int(x)), int); // { dg-error "redefinition" } +} diff --git a/gcc/testsuite/g++.dg/parse/ambig13.C b/gcc/testsuite/g++.dg/parse/ambig13.C new file mode 100644 index 00000000000..5f6a3b934c2 --- /dev/null +++ b/gcc/testsuite/g++.dg/parse/ambig13.C @@ -0,0 +1,32 @@ +// PR c++/64679 +// { dg-do compile { target c++11 } } + +struct Bar { + Bar (int, int, int); +}; + +template +void +g () +{ + int x = 1; + Bar v1(T(x), T(x), T{x}); + Bar v2(T(x), T(x), T(1)); +} + +void +invoke (Bar (*p)) noexcept(noexcept(*p)) +{ +} + +auto +pmf (int (Bar::*p)) -> decltype(p) +{ + return nullptr; +} + +void +f () +{ + g(); +} diff --git a/gcc/testsuite/g++.dg/parse/ambig14.C b/gcc/testsuite/g++.dg/parse/ambig14.C new file mode 100644 index 00000000000..0305596a835 --- /dev/null +++ b/gcc/testsuite/g++.dg/parse/ambig14.C @@ -0,0 +1,21 @@ +// PR c++/64679 +// { dg-do compile { target c++11 } } + +struct F { + F(int, int); +}; + +void +g () +{ + int x = 42; + + F v1(int(x), decltype(x)(42)); + + F f1(int(i), decltype(i) j = 42); + F f2(int(i), decltype(i) j); + F f3(int(i), decltype(i)(j)); // { dg-warning "function declaration" } + F f4(int(i), decltype(i)(j) = 42); // { dg-warning "function declaration" } + F f5(int (i), bool b = true, decltype(i) j = 42); + F f6(int(i), decltype(x)(x)); // { dg-warning "function declaration" } +}