On Wed, 2017-10-11 at 17:18 -0400, Jason Merrill wrote: > On Tue, Sep 26, 2017 at 9:56 AM, David Malcolm <dmalc...@redhat.com> > wrote: > > In r251026 (aka 3fe34694f0990d1d649711ede0326497f8a849dc, > > "C/C++: show pertinent open token when missing a close token") > > I copied part of cp_parser_error into cp_parser_required_error, > > leading to duplication of code. > > > > This patch eliminates this duplication by merging the two copies of > > the > > code into a new cp_parser_error_1 subroutine. > > > > Doing so removes an indentation level, making the patch appear to > > have > > more churn than it really does. > > FWIW, you could also attach a patch generated with -w to ignore > whitespace changes. > > The patch is OK. > > Jason
Thanks; committed to trunk as r253686. For reference, here's a version of the patch generated with -w. gcc/cp/ChangeLog: * parser.c (get_matching_symbol): Move to before... (cp_parser_error): Split out into... (cp_parser_error_1): ...this new function, merging in content from... (cp_parser_required_error): ...here. Eliminate partial duplicate of body of cp_parser_error in favor of a call to the new cp_parser_error_1 helper function. gcc/testsuite/ChangeLog: * g++.dg/parse/pragma2.C: Update to reflect reinstatement of the "#pragma is not allowed here" error. --- gcc/cp/parser.c | 119 ++++++++++++++++++++--------------- gcc/testsuite/g++.dg/parse/pragma2.C | 4 +- 2 files changed, 72 insertions(+), 51 deletions(-) diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index a556a21..4e08032 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -2769,16 +2769,43 @@ cp_lexer_peek_conflict_marker (cp_lexer *lexer, enum cpp_ttype tok1_kind, return true; } -/* If not parsing tentatively, issue a diagnostic of the form +/* Get a description of the matching symbol to TOKEN_DESC e.g. "(" for + RT_CLOSE_PAREN. */ + +static const char * +get_matching_symbol (required_token token_desc) +{ + switch (token_desc) + { + default: + gcc_unreachable (); + return ""; + case RT_CLOSE_BRACE: + return "{"; + case RT_CLOSE_PAREN: + return "("; + } +} + +/* Subroutine of cp_parser_error and cp_parser_required_error. + + Issue a diagnostic of the form FILE:LINE: MESSAGE before TOKEN where TOKEN is the next token in the input stream. MESSAGE (specified by the caller) is usually of the form "expected - OTHER-TOKEN". */ + OTHER-TOKEN". + + This bypasses the check for tentative passing, and potentially + adds material needed by cp_parser_required_error. + + If MISSING_TOKEN_DESC is not RT_NONE, and MATCHING_LOCATION is not + UNKNOWN_LOCATION, then we have an unmatched symbol at + MATCHING_LOCATION; highlight this secondary location. */ static void -cp_parser_error (cp_parser* parser, const char* gmsgid) -{ - if (!cp_parser_simulate_error (parser)) +cp_parser_error_1 (cp_parser* parser, const char* gmsgid, + required_token missing_token_desc, + location_t matching_location) { cp_token *token = cp_lexer_peek_token (parser->lexer); /* This diagnostic makes more sense if it is tagged to the line @@ -2806,16 +2833,52 @@ cp_parser_error (cp_parser* parser, const char* gmsgid) } } - rich_location richloc (line_table, input_location); + gcc_rich_location richloc (input_location); + + bool added_matching_location = false; + + if (missing_token_desc != RT_NONE) + { + /* If matching_location != UNKNOWN_LOCATION, highlight it. + Attempt to consolidate diagnostics by printing it as a + secondary range within the main diagnostic. */ + if (matching_location != UNKNOWN_LOCATION) + added_matching_location + = richloc.add_location_if_nearby (matching_location); + } + + /* Actually emit the error. */ c_parse_error (gmsgid, /* Because c_parser_error does not understand CPP_KEYWORD, keywords are treated like identifiers. */ (token->type == CPP_KEYWORD ? CPP_NAME : token->type), token->u.value, token->flags, &richloc); + + if (missing_token_desc != RT_NONE) + { + /* If we weren't able to consolidate matching_location, then + print it as a secondary diagnostic. */ + if (matching_location != UNKNOWN_LOCATION + && !added_matching_location) + inform (matching_location, "to match this %qs", + get_matching_symbol (missing_token_desc)); } } +/* If not parsing tentatively, issue a diagnostic of the form + FILE:LINE: MESSAGE before TOKEN + where TOKEN is the next token in the input stream. MESSAGE + (specified by the caller) is usually of the form "expected + OTHER-TOKEN". */ + +static void +cp_parser_error (cp_parser* parser, const char* gmsgid) +{ + if (!cp_parser_simulate_error (parser)) + cp_parser_error_1 (parser, gmsgid, RT_NONE, UNKNOWN_LOCATION); +} + /* Issue an error about name-lookup failing. NAME is the IDENTIFIER_NODE DECL is the result of the lookup (as returned from cp_parser_lookup_name). DESIRED is @@ -28087,24 +28150,6 @@ cp_parser_friend_p (const cp_decl_specifier_seq *decl_specifiers) return decl_spec_seq_has_spec_p (decl_specifiers, ds_friend); } -/* Get a description of the matching symbol to TOKEN_DESC e.g. "(" for - RT_CLOSE_PAREN. */ - -static const char * -get_matching_symbol (required_token token_desc) -{ - switch (token_desc) - { - default: - gcc_unreachable (); - return ""; - case RT_CLOSE_BRACE: - return "{"; - case RT_CLOSE_PAREN: - return "("; - } -} - /* Issue an error message indicating that TOKEN_DESC was expected. If KEYWORD is true, it indicated this function is called by cp_parser_require_keword and the required token can only be @@ -28282,31 +28327,7 @@ cp_parser_required_error (cp_parser *parser, } if (gmsgid) - { - /* Emulate rest of cp_parser_error. */ - cp_token *token = cp_lexer_peek_token (parser->lexer); - cp_lexer_set_source_position_from_token (token); - - gcc_rich_location richloc (input_location); - - /* If matching_location != UNKNOWN_LOCATION, highlight it. - Attempt to consolidate diagnostics by printing it as a - secondary range within the main diagnostic. */ - bool added_matching_location = false; - if (matching_location != UNKNOWN_LOCATION) - added_matching_location - = richloc.add_location_if_nearby (matching_location); - - c_parse_error (gmsgid, - (token->type == CPP_KEYWORD ? CPP_NAME : token->type), - token->u.value, token->flags, &richloc); - - /* If we weren't able to consolidate matching_location, then - print it as a secondary diagnostic. */ - if (matching_location != UNKNOWN_LOCATION && !added_matching_location) - inform (matching_location, "to match this %qs", - get_matching_symbol (token_desc)); - } + cp_parser_error_1 (parser, gmsgid, token_desc, matching_location); } diff --git a/gcc/testsuite/g++.dg/parse/pragma2.C b/gcc/testsuite/g++.dg/parse/pragma2.C index 3dc5fc1..c5616ff 100644 --- a/gcc/testsuite/g++.dg/parse/pragma2.C +++ b/gcc/testsuite/g++.dg/parse/pragma2.C @@ -4,5 +4,5 @@ // does not. int f(int x, #pragma interface // { dg-error "not allowed here" } - // { dg-bogus "expected identifier" "" { xfail *-*-* } .-1 } - int y); + // The parser gets confused and issues an error on the next line. + int y); // { dg-bogus "" "" { xfail *-*-* } } -- 1.8.5.3