Ping
On Tue, 2017-09-26 at 09:56 -0400, David Malcolm 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.
>
> The patch also undoes the change to g++.dg/parse/pragma2.C, as the
> old behavior is restored.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> 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 | 169 ++++++++++++++++++++-----
> ----------
> gcc/testsuite/g++.dg/parse/pragma2.C | 4 +-
> 2 files changed, 97 insertions(+), 76 deletions(-)
>
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 25b91df..56d9442 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -2767,53 +2767,116 @@ 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)
> +cp_parser_error_1 (cp_parser* parser, const char* gmsgid,
> + required_token missing_token_desc,
> + location_t matching_location)
> {
> - if (!cp_parser_simulate_error (parser))
> + cp_token *token = cp_lexer_peek_token (parser->lexer);
> + /* This diagnostic makes more sense if it is tagged to the line
> + of the token we just peeked at. */
> + cp_lexer_set_source_position_from_token (token);
> +
> + if (token->type == CPP_PRAGMA)
> {
> - cp_token *token = cp_lexer_peek_token (parser->lexer);
> - /* This diagnostic makes more sense if it is tagged to the
> line
> - of the token we just peeked at. */
> - cp_lexer_set_source_position_from_token (token);
> + error_at (token->location,
> + "%<#pragma%> is not allowed here");
> + cp_parser_skip_to_pragma_eol (parser, token);
> + return;
> + }
>
> - if (token->type == CPP_PRAGMA)
> + /* If this is actually a conflict marker, report it as such. */
> + if (token->type == CPP_LSHIFT
> + || token->type == CPP_RSHIFT
> + || token->type == CPP_EQ_EQ)
> + {
> + location_t loc;
> + if (cp_lexer_peek_conflict_marker (parser->lexer, token->type,
> &loc))
> {
> - error_at (token->location,
> - "%<#pragma%> is not allowed here");
> - cp_parser_skip_to_pragma_eol (parser, token);
> + error_at (loc, "version control conflict marker in file");
> return;
> }
> + }
>
> - /* If this is actually a conflict marker, report it as
> such. */
> - if (token->type == CPP_LSHIFT
> - || token->type == CPP_RSHIFT
> - || token->type == CPP_EQ_EQ)
> - {
> - location_t loc;
> - if (cp_lexer_peek_conflict_marker (parser->lexer, token-
> >type, &loc))
> - {
> - error_at (loc, "version control conflict marker in
> file");
> - return;
> - }
> - }
> + 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);
>
> - rich_location richloc (line_table, input_location);
> - 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
> @@ -27960,24 +28023,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
> @@ -28155,31 +28200,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 *-*-* } }
