[PATCH 1/2] C++: avoid partial duplicate implementation of cp_parser_error

David Malcolm dmalcolm@redhat.com
Thu Oct 12 18:10:00 GMT 2017


On Wed, 2017-10-11 at 17:18 -0400, Jason Merrill wrote:
> On Tue, Sep 26, 2017 at 9:56 AM, David Malcolm <dmalcolm@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



More information about the Gcc-patches mailing list