[{"id":1784843,"web_url":"http://patchwork.ozlabs.org/comment/1784843/","msgid":"<1507742757.29092.34.camel@redhat.com>","list_archive_url":null,"date":"2017-10-11T17:25:57","subject":"PING: Re: [PATCH 1/2] C++: avoid partial duplicate implementation\n\tof cp_parser_error","submitter":{"id":24465,"url":"http://patchwork.ozlabs.org/api/people/24465/","name":"David Malcolm","email":"dmalcolm@redhat.com"},"content":"Ping\n\nOn Tue, 2017-09-26 at 09:56 -0400, David Malcolm wrote:\n> In r251026 (aka 3fe34694f0990d1d649711ede0326497f8a849dc,\n> \"C/C++: show pertinent open token when missing a close token\")\n> I copied part of cp_parser_error into cp_parser_required_error,\n> leading to duplication of code.\n> \n> This patch eliminates this duplication by merging the two copies of\n> the\n> code into a new cp_parser_error_1 subroutine.\n> \n> Doing so removes an indentation level, making the patch appear to\n> have\n> more churn than it really does.\n> \n> The patch also undoes the change to g++.dg/parse/pragma2.C, as the\n> old behavior is restored.\n> \n> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.\n> \n> OK for trunk?\n> \n> gcc/cp/ChangeLog:\n> \t* parser.c (get_matching_symbol): Move to before...\n> \t(cp_parser_error): Split out into...\n> \t(cp_parser_error_1): ...this new function, merging in content\n> \tfrom...\n> \t(cp_parser_required_error): ...here.  Eliminate partial\n> duplicate\n> \tof body of cp_parser_error in favor of a call to the new\n> \tcp_parser_error_1 helper function.\n> \n> gcc/testsuite/ChangeLog:\n> \t* g++.dg/parse/pragma2.C: Update to reflect reinstatement of\n> the\n> \t\"#pragma is not allowed here\" error.\n> ---\n>  gcc/cp/parser.c                      | 169 ++++++++++++++++++++-----\n> ----------\n>  gcc/testsuite/g++.dg/parse/pragma2.C |   4 +-\n>  2 files changed, 97 insertions(+), 76 deletions(-)\n> \n> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c\n> index 25b91df..56d9442 100644\n> --- a/gcc/cp/parser.c\n> +++ b/gcc/cp/parser.c\n> @@ -2767,53 +2767,116 @@ cp_lexer_peek_conflict_marker (cp_lexer\n> *lexer, enum cpp_ttype tok1_kind,\n>    return true;\n>  }\n>  \n> -/* If not parsing tentatively, issue a diagnostic of the form\n> +/* Get a description of the matching symbol to TOKEN_DESC e.g. \"(\"\n> for\n> +   RT_CLOSE_PAREN.  */\n> +\n> +static const char *\n> +get_matching_symbol (required_token token_desc)\n> +{\n> +  switch (token_desc)\n> +    {\n> +    default:\n> +      gcc_unreachable ();\n> +      return \"\";\n> +    case RT_CLOSE_BRACE:\n> +      return \"{\";\n> +    case RT_CLOSE_PAREN:\n> +      return \"(\";\n> +    }\n> +}\n> +\n> +/* Subroutine of cp_parser_error and cp_parser_required_error.\n> +\n> +   Issue a diagnostic of the form\n>        FILE:LINE: MESSAGE before TOKEN\n>     where TOKEN is the next token in the input stream.  MESSAGE\n>     (specified by the caller) is usually of the form \"expected\n> -   OTHER-TOKEN\".  */\n> +   OTHER-TOKEN\".\n> +\n> +   This bypasses the check for tentative passing, and potentially\n> +   adds material needed by cp_parser_required_error.\n> +\n> +   If MISSING_TOKEN_DESC is not RT_NONE, and MATCHING_LOCATION is\n> not\n> +   UNKNOWN_LOCATION, then we have an unmatched symbol at\n> +   MATCHING_LOCATION; highlight this secondary location.  */\n>  \n>  static void\n> -cp_parser_error (cp_parser* parser, const char* gmsgid)\n> +cp_parser_error_1 (cp_parser* parser, const char* gmsgid,\n> +\t\t   required_token missing_token_desc,\n> +\t\t   location_t matching_location)\n>  {\n> -  if (!cp_parser_simulate_error (parser))\n> +  cp_token *token = cp_lexer_peek_token (parser->lexer);\n> +  /* This diagnostic makes more sense if it is tagged to the line\n> +     of the token we just peeked at.  */\n> +  cp_lexer_set_source_position_from_token (token);\n> +\n> +  if (token->type == CPP_PRAGMA)\n>      {\n> -      cp_token *token = cp_lexer_peek_token (parser->lexer);\n> -      /* This diagnostic makes more sense if it is tagged to the\n> line\n> -\t of the token we just peeked at.  */\n> -      cp_lexer_set_source_position_from_token (token);\n> +      error_at (token->location,\n> +\t\t\"%<#pragma%> is not allowed here\");\n> +      cp_parser_skip_to_pragma_eol (parser, token);\n> +      return;\n> +    }\n>  \n> -      if (token->type == CPP_PRAGMA)\n> +  /* If this is actually a conflict marker, report it as such.  */\n> +  if (token->type == CPP_LSHIFT\n> +      || token->type == CPP_RSHIFT\n> +      || token->type == CPP_EQ_EQ)\n> +    {\n> +      location_t loc;\n> +      if (cp_lexer_peek_conflict_marker (parser->lexer, token->type, \n> &loc))\n>  \t{\n> -\t  error_at (token->location,\n> -\t\t    \"%<#pragma%> is not allowed here\");\n> -\t  cp_parser_skip_to_pragma_eol (parser, token);\n> +\t  error_at (loc, \"version control conflict marker in file\");\n>  \t  return;\n>  \t}\n> +    }\n>  \n> -      /* If this is actually a conflict marker, report it as\n> such.  */\n> -      if (token->type == CPP_LSHIFT\n> -\t  || token->type == CPP_RSHIFT\n> -\t  || token->type == CPP_EQ_EQ)\n> -\t{\n> -\t  location_t loc;\n> -\t  if (cp_lexer_peek_conflict_marker (parser->lexer, token-\n> >type, &loc))\n> -\t    {\n> -\t      error_at (loc, \"version control conflict marker in\n> file\");\n> -\t      return;\n> -\t    }\n> -\t}\n> +  gcc_rich_location richloc (input_location);\n> +\n> +  bool added_matching_location = false;\n> +\n> +  if (missing_token_desc != RT_NONE)\n> +    {\n> +      /* If matching_location != UNKNOWN_LOCATION, highlight it.\n> +\t Attempt to consolidate diagnostics by printing it as a\n> +\tsecondary range within the main diagnostic.  */\n> +      if (matching_location != UNKNOWN_LOCATION)\n> +\tadded_matching_location\n> +\t  = richloc.add_location_if_nearby (matching_location);\n> +    }\n> +\n> +  /* Actually emit the error.  */\n> +  c_parse_error (gmsgid,\n> +\t\t /* Because c_parser_error does not understand\n> +\t\t    CPP_KEYWORD, keywords are treated like\n> +\t\t    identifiers.  */\n> +\t\t (token->type == CPP_KEYWORD ? CPP_NAME : token-\n> >type),\n> +\t\t token->u.value, token->flags, &richloc);\n>  \n> -      rich_location richloc (line_table, input_location);\n> -      c_parse_error (gmsgid,\n> -\t\t     /* Because c_parser_error does not understand\n> -\t\t\tCPP_KEYWORD, keywords are treated like\n> -\t\t\tidentifiers.  */\n> -\t\t     (token->type == CPP_KEYWORD ? CPP_NAME : token-\n> >type),\n> -\t\t     token->u.value, token->flags, &richloc);\n> +  if (missing_token_desc != RT_NONE)\n> +    {\n> +      /* If we weren't able to consolidate matching_location, then\n> +\t print it as a secondary diagnostic.  */\n> +      if (matching_location != UNKNOWN_LOCATION\n> +\t  && !added_matching_location)\n> +\tinform (matching_location, \"to match this %qs\",\n> +\t\tget_matching_symbol (missing_token_desc));\n>      }\n>  }\n>  \n> +/* If not parsing tentatively, issue a diagnostic of the form\n> +      FILE:LINE: MESSAGE before TOKEN\n> +   where TOKEN is the next token in the input stream.  MESSAGE\n> +   (specified by the caller) is usually of the form \"expected\n> +   OTHER-TOKEN\".  */\n> +\n> +static void\n> +cp_parser_error (cp_parser* parser, const char* gmsgid)\n> +{\n> +  if (!cp_parser_simulate_error (parser))\n> +    cp_parser_error_1 (parser, gmsgid, RT_NONE, UNKNOWN_LOCATION);\n> +}\n> +\n>  /* Issue an error about name-lookup failing.  NAME is the\n>     IDENTIFIER_NODE DECL is the result of\n>     the lookup (as returned from cp_parser_lookup_name).  DESIRED is\n> @@ -27960,24 +28023,6 @@ cp_parser_friend_p (const\n> cp_decl_specifier_seq *decl_specifiers)\n>    return decl_spec_seq_has_spec_p (decl_specifiers, ds_friend);\n>  }\n>  \n> -/* Get a description of the matching symbol to TOKEN_DESC e.g. \"(\"\n> for\n> -   RT_CLOSE_PAREN.  */\n> -\n> -static const char *\n> -get_matching_symbol (required_token token_desc)\n> -{\n> -  switch (token_desc)\n> -    {\n> -    default:\n> -      gcc_unreachable ();\n> -      return \"\";\n> -    case RT_CLOSE_BRACE:\n> -      return \"{\";\n> -    case RT_CLOSE_PAREN:\n> -      return \"(\";\n> -    }\n> -}\n> -\n>  /* Issue an error message indicating that TOKEN_DESC was expected.\n>     If KEYWORD is true, it indicated this function is called by\n>     cp_parser_require_keword and the required token can only be\n> @@ -28155,31 +28200,7 @@ cp_parser_required_error (cp_parser *parser,\n>      }\n>  \n>    if (gmsgid)\n> -    {\n> -      /* Emulate rest of cp_parser_error.  */\n> -      cp_token *token = cp_lexer_peek_token (parser->lexer);\n> -      cp_lexer_set_source_position_from_token (token);\n> -\n> -      gcc_rich_location richloc (input_location);\n> -\n> -      /* If matching_location != UNKNOWN_LOCATION, highlight it.\n> -\t Attempt to consolidate diagnostics by printing it as a\n> -\tsecondary range within the main diagnostic.  */\n> -      bool added_matching_location = false;\n> -      if (matching_location != UNKNOWN_LOCATION)\n> -\tadded_matching_location\n> -\t  = richloc.add_location_if_nearby (matching_location);\n> -\n> -      c_parse_error (gmsgid,\n> -\t\t     (token->type == CPP_KEYWORD ? CPP_NAME : token-\n> >type),\n> -\t\t     token->u.value, token->flags, &richloc);\n> -\n> -      /* If we weren't able to consolidate matching_location, then\n> -\t print it as a secondary diagnostic.  */\n> -      if (matching_location != UNKNOWN_LOCATION &&\n> !added_matching_location)\n> -\tinform (matching_location, \"to match this %qs\",\n> -\t\tget_matching_symbol (token_desc));\n> -    }\n> +    cp_parser_error_1 (parser, gmsgid, token_desc,\n> matching_location);\n>  }\n>  \n>  \n> diff --git a/gcc/testsuite/g++.dg/parse/pragma2.C\n> b/gcc/testsuite/g++.dg/parse/pragma2.C\n> index 3dc5fc1..c5616ff 100644\n> --- a/gcc/testsuite/g++.dg/parse/pragma2.C\n> +++ b/gcc/testsuite/g++.dg/parse/pragma2.C\n> @@ -4,5 +4,5 @@\n>  // does not.\n>  int f(int x,\n>  #pragma interface  // { dg-error \"not allowed here\" }\n> -      // { dg-bogus \"expected identifier\" \"\" { xfail *-*-* } .-1 }\n> -      int y);\n> +      // The parser gets confused and issues an error on the next\n> line.\n> +      int y); // { dg-bogus \"\" \"\" { xfail *-*-* } }","headers":{"Return-Path":"<gcc-patches-return-463963-incoming=patchwork.ozlabs.org@gcc.gnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","mailing list gcc-patches@gcc.gnu.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org\n\t(client-ip=209.132.180.131; helo=sourceware.org;\n\tenvelope-from=gcc-patches-return-463963-incoming=patchwork.ozlabs.org@gcc.gnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org\n\theader.b=\"fCLcggp9\"; dkim-atps=neutral","sourceware.org; auth=none","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=dmalcolm@redhat.com"],"Received":["from sourceware.org (server1.sourceware.org [209.132.180.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yC19F4v40z9sRq\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 12 Oct 2017 04:26:09 +1100 (AEDT)","(qmail 117707 invoked by alias); 11 Oct 2017 17:26:02 -0000","(qmail 117012 invoked by uid 89); 11 Oct 2017 17:26:01 -0000","from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by\n\tsourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP;\n\tWed, 11 Oct 2017 17:25:59 +0000","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com\n\t[10.5.11.15])\t(using TLSv1.2 with cipher AECDH-AES256-SHA\n\t(256/256 bits))\t(No client certificate requested)\tby\n\tmx1.redhat.com (Postfix) with ESMTPS id BA65A13CF3\tfor\n\t<gcc-patches@gcc.gnu.org>; Wed, 11 Oct 2017 17:25:58 +0000 (UTC)","from ovpn-116-23.phx2.redhat.com (ovpn-116-23.phx2.redhat.com\n\t[10.3.116.23])\tby smtp.corp.redhat.com (Postfix) with ESMTP\n\tid 382C46293F; Wed, 11 Oct 2017 17:25:57 +0000 (UTC)"],"DomainKey-Signature":"a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:message-id:subject:from:to:date:in-reply-to:references\n\t:content-type:mime-version:content-transfer-encoding; q=dns; s=\n\tdefault; b=nVL2LIu5DAznD+QYnQccGkee97CWpckawFxoRckQBHsqA3nOQm+zI\n\ts5/iJRT+vWKXNd3h0ACGGfLbm3dN0qeaBbAadh2/6XpAEk38SkzF1Z448CD0PTYz\n\toSw+hIfGjyrb90/aBwO2kj1JkOLUraFTla7ydKs17AHwk1jhhryBz4=","DKIM-Signature":"v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:message-id:subject:from:to:date:in-reply-to:references\n\t:content-type:mime-version:content-transfer-encoding; s=default;\n\tbh=B1dyo439BO5x3qVLqY6u6ry4mSY=; b=fCLcggp9U/vv7aGTytDbj8eQqsKU\n\tj/xkLeMNNZo8CPgY9fSaqwkrFQwtNqFQGIj1NwgQuZVGbDD52t4Hgl5M2VklpA+f\n\tHdtqBsVeHu64x0tnU675fYyryyw+yt7AcCUSUBpZbS7x5pgvBHoQM+XKSBQJX6Ig\n\tDsARgMW93TVjzjw=","Mailing-List":"contact gcc-patches-help@gcc.gnu.org; run by ezmlm","Precedence":"bulk","List-Id":"<gcc-patches.gcc.gnu.org>","List-Unsubscribe":"<mailto:gcc-patches-unsubscribe-incoming=patchwork.ozlabs.org@gcc.gnu.org>","List-Archive":"<http://gcc.gnu.org/ml/gcc-patches/>","List-Post":"<mailto:gcc-patches@gcc.gnu.org>","List-Help":"<mailto:gcc-patches-help@gcc.gnu.org>","Sender":"gcc-patches-owner@gcc.gnu.org","X-Virus-Found":"No","X-Spam-SWARE-Status":"No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0,\n\tGIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD,\n\tSPF_HELO_PASS autolearn=ham version=3.3.2 spammy=weren't,\n\twerent, pertinent","X-HELO":"mx1.redhat.com","DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com BA65A13CF3","Message-ID":"<1507742757.29092.34.camel@redhat.com>","Subject":"PING: Re: [PATCH 1/2] C++: avoid partial duplicate implementation\n\tof cp_parser_error","From":"David Malcolm <dmalcolm@redhat.com>","To":"Jeff Law <law@redhat.com>, gcc-patches@gcc.gnu.org","Date":"Wed, 11 Oct 2017 13:25:57 -0400","In-Reply-To":"<1506434179-2736-2-git-send-email-dmalcolm@redhat.com>","References":"<1499107059-28855-1-git-send-email-dmalcolm@redhat.com>\t\n\t<1506434179-2736-1-git-send-email-dmalcolm@redhat.com>\t\n\t<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"}},{"id":1784984,"web_url":"http://patchwork.ozlabs.org/comment/1784984/","msgid":"<CADzB+2kGNTWrfynftSsz2Vdm6nMuy8sFOz-O=OSEkTJEPG2gnA@mail.gmail.com>","list_archive_url":null,"date":"2017-10-11T21:18:22","subject":"Re: [PATCH 1/2] C++: avoid partial duplicate implementation of\n\tcp_parser_error","submitter":{"id":4337,"url":"http://patchwork.ozlabs.org/api/people/4337/","name":"Jason Merrill","email":"jason@redhat.com"},"content":"On Tue, Sep 26, 2017 at 9:56 AM, David Malcolm <dmalcolm@redhat.com> wrote:\n> In r251026 (aka 3fe34694f0990d1d649711ede0326497f8a849dc,\n> \"C/C++: show pertinent open token when missing a close token\")\n> I copied part of cp_parser_error into cp_parser_required_error,\n> leading to duplication of code.\n>\n> This patch eliminates this duplication by merging the two copies of the\n> code into a new cp_parser_error_1 subroutine.\n>\n> Doing so removes an indentation level, making the patch appear to have\n> more churn than it really does.\n\nFWIW, you could also attach a patch generated with -w to ignore\nwhitespace changes.\n\nThe patch is OK.\n\nJason","headers":{"Return-Path":"<gcc-patches-return-463983-incoming=patchwork.ozlabs.org@gcc.gnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","mailing list gcc-patches@gcc.gnu.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org\n\t(client-ip=209.132.180.131; helo=sourceware.org;\n\tenvelope-from=gcc-patches-return-463983-incoming=patchwork.ozlabs.org@gcc.gnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org\n\theader.b=\"xRHCEpLh\"; dkim-atps=neutral","sourceware.org; auth=none"],"Received":["from sourceware.org (server1.sourceware.org [209.132.180.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yC6Kr0MZyz9sRq\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 12 Oct 2017 08:18:55 +1100 (AEDT)","(qmail 110464 invoked by alias); 11 Oct 2017 21:18:46 -0000","(qmail 109413 invoked by uid 89); 11 Oct 2017 21:18:46 -0000","from mail-it0-f53.google.com (HELO mail-it0-f53.google.com)\n\t(209.85.214.53) by sourceware.org\n\t(qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP;\n\tWed, 11 Oct 2017 21:18:44 +0000","by mail-it0-f53.google.com with SMTP id l196so4500674itl.4 for\n\t<gcc-patches@gcc.gnu.org>; Wed, 11 Oct 2017 14:18:44 -0700 (PDT)","by 10.107.170.26 with HTTP; Wed, 11 Oct 2017 14:18:22 -0700 (PDT)"],"DomainKey-Signature":"a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:mime-version:in-reply-to:references:from:date:message-id\n\t:subject:to:cc:content-type; q=dns; s=default; b=GWLXjf8cSwLz9er\n\texwy+zdBH1ECJr+StRHKFvhd7PbHCQWLmGpfKyjvJZgoT/dOcMTsp+1/DRCgEaQR\n\tCb162E9jtT16eepwa4avUdIM+BLngFiRj3iFq5wvoBAwfVZNdFe6ExzqUB5sZSbP\n\tukk34//+4JX4gbfsGbmQp9lSkKQc=","DKIM-Signature":"v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:mime-version:in-reply-to:references:from:date:message-id\n\t:subject:to:cc:content-type; s=default; bh=/8vNKHiBewnbdlS0lWFqk\n\tX7aQTw=; b=xRHCEpLhkw4X2S+2rWB0HJfY/P1DwCmXD/1LPU4mu4uYm0X5vqzwm\n\tMT2SKh2W8G/xdUHkA9WfELbXUevK7Yu8NZHi/v7HrHMcKGpMfFaFpHSZcMbHCgjC\n\t13cDk6ysop+aGp8e3s4ytivKb5HiLjeOxHZ93If3PP6Id9FEwcDogc=","Mailing-List":"contact gcc-patches-help@gcc.gnu.org; run by ezmlm","Precedence":"bulk","List-Id":"<gcc-patches.gcc.gnu.org>","List-Unsubscribe":"<mailto:gcc-patches-unsubscribe-incoming=patchwork.ozlabs.org@gcc.gnu.org>","List-Archive":"<http://gcc.gnu.org/ml/gcc-patches/>","List-Post":"<mailto:gcc-patches@gcc.gnu.org>","List-Help":"<mailto:gcc-patches-help@gcc.gnu.org>","Sender":"gcc-patches-owner@gcc.gnu.org","X-Virus-Found":"No","X-Spam-SWARE-Status":"No, score=-2.9 required=5.0 tests=AWL, BAYES_00,\n\tRCVD_IN_DNSWL_LOW,\n\tRCVD_IN_SORBS_SPAM autolearn=ham version=3.3.2 spammy=","X-HELO":"mail-it0-f53.google.com","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net;\n\ts=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=dHZe/2HPrdKQ+5xhepjNisao8BT7ZCjWk8mBHS9E3Tk=;\n\tb=hjtYcRUE5ti1pV3kd51jjuQOHlauQIwF8vfwgNGWPQqRdXpRcbluLG2jTF70WtYaKi\n\tMgTdMlJNniMjPfluFWxwoFF7p6X0h+1vCoLrlwbEuNYblBuoLS6POdxczzp+wJQYOQCJ\n\tBdghTCl6YwV/T5UFXJCdrZGtsxvFRsDNOhPa5VPhiv+dfwAMegQFQ5IhzFVtQsVnm9cH\n\tlRwmFEa0+ONX+r3uM/RsM8WgmIpWW0IaQwBIpViri3RVct+LFJ4xoobHnb5wlhhhwWew\n\tUfIG+qfbKSBQH4EbhQZYyR8KiiabaZ3c5F3FrmAe/k/o637flRf8rLIWM/UN2H2Wa7tK\n\t7rQg==","X-Gm-Message-State":"AMCzsaVtS591XRI8iR5beXQGmomAXw0Gi+qPZD8e5NZIsl9kYGdS36aG\tGZyJVwNc8WoNvwRz/Him6NzyNIOndLTTiQZb8uvB3w==","X-Google-Smtp-Source":"AOwi7QC3LKsn/gIySWGsSa06Uqk1TPlmUWp+Mn2VGG/uJ0qqxj+Asss+DCcS7WZGbF3Eqwl8CD5+HWP6KoW5CuBnL7c=","X-Received":"by 10.36.200.132 with SMTP id w126mr433355itf.101.1507756723068;\n\tWed, 11 Oct 2017 14:18:43 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<1506434179-2736-2-git-send-email-dmalcolm@redhat.com>","References":"<1499107059-28855-1-git-send-email-dmalcolm@redhat.com>\n\t<1506434179-2736-1-git-send-email-dmalcolm@redhat.com>\n\t<1506434179-2736-2-git-send-email-dmalcolm@redhat.com>","From":"Jason Merrill <jason@redhat.com>","Date":"Wed, 11 Oct 2017 17:18:22 -0400","Message-ID":"<CADzB+2kGNTWrfynftSsz2Vdm6nMuy8sFOz-O=OSEkTJEPG2gnA@mail.gmail.com>","Subject":"Re: [PATCH 1/2] C++: avoid partial duplicate implementation of\n\tcp_parser_error","To":"David Malcolm <dmalcolm@redhat.com>","Cc":"Jeff Law <law@redhat.com>, gcc-patches List <gcc-patches@gcc.gnu.org>","Content-Type":"text/plain; charset=\"UTF-8\"","X-IsSubscribed":"yes"}},{"id":1785691,"web_url":"http://patchwork.ozlabs.org/comment/1785691/","msgid":"<1507829736-15009-1-git-send-email-dmalcolm@redhat.com>","list_archive_url":null,"date":"2017-10-12T17:35:36","subject":"Re: [PATCH 1/2] C++: avoid partial duplicate implementation of\n\tcp_parser_error","submitter":{"id":24465,"url":"http://patchwork.ozlabs.org/api/people/24465/","name":"David Malcolm","email":"dmalcolm@redhat.com"},"content":"On Wed, 2017-10-11 at 17:18 -0400, Jason Merrill wrote:\n> On Tue, Sep 26, 2017 at 9:56 AM, David Malcolm <dmalcolm@redhat.com>\n> wrote:\n> > In r251026 (aka 3fe34694f0990d1d649711ede0326497f8a849dc,\n> > \"C/C++: show pertinent open token when missing a close token\")\n> > I copied part of cp_parser_error into cp_parser_required_error,\n> > leading to duplication of code.\n> > \n> > This patch eliminates this duplication by merging the two copies of\n> > the\n> > code into a new cp_parser_error_1 subroutine.\n> > \n> > Doing so removes an indentation level, making the patch appear to\n> > have\n> > more churn than it really does.\n> \n> FWIW, you could also attach a patch generated with -w to ignore\n> whitespace changes.\n> \n> The patch is OK.\n> \n> Jason\n\nThanks; committed to trunk as r253686.\n\nFor reference, here's a version of the patch generated with -w.\n\ngcc/cp/ChangeLog:\n\t* parser.c (get_matching_symbol): Move to before...\n\t(cp_parser_error): Split out into...\n\t(cp_parser_error_1): ...this new function, merging in content\n\tfrom...\n\t(cp_parser_required_error): ...here.  Eliminate partial duplicate\n\tof body of cp_parser_error in favor of a call to the new\n\tcp_parser_error_1 helper function.\n\ngcc/testsuite/ChangeLog:\n\t* g++.dg/parse/pragma2.C: Update to reflect reinstatement of the\n\t\"#pragma is not allowed here\" error.\n---\n gcc/cp/parser.c                      | 119 ++++++++++++++++++++---------------\n gcc/testsuite/g++.dg/parse/pragma2.C |   4 +-\n 2 files changed, 72 insertions(+), 51 deletions(-)\n\ndiff --git a/gcc/cp/parser.c b/gcc/cp/parser.c\nindex a556a21..4e08032 100644\n--- a/gcc/cp/parser.c\n+++ b/gcc/cp/parser.c\n@@ -2769,16 +2769,43 @@ cp_lexer_peek_conflict_marker (cp_lexer *lexer, enum cpp_ttype tok1_kind,\n   return true;\n }\n \n-/* If not parsing tentatively, issue a diagnostic of the form\n+/* Get a description of the matching symbol to TOKEN_DESC e.g. \"(\" for\n+   RT_CLOSE_PAREN.  */\n+\n+static const char *\n+get_matching_symbol (required_token token_desc)\n+{\n+  switch (token_desc)\n+    {\n+    default:\n+      gcc_unreachable ();\n+      return \"\";\n+    case RT_CLOSE_BRACE:\n+      return \"{\";\n+    case RT_CLOSE_PAREN:\n+      return \"(\";\n+    }\n+}\n+\n+/* Subroutine of cp_parser_error and cp_parser_required_error.\n+\n+   Issue a diagnostic of the form\n       FILE:LINE: MESSAGE before TOKEN\n    where TOKEN is the next token in the input stream.  MESSAGE\n    (specified by the caller) is usually of the form \"expected\n-   OTHER-TOKEN\".  */\n+   OTHER-TOKEN\".\n+\n+   This bypasses the check for tentative passing, and potentially\n+   adds material needed by cp_parser_required_error.\n+\n+   If MISSING_TOKEN_DESC is not RT_NONE, and MATCHING_LOCATION is not\n+   UNKNOWN_LOCATION, then we have an unmatched symbol at\n+   MATCHING_LOCATION; highlight this secondary location.  */\n \n static void\n-cp_parser_error (cp_parser* parser, const char* gmsgid)\n-{\n-  if (!cp_parser_simulate_error (parser))\n+cp_parser_error_1 (cp_parser* parser, const char* gmsgid,\n+\t\t   required_token missing_token_desc,\n+\t\t   location_t matching_location)\n {\n   cp_token *token = cp_lexer_peek_token (parser->lexer);\n   /* This diagnostic makes more sense if it is tagged to the line\n@@ -2806,16 +2833,52 @@ cp_parser_error (cp_parser* parser, const char* gmsgid)\n \t}\n     }\n \n-      rich_location richloc (line_table, input_location);\n+  gcc_rich_location richloc (input_location);\n+\n+  bool added_matching_location = false;\n+\n+  if (missing_token_desc != RT_NONE)\n+    {\n+      /* If matching_location != UNKNOWN_LOCATION, highlight it.\n+\t Attempt to consolidate diagnostics by printing it as a\n+\tsecondary range within the main diagnostic.  */\n+      if (matching_location != UNKNOWN_LOCATION)\n+\tadded_matching_location\n+\t  = richloc.add_location_if_nearby (matching_location);\n+    }\n+\n+  /* Actually emit the error.  */\n   c_parse_error (gmsgid,\n \t\t /* Because c_parser_error does not understand\n \t\t    CPP_KEYWORD, keywords are treated like\n \t\t    identifiers.  */\n \t\t (token->type == CPP_KEYWORD ? CPP_NAME : token->type),\n \t\t token->u.value, token->flags, &richloc);\n+\n+  if (missing_token_desc != RT_NONE)\n+    {\n+      /* If we weren't able to consolidate matching_location, then\n+\t print it as a secondary diagnostic.  */\n+      if (matching_location != UNKNOWN_LOCATION\n+\t  && !added_matching_location)\n+\tinform (matching_location, \"to match this %qs\",\n+\t\tget_matching_symbol (missing_token_desc));\n     }\n }\n \n+/* If not parsing tentatively, issue a diagnostic of the form\n+      FILE:LINE: MESSAGE before TOKEN\n+   where TOKEN is the next token in the input stream.  MESSAGE\n+   (specified by the caller) is usually of the form \"expected\n+   OTHER-TOKEN\".  */\n+\n+static void\n+cp_parser_error (cp_parser* parser, const char* gmsgid)\n+{\n+  if (!cp_parser_simulate_error (parser))\n+    cp_parser_error_1 (parser, gmsgid, RT_NONE, UNKNOWN_LOCATION);\n+}\n+\n /* Issue an error about name-lookup failing.  NAME is the\n    IDENTIFIER_NODE DECL is the result of\n    the lookup (as returned from cp_parser_lookup_name).  DESIRED is\n@@ -28087,24 +28150,6 @@ cp_parser_friend_p (const cp_decl_specifier_seq *decl_specifiers)\n   return decl_spec_seq_has_spec_p (decl_specifiers, ds_friend);\n }\n \n-/* Get a description of the matching symbol to TOKEN_DESC e.g. \"(\" for\n-   RT_CLOSE_PAREN.  */\n-\n-static const char *\n-get_matching_symbol (required_token token_desc)\n-{\n-  switch (token_desc)\n-    {\n-    default:\n-      gcc_unreachable ();\n-      return \"\";\n-    case RT_CLOSE_BRACE:\n-      return \"{\";\n-    case RT_CLOSE_PAREN:\n-      return \"(\";\n-    }\n-}\n-\n /* Issue an error message indicating that TOKEN_DESC was expected.\n    If KEYWORD is true, it indicated this function is called by\n    cp_parser_require_keword and the required token can only be\n@@ -28282,31 +28327,7 @@ cp_parser_required_error (cp_parser *parser,\n     }\n \n   if (gmsgid)\n-    {\n-      /* Emulate rest of cp_parser_error.  */\n-      cp_token *token = cp_lexer_peek_token (parser->lexer);\n-      cp_lexer_set_source_position_from_token (token);\n-\n-      gcc_rich_location richloc (input_location);\n-\n-      /* If matching_location != UNKNOWN_LOCATION, highlight it.\n-\t Attempt to consolidate diagnostics by printing it as a\n-\tsecondary range within the main diagnostic.  */\n-      bool added_matching_location = false;\n-      if (matching_location != UNKNOWN_LOCATION)\n-\tadded_matching_location\n-\t  = richloc.add_location_if_nearby (matching_location);\n-\n-      c_parse_error (gmsgid,\n-\t\t     (token->type == CPP_KEYWORD ? CPP_NAME : token->type),\n-\t\t     token->u.value, token->flags, &richloc);\n-\n-      /* If we weren't able to consolidate matching_location, then\n-\t print it as a secondary diagnostic.  */\n-      if (matching_location != UNKNOWN_LOCATION && !added_matching_location)\n-\tinform (matching_location, \"to match this %qs\",\n-\t\tget_matching_symbol (token_desc));\n-    }\n+    cp_parser_error_1 (parser, gmsgid, token_desc, matching_location);\n }\n \n \ndiff --git a/gcc/testsuite/g++.dg/parse/pragma2.C b/gcc/testsuite/g++.dg/parse/pragma2.C\nindex 3dc5fc1..c5616ff 100644\n--- a/gcc/testsuite/g++.dg/parse/pragma2.C\n+++ b/gcc/testsuite/g++.dg/parse/pragma2.C\n@@ -4,5 +4,5 @@\n // does not.\n int f(int x,\n #pragma interface  // { dg-error \"not allowed here\" }\n-      // { dg-bogus \"expected identifier\" \"\" { xfail *-*-* } .-1 }\n-      int y);\n+      // The parser gets confused and issues an error on the next line.\n+      int y); // { dg-bogus \"\" \"\" { xfail *-*-* } }","headers":{"Return-Path":"<gcc-patches-return-464049-incoming=patchwork.ozlabs.org@gcc.gnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","mailing list gcc-patches@gcc.gnu.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org\n\t(client-ip=209.132.180.131; helo=sourceware.org;\n\tenvelope-from=gcc-patches-return-464049-incoming=patchwork.ozlabs.org@gcc.gnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org\n\theader.b=\"E2dGx9GS\"; dkim-atps=neutral","sourceware.org; auth=none","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=dmalcolm@redhat.com"],"Received":["from sourceware.org (server1.sourceware.org [209.132.180.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yCdK81MBtz9t3R\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri, 13 Oct 2017 04:35:06 +1100 (AEDT)","(qmail 70266 invoked by alias); 12 Oct 2017 17:34:57 -0000","(qmail 70249 invoked by uid 89); 12 Oct 2017 17:34:56 -0000","from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by\n\tsourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP;\n\tThu, 12 Oct 2017 17:34:55 +0000","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com\n\t[10.5.11.13])\t(using TLSv1.2 with cipher AECDH-AES256-SHA\n\t(256/256 bits))\t(No client certificate requested)\tby\n\tmx1.redhat.com (Postfix) with ESMTPS id 7255480476\tfor\n\t<gcc-patches@gcc.gnu.org>; Thu, 12 Oct 2017 17:34:53 +0000 (UTC)","from c64.redhat.com (ovpn-112-12.phx2.redhat.com [10.3.112.12])\tby\n\tsmtp.corp.redhat.com (Postfix) with ESMTP id 0D7BB60841;\n\tThu, 12 Oct 2017 17:34:51 +0000 (UTC)"],"DomainKey-Signature":"a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender:from\n\t:to:cc:subject:date:message-id:in-reply-to:references; q=dns; s=\n\tdefault; b=XWzHRdkoPzigVTGWuGAcE6HymhJHtaDtlcE6trA6AT4IuPSqharSc\n\tmr7M7moFnw6DHRfzNVEiN5yyIj1mA9oQ2JChH4xLrsMmCvJtxFkoslbZvmra2ZC6\n\tnM8gfYD4xqsKaPZ+xQ3u5Pr45HPcMve+U/fFHYFvSnmuKSqtgwQ030=","DKIM-Signature":"v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender:from\n\t:to:cc:subject:date:message-id:in-reply-to:references; s=\n\tdefault; bh=rjGrxLP0BOCE66URjSByGae4M3o=; b=E2dGx9GS1it40oiv2rF8\n\txBCGQHXcH2SPsxU2M+kOai1vjhnhr1KdKH9Y3nT9NRhB13qfXseWjd7s1of6gqkN\n\tt2EYZb//EPU0VhzP9MTNYFhsY2QIimvbbhAeKEnCPjEPGydbtMrgJwWEKjvTH+sp\n\tHqhtxzhjTYqW42ZRv2KhiTM=","Mailing-List":"contact gcc-patches-help@gcc.gnu.org; run by ezmlm","Precedence":"bulk","List-Id":"<gcc-patches.gcc.gnu.org>","List-Unsubscribe":"<mailto:gcc-patches-unsubscribe-incoming=patchwork.ozlabs.org@gcc.gnu.org>","List-Archive":"<http://gcc.gnu.org/ml/gcc-patches/>","List-Post":"<mailto:gcc-patches@gcc.gnu.org>","List-Help":"<mailto:gcc-patches-help@gcc.gnu.org>","Sender":"gcc-patches-owner@gcc.gnu.org","X-Virus-Found":"No","X-Spam-SWARE-Status":"No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0,\n\tGIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD,\n\tSPF_HELO_PASS autolearn=ham version=3.3.2 spammy=","X-HELO":"mx1.redhat.com","DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 7255480476","From":"David Malcolm <dmalcolm@redhat.com>","To":"Jason Merrill <jason@redhat.com>","Cc":"Jeff Law <law@redhat.com>, gcc-patches List <gcc-patches@gcc.gnu.org>,\n\tDavid Malcolm <dmalcolm@redhat.com>","Subject":"Re: [PATCH 1/2] C++: avoid partial duplicate implementation of\n\tcp_parser_error","Date":"Thu, 12 Oct 2017 13:35:36 -0400","Message-Id":"<1507829736-15009-1-git-send-email-dmalcolm@redhat.com>","In-Reply-To":"<CADzB+2kGNTWrfynftSsz2Vdm6nMuy8sFOz-O=OSEkTJEPG2gnA@mail.gmail.com>","References":"<CADzB+2kGNTWrfynftSsz2Vdm6nMuy8sFOz-O=OSEkTJEPG2gnA@mail.gmail.com>","X-IsSubscribed":"yes"}}]