[v3] Improve bad error message with stray semicolon in initializer (and related) [PR101232]
Checks
Commit Message
Hi, could someone review this patch?
This is built on top of the v2 from
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101232 the only
difference is fix for error59.C
I have tested it on x86_64 Ubuntu 22 machine.
Regards
Franciszek
---
Author: Franciszek Witt <franek.witt@gmail.com>
Date: Mon Aug 5 09:00:35 2024 +0200
c++: [PR101232]
PR c++/101232
gcc/cp/ChangeLog:
* parser.cc (cp_parser_postfix_expression): Commit to the
parse in case we know its either a cast or invalid syntax.
(cp_parser_braced_list): Add a heuristic to inform about
missing comma or operator.
gcc/testsuite/ChangeLog:
* g++.dg/parse/error59.C: Change the test so the new error
message is accepted.
* g++.dg/cpp0x/initlist-err1.C: New test.
* g++.dg/cpp0x/initlist-err2.C: New test.
* g++.dg/cpp0x/initlist-err3.C: New test.
---
Comments
On 05/08/24 09:32 +0200, Franciszek Witt wrote:
>Hi, could someone review this patch?
>
>This is built on top of the v2 from
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101232 the only
>difference is fix for error59.C
>
>I have tested it on x86_64 Ubuntu 22 machine.
>
>
>Regards
>Franciszek
>
>---
>
>Author: Franciszek Witt <franek.witt@gmail.com>
>Date: Mon Aug 5 09:00:35 2024 +0200
>
> c++: [PR101232]
The line above should be the first line of your Git commit message,
which should be in the form "component: Summary of change [PRnnnn]"
The Subject: of your email seems like it should be repeated here
(although something shorter would be even nicer, if possible).
https://cbea.ms/git-commit/
> PR c++/101232
>
> gcc/cp/ChangeLog:
>
> * parser.cc (cp_parser_postfix_expression): Commit to the
>parse in case we know its either a cast or invalid syntax.
> (cp_parser_braced_list): Add a heuristic to inform about
>missing comma or operator.
The ChangeLog part of the commit message should follow the rules in
the GNU Coding Standards:
https://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html
Specifically, they should be wrapped to fit in an 80 column terminal
window. I think wrapping around 76 columns is common. Please add line
breaks here, e.g.
* parser.cc (cp_parser_postfix_expression): Commit to the
parse in case we know its either a cast or invalid syntax.
(cp_parser_braced_list): Add a heuristic to inform about
missing comma or operator.
(but with a leading TAB, not spaces).
On 8/19/24 6:01 AM, Jonathan Wakely wrote:
> On 05/08/24 09:32 +0200, Franciszek Witt wrote:
>> Author: Franciszek Witt <franek.witt@gmail.com>
>> Date: Mon Aug 5 09:00:35 2024 +0200
>>
>> c++: [PR101232]
>
> The line above should be the first line of your Git commit message,
> which should be in the form "component: Summary of change [PRnnnn]"
> The Subject: of your email seems like it should be repeated here
> (although something shorter would be even nicer, if possible).
> https://cbea.ms/git-commit/
>
>
>> PR c++/101232
>>
>> gcc/cp/ChangeLog:
>>
>> * parser.cc (cp_parser_postfix_expression): Commit to the
>> parse in case we know its either a cast or invalid syntax.
>> (cp_parser_braced_list): Add a heuristic to inform about
>> missing comma or operator.
>
> The ChangeLog part of the commit message should follow the rules in
> the GNU Coding Standards:
> https://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html
>
> Specifically, they should be wrapped to fit in an 80 column terminal
> window. I think wrapping around 76 columns is common. Please add line
> breaks here, e.g.
>
> * parser.cc (cp_parser_postfix_expression): Commit to the
> parse in case we know its either a cast or invalid syntax.
> (cp_parser_braced_list): Add a heuristic to inform about
> missing comma or operator.
>
> (but with a leading TAB, not spaces).
Agreed. In addition, we need DCO certification or copyright assignment
(https://gcc.gnu.org/contribute.html#legal).
Also, the patch itself was corrupted by word wrap in your mail client,
so it doesn't apply, e.g.:
> @@ -7881,8 +7881,13 @@ cp_parser_postfix_expression (cp_parser
> *parser, bool address_p, bool cast_p,
https://www.kernel.org/doc/html/latest/process/email-clients.html has
some tips for avoiding word wrap, or you can just attach the patch
instead of pasting it inline.
> + /* This is just a heuristic. */
Two spaces after the .
> + inform (finish_loc,
> + "probably missing a comma or an operator before");
The string should line up after the ( on the previous line.
We might as well also put this inform under the
cp_parser_skip_to_closing_brace check; if there's no } at all, the
problem isn't a missing comma...
Jason
On 8/19/24 12:01 PM, Jason Merrill wrote:
> On 8/19/24 6:01 AM, Jonathan Wakely wrote:
>> On 05/08/24 09:32 +0200, Franciszek Witt wrote:
>>> Author: Franciszek Witt <franek.witt@gmail.com>
>>> Date: Mon Aug 5 09:00:35 2024 +0200
>>>
>>> c++: [PR101232]
>>
>> The line above should be the first line of your Git commit message,
>> which should be in the form "component: Summary of change [PRnnnn]"
>> The Subject: of your email seems like it should be repeated here
>> (although something shorter would be even nicer, if possible).
>> https://cbea.ms/git-commit/
>>
>>
>>> PR c++/101232
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> * parser.cc (cp_parser_postfix_expression): Commit to the
>>> parse in case we know its either a cast or invalid syntax.
>>> (cp_parser_braced_list): Add a heuristic to inform about
>>> missing comma or operator.
>>
>> The ChangeLog part of the commit message should follow the rules in
>> the GNU Coding Standards:
>> https://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html
>>
>> Specifically, they should be wrapped to fit in an 80 column terminal
>> window. I think wrapping around 76 columns is common. Please add line
>> breaks here, e.g.
>>
>> * parser.cc (cp_parser_postfix_expression): Commit to the
>> parse in case we know its either a cast or invalid syntax.
>> (cp_parser_braced_list): Add a heuristic to inform about
>> missing comma or operator.
>>
>> (but with a leading TAB, not spaces).
>
> Agreed. In addition, we need DCO certification or copyright assignment
> (https://gcc.gnu.org/contribute.html#legal).
>
> Also, the patch itself was corrupted by word wrap in your mail client,
> so it doesn't apply, e.g.:
>
>> @@ -7881,8 +7881,13 @@ cp_parser_postfix_expression (cp_parser
>> *parser, bool address_p, bool cast_p,
>
> https://www.kernel.org/doc/html/latest/process/email-clients.html has
> some tips for avoiding word wrap, or you can just attach the patch
> instead of pasting it inline.
>
>> + /* This is just a heuristic. */
>
> Two spaces after the .
>
>> + inform (finish_loc, + "probably missing a comma
>> or an operator before");
>
> The string should line up after the ( on the previous line.
>
> We might as well also put this inform under the
> cp_parser_skip_to_closing_brace check; if there's no } at all, the
> problem isn't a missing comma...
Also, error59 seems like an error-recovery regression:
> +++ b/gcc/testsuite/g++.dg/parse/error59.C
> @@ -3,4 +3,4 @@
> void foo()
> {
> (struct {}x){}; // { dg-error "" }
> -}
> +} // { dg-excess-errors "" }
as we now get more of an error cascade. Perhaps also check the return
value of the earlier require_open and don't skip_to_closing_brace if it
failed?
Jason
Thanks for the feedback. I am attatching fixed patch as a file.
Regards
Franciszek
On Mon, 19 Aug 2024, 18:32 Jason Merrill, <jason@redhat.com> wrote:
> On 8/19/24 12:01 PM, Jason Merrill wrote:
> > On 8/19/24 6:01 AM, Jonathan Wakely wrote:
> >> On 05/08/24 09:32 +0200, Franciszek Witt wrote:
> >>> Author: Franciszek Witt <franek.witt@gmail.com>
> >>> Date: Mon Aug 5 09:00:35 2024 +0200
> >>>
> >>> c++: [PR101232]
> >>
> >> The line above should be the first line of your Git commit message,
> >> which should be in the form "component: Summary of change [PRnnnn]"
> >> The Subject: of your email seems like it should be repeated here
> >> (although something shorter would be even nicer, if possible).
> >> https://cbea.ms/git-commit/
> >>
> >>
> >>> PR c++/101232
> >>>
> >>> gcc/cp/ChangeLog:
> >>>
> >>> * parser.cc (cp_parser_postfix_expression): Commit to the
> >>> parse in case we know its either a cast or invalid syntax.
> >>> (cp_parser_braced_list): Add a heuristic to inform about
> >>> missing comma or operator.
> >>
> >> The ChangeLog part of the commit message should follow the rules in
> >> the GNU Coding Standards:
> >> https://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html
> >>
> >> Specifically, they should be wrapped to fit in an 80 column terminal
> >> window. I think wrapping around 76 columns is common. Please add line
> >> breaks here, e.g.
> >>
> >> * parser.cc (cp_parser_postfix_expression): Commit to the
> >> parse in case we know its either a cast or invalid syntax.
> >> (cp_parser_braced_list): Add a heuristic to inform about
> >> missing comma or operator.
> >>
> >> (but with a leading TAB, not spaces).
> >
> > Agreed. In addition, we need DCO certification or copyright assignment
> > (https://gcc.gnu.org/contribute.html#legal).
> >
> > Also, the patch itself was corrupted by word wrap in your mail client,
> > so it doesn't apply, e.g.:
> >
> >> @@ -7881,8 +7881,13 @@ cp_parser_postfix_expression (cp_parser
> >> *parser, bool address_p, bool cast_p,
> >
> > https://www.kernel.org/doc/html/latest/process/email-clients.html has
> > some tips for avoiding word wrap, or you can just attach the patch
> > instead of pasting it inline.
> >
> >> + /* This is just a heuristic. */
> >
> > Two spaces after the .
> >
> >> + inform (finish_loc, + "probably missing a comma
> >> or an operator before");
> >
> > The string should line up after the ( on the previous line.
> >
> > We might as well also put this inform under the
> > cp_parser_skip_to_closing_brace check; if there's no } at all, the
> > problem isn't a missing comma...
>
> Also, error59 seems like an error-recovery regression:
>
> > +++ b/gcc/testsuite/g++.dg/parse/error59.C
> > @@ -3,4 +3,4 @@
> > void foo()
> > {
> > (struct {}x){}; // { dg-error "" }
> > -}
> > +} // { dg-excess-errors "" }
>
> as we now get more of an error cascade. Perhaps also check the return
> value of the earlier require_open and don't skip_to_closing_brace if it
> failed?
>
> Jason
>
>
On 8/20/24 9:11 AM, Franciszek Witt wrote:
> Thanks for the feedback. I am attatching fixed patch as a file.
Pushed, thanks.
> Regards
> Franciszek
>
> On Mon, 19 Aug 2024, 18:32 Jason Merrill, <jason@redhat.com
> <mailto:jason@redhat.com>> wrote:
>
> On 8/19/24 12:01 PM, Jason Merrill wrote:
> > On 8/19/24 6:01 AM, Jonathan Wakely wrote:
> >> On 05/08/24 09:32 +0200, Franciszek Witt wrote:
> >>> Author: Franciszek Witt <franek.witt@gmail.com
> <mailto:franek.witt@gmail.com>>
> >>> Date: Mon Aug 5 09:00:35 2024 +0200
> >>>
> >>> c++: [PR101232]
> >>
> >> The line above should be the first line of your Git commit message,
> >> which should be in the form "component: Summary of change [PRnnnn]"
> >> The Subject: of your email seems like it should be repeated here
> >> (although something shorter would be even nicer, if possible).
> >> https://cbea.ms/git-commit/ <https://cbea.ms/git-commit/>
> >>
> >>
> >>> PR c++/101232
> >>>
> >>> gcc/cp/ChangeLog:
> >>>
> >>> * parser.cc (cp_parser_postfix_expression): Commit
> to the
> >>> parse in case we know its either a cast or invalid syntax.
> >>> (cp_parser_braced_list): Add a heuristic to inform about
> >>> missing comma or operator.
> >>
> >> The ChangeLog part of the commit message should follow the rules in
> >> the GNU Coding Standards:
> >>
> https://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html <https://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html>
> >>
> >> Specifically, they should be wrapped to fit in an 80 column terminal
> >> window. I think wrapping around 76 columns is common. Please add
> line
> >> breaks here, e.g.
> >>
> >> * parser.cc (cp_parser_postfix_expression): Commit to the
> >> parse in case we know its either a cast or invalid syntax.
> >> (cp_parser_braced_list): Add a heuristic to inform about
> >> missing comma or operator.
> >>
> >> (but with a leading TAB, not spaces).
> >
> > Agreed. In addition, we need DCO certification or copyright
> assignment
> > (https://gcc.gnu.org/contribute.html#legal
> <https://gcc.gnu.org/contribute.html#legal>).
> >
> > Also, the patch itself was corrupted by word wrap in your mail
> client,
> > so it doesn't apply, e.g.:
> >
> >> @@ -7881,8 +7881,13 @@ cp_parser_postfix_expression (cp_parser
> >> *parser, bool address_p, bool cast_p,
> >
> > https://www.kernel.org/doc/html/latest/process/email-clients.html
> <https://www.kernel.org/doc/html/latest/process/email-clients.html> has
> > some tips for avoiding word wrap, or you can just attach the patch
> > instead of pasting it inline.
> >
> >> + /* This is just a heuristic. */
> >
> > Two spaces after the .
> >
> >> + inform (finish_loc, + "probably missing a
> comma
> >> or an operator before");
> >
> > The string should line up after the ( on the previous line.
> >
> > We might as well also put this inform under the
> > cp_parser_skip_to_closing_brace check; if there's no } at all, the
> > problem isn't a missing comma...
>
> Also, error59 seems like an error-recovery regression:
>
> > +++ b/gcc/testsuite/g++.dg/parse/error59.C
> > @@ -3,4 +3,4 @@
> > void foo()
> > {
> > (struct {}x){}; // { dg-error "" }
> > -}
> > +} // { dg-excess-errors "" }
>
> as we now get more of an error cascade. Perhaps also check the return
> value of the earlier require_open and don't skip_to_closing_brace if it
> failed?
>
> Jason
>
@@ -7881,8 +7881,13 @@ cp_parser_postfix_expression (cp_parser
*parser, bool address_p, bool cast_p,
--parser->prevent_constrained_type_specifiers;
/* Parse the cast itself. */
if (!cp_parser_error_occurred (parser))
- postfix_expression
- = cp_parser_functional_cast (parser, type);
+ {
+ if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_BRACE))
+ /* This can only be a cast. */
+ cp_parser_commit_to_topmost_tentative_parse (parser);
+ postfix_expression
+ = cp_parser_functional_cast (parser, type);
+ }
/* If that worked, we're done. */
if (cp_parser_parse_definitely (parser))
break;
@@ -26350,8 +26355,19 @@ cp_parser_braced_list (cp_parser *parser,
bool *non_constant_p /*=nullptr*/)
else if (non_constant_p)
*non_constant_p = false;
/* Now, there should be a trailing `}'. */
- location_t finish_loc = cp_lexer_peek_token (parser->lexer)->location;
- braces.require_close (parser);
+ cp_token * token = cp_lexer_peek_token (parser->lexer);
+ location_t finish_loc = token->location;
+ if (!braces.require_close (parser))
+ {
+ /* This is just a heuristic. */
+ if (token->type != CPP_SEMICOLON)
+ {
+ inform (finish_loc,
+ "probably missing a comma or an operator before");
+ if (cp_parser_skip_to_closing_brace (parser))
+ cp_lexer_consume_token (parser->lexer);
+ }
+ }
TREE_TYPE (initializer) = init_list_type_node;
recompute_constructor_flags (initializer);
b/gcc/testsuite/g++.dg/cpp0x/initlist-err1.C
new file mode 100644
@@ -0,0 +1,11 @@
+// PR c++/101232
+// { dg-do compile { target c++11 } }
+
+struct X {
+ int a;
+ int b;
+};
+
+void f() {
+ auto x = X{ 1, 2; }; // { dg-error "21:" }
+} // { dg-prune-output "expected declaration" }
b/gcc/testsuite/g++.dg/cpp0x/initlist-err2.C
new file mode 100644
@@ -0,0 +1,11 @@
+// PR c++/101232
+// { dg-do compile { target c++11 } }
+
+struct X {
+ int a;
+ int b;
+};
+
+void f() {
+ auto x = X{ 1 2 }; // { dg-error "19:.*probably" }
+}
b/gcc/testsuite/g++.dg/cpp0x/initlist-err3.C
new file mode 100644
@@ -0,0 +1,11 @@
+// PR c++/101232
+// { dg-do compile { target c++11 } }
+
+struct X {
+ int a;
+ int b;
+};
+
+void f() {
+ auto x = X{ 1, {2 }; // { dg-error "expected.*before" }
+}
b/gcc/testsuite/g++.dg/parse/error59.C
@@ -3,4 +3,4 @@
void foo()
{
(struct {}x){}; // { dg-error "" }
-}
+} // { dg-excess-errors "" }