From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 117707 invoked by alias); 11 Oct 2017 17:26:02 -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 117012 invoked by uid 89); 11 Oct 2017 17:26:01 -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=weren't, werent, pertinent 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; Wed, 11 Oct 2017 17:25:59 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BA65A13CF3 for ; Wed, 11 Oct 2017 17:25:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com BA65A13CF3 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=dmalcolm@redhat.com Received: from ovpn-116-23.phx2.redhat.com (ovpn-116-23.phx2.redhat.com [10.3.116.23]) by smtp.corp.redhat.com (Postfix) with ESMTP id 382C46293F; Wed, 11 Oct 2017 17:25:57 +0000 (UTC) Message-ID: <1507742757.29092.34.camel@redhat.com> Subject: PING: Re: [PATCH 1/2] C++: avoid partial duplicate implementation of cp_parser_error From: David Malcolm To: Jeff Law , gcc-patches@gcc.gnu.org Date: Wed, 11 Oct 2017 17:26:00 -0000 In-Reply-To: <1506434179-2736-2-git-send-email-dmalcolm@redhat.com> References: <1499107059-28855-1-git-send-email-dmalcolm@redhat.com> <1506434179-2736-1-git-send-email-dmalcolm@redhat.com> <1506434179-2736-2-git-send-email-dmalcolm@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2017-10/txt/msg00688.txt.bz2 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 *-*-* } }