From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 70266 invoked by alias); 12 Oct 2017 17:34:57 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 70249 invoked by uid 89); 12 Oct 2017 17:34:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 12 Oct 2017 17:34:55 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7255480476 for ; Thu, 12 Oct 2017 17:34:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 7255480476 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=dmalcolm@redhat.com Received: from c64.redhat.com (ovpn-112-12.phx2.redhat.com [10.3.112.12]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0D7BB60841; Thu, 12 Oct 2017 17:34:51 +0000 (UTC) From: David Malcolm To: Jason Merrill Cc: Jeff Law , gcc-patches List , David Malcolm Subject: Re: [PATCH 1/2] C++: avoid partial duplicate implementation of cp_parser_error Date: Thu, 12 Oct 2017 18:10:00 -0000 Message-Id: <1507829736-15009-1-git-send-email-dmalcolm@redhat.com> In-Reply-To: References: X-IsSubscribed: yes X-SW-Source: 2017-10/txt/msg00774.txt.bz2 On Wed, 2017-10-11 at 17:18 -0400, Jason Merrill wrote: > On Tue, Sep 26, 2017 at 9:56 AM, 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. > > 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