On Thu, 26 Mar 2020, David Malcolm wrote: > On Thu, 2020-03-26 at 17:28 -0400, Patrick Palka via Gcc-patches wrote: > > This adds support to detect and recover from the case where an > > opening brace > > immediately follows the start of a requires-clause. So rather than > > emitting the > > error > > > > error: expected primary-expression before '{' token > > > > followed by a slew of irrevelant errors, we now assume the user had > > intended to > > write "requires requires {" and diagnose and recover accordingly. > > > > Tested on x86_64-pc-linux-gnu, does this look OK? > > > > gcc/cp/ChangeLog: > > > > PR c++/94306 > > * parser.c (cp_parser_requires_clause_opt): Diagnose and > > recover from > > "requires {" when "requires requires {" was probably intended. > > > > gcc/testsuite/ChangeLog: > > > > PR c++/94306 > > * g++.dg/concepts/diagnostic8.C: New test. > > --- > > gcc/cp/parser.c | 17 ++++++++++++++++- > > gcc/testsuite/g++.dg/concepts/diagnostic8.C | 6 ++++++ > > 2 files changed, 22 insertions(+), 1 deletion(-) > > create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic8.C > > > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > > index 05363653691..73c2c2cb010 100644 > > --- a/gcc/cp/parser.c > > +++ b/gcc/cp/parser.c > > @@ -27639,7 +27639,22 @@ cp_parser_requires_clause_opt (cp_parser > > *parser, bool lambda_p) > > } > > return NULL_TREE; > > } > > - cp_lexer_consume_token (parser->lexer); > > + > > + cp_token *tok2 = cp_lexer_peek_nth_token (parser->lexer, 2); > > + if (tok2->type == CPP_OPEN_BRACE) > > + { > > + /* An opening brace following the start of a requires-clause > > is > > + ill-formed; the user likely forgot the second `requires' that > > + would start a requires-expression. */ > > + gcc_rich_location richloc (tok2->location); > > + richloc.add_fixit_insert_before (" requires"); > > Thanks for adding a fix-it hint. That said, is this spacing here > correct? If I'm reading it right, this adds " requires" immediately > before the open brace, leading to a suggestion of: > "requires {" > becoming: > "requires requires{" > > Perhaps adding it immediately after the first requires via: > richloc.add_fixit_insert_after (first_requires_location, > " requires"); > would be better, which ought to lead to a suggestion of: > "requires {" > becoming: > "requires requires {" > > (assuming I'm reading the patch right) > > Might be an idea to test the effect of the fix-it hint e.g. via > -fdiagnostics-generate-patch. > > Hope this is constructive > Dave
That makes a lot of sense, thanks. Luckily we have the location of the first "requires" already handy. Here's the updated patch which incorporates the improved fix-it hint: -- >8 -- gcc/cp/ChangeLog: PR c++/94306 * parser.c (cp_parser_requires_clause_opt): Diagnose and recover from "requires {" when "requires requires {" was probably intended. gcc/testsuite/ChangeLog: PR c++/94306 * g++.dg/concepts/diagnostic8.C: New test. --- gcc/cp/parser.c | 17 ++++++++++++++++- gcc/testsuite/g++.dg/concepts/diagnostic8.C | 6 ++++++ 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic8.C diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 05363653691..5bc1ecd102e 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -27639,7 +27639,22 @@ cp_parser_requires_clause_opt (cp_parser *parser, bool lambda_p) } return NULL_TREE; } - cp_lexer_consume_token (parser->lexer); + + cp_token *tok2 = cp_lexer_peek_nth_token (parser->lexer, 2); + if (tok2->type == CPP_OPEN_BRACE) + { + /* An opening brace following the start of a requires-clause is + ill-formed; the user likely forgot the second `requires' that + would start a requires-expression. */ + gcc_rich_location richloc (tok2->location); + richloc.add_fixit_insert_after (tok->location, " requires"); + error_at (&richloc, "missing additional %<requires%> to start " + "a requires-expression"); + /* Don't consume the `requires', so that it's reused as the start of a + requires-expression. */ + } + else + cp_lexer_consume_token (parser->lexer); if (!flag_concepts_ts) return cp_parser_requires_clause_expression (parser, lambda_p); diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic8.C b/gcc/testsuite/g++.dg/concepts/diagnostic8.C new file mode 100644 index 00000000000..70d7e4a9cc1 --- /dev/null +++ b/gcc/testsuite/g++.dg/concepts/diagnostic8.C @@ -0,0 +1,6 @@ +// PR c++/94306 +// { dg-do compile { target c++2a } } + +template<typename T> struct S { }; +template<typename T> requires { typename T::type; } struct S<T> { }; +// { dg-error "missing additional .requires." "" { target *-*-* } .-1 } -- 2.26.0.rc1.11.g30e9940356