Patchwork [c++] fix PR 45332, missing semicolon after member declaration

login
register
mail settings
Submitter Nathan Froyd
Date Nov. 5, 2010, 4:13 p.m.
Message ID <20101105161332.GY26083@nightcrawler>
Download mbox | patch
Permalink /patch/70266/
State New
Headers show

Comments

Nathan Froyd - Nov. 5, 2010, 4:13 p.m.
PR 45332 points out that GCC's diagnostic for a missing semicolon after
a member declaration is not overly helpful: we don't point to the
correct location for the missing semicolon and we may issue spurious
errors due to handling the missing semicolon poorly.

The patch below attempts to address this by using the previous token as
the location for the error message; this fixes the confusing location
for the error message.  Rather than skipping to the next semicolon,
which may land us in an unexpected place, the patch also silently
assumes that the user simply missed a semicolon; this fixes the spurious
errors.

Making these changes requires a bit of adjustment to a few existing
testcases.  I think the changes are reasonable; for comparison purposes,
I've reproduced the error messages before and after for the changed
testcases.

* ext/asmspec1.c before

g++.dg/ext/asmspec1.C:6: error: expected string-literal before ‘int’
g++.dg/ext/asmspec1.C:6: error: expected ‘)’ before ‘int’
g++.dg/ext/asmspec1.C:6: error: expected ‘;’ before ‘int’
g++.dg/ext/asmspec1.C:7: error: expected string-literal before ‘int’
g++.dg/ext/asmspec1.C:7: error: expected ‘)’ before ‘int’
g++.dg/ext/asmspec1.C:7: error: expected ‘;’ before ‘int’

* ext/asmspec1.C after

g++.dg/ext/asmspec1.C:6:17: error: expected string-literal before 'int'
g++.dg/ext/asmspec1.C:6:17: error: expected ')' before 'int'
g++.dg/ext/asmspec1.C:6:16: error: expected ';' at end of member declaration
g++.dg/ext/asmspec1.C:6:20: error: expected unqualified-id before ')' token
g++.dg/ext/asmspec1.C:7:24: error: expected string-literal before 'int'
g++.dg/ext/asmspec1.C:7:24: error: expected ')' before 'int'
g++.dg/ext/asmspec1.C:7:23: error: expected ';' at end of member declaration
g++.dg/ext/asmspec1.C:7:27: error: expected unqualified-id before ')' token

* init/new13.C before

g++.dg/init/new13.C:8: error: expected type-specifier before ‘X’
g++.dg/init/new13.C:8: error: expected ‘)’ before ‘X’
g++.dg/init/new13.C:8: error: expected ‘;’ before ‘X’
g++.dg/init/new13.C:11: error: no suitable ‘operator new’ found in class ‘A’

* init/new13.C after

g++.dg/init/new13.C:8:43: error: expected type-specifier before 'X'
g++.dg/init/new13.C:8:43: error: expected ')' before 'X'
g++.dg/init/new13.C:8:42: error: expected ';' at end of member declaration
g++.dg/init/new13.C:8:43: error: 'X' does not name a type

* parse/ctor5.C before

g++.dg/parse/ctor5.C:6: error: expected ‘;’ before ‘i’
g++.dg/parse/ctor5.C:7: error: expected ‘;’ before ‘}’ token

* parse/ctor5.C after

g++.dg/parse/ctor5.C:6:5: error: expected ';' at end of member declaration
g++.dg/parse/ctor5.C:6:9: error: ISO C++ forbids declaration of 'i' with no type [-fpermissive]
g++.dg/parse/ctor5.C:6:12: error: declaration of 'int A::i()'
g++.dg/parse/ctor5.C:5:7: error: conflicts with previous declaration 'int A::i'

I think these are reasonable changes.

Tested on x86_64-unknown-linux-gnu.  OK to commit?

-Nathan

gcc/cp/
	PR c++/45332
	* parser.c (cp_lexer_previous_token): New function.
	(cp_parser_error_at_token): New function.
	(cp_parser_error): Call it.
	(cp_parser_member_declaration): Use previous token for error
	messages.  Assume semicolon presence rather than grovelling for
	the next one.

gcc/testsuite/
	PR c++/45332
	* g++.dg/parse/semicolon2.C: New testcase.
	* g++.dg/ext/asmspec1.C: Adjust.
	* g++.dg/init/new13.C: Adjust.
	* g++.dg/parse/ctor5.C: Adjust.
Jason Merrill - Nov. 5, 2010, 7:39 p.m.
On 11/05/2010 12:13 PM, Nathan Froyd wrote:
> +	      error_at (token->location,
> +			"expected %<;%>  at end of member declaration");

Did you mean for this to use cp_parser_error_at_token?

Jason

Patch

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 6302864..7efb133 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -502,6 +502,19 @@  cp_lexer_token_at (cp_lexer *lexer ATTRIBUTE_UNUSED, cp_token_position pos)
   return pos;
 }
 
+static inline cp_token *
+cp_lexer_previous_token (cp_lexer *lexer)
+{
+  cp_token_position tp;
+
+  if (lexer->next_token == &eof_token)
+    tp = lexer->last_token - 1;
+  else
+    tp = cp_lexer_token_position (lexer, true);
+
+  return cp_lexer_token_at (lexer, tp);
+}
+
 /* nonzero if we are presently saving tokens.  */
 
 static inline int
@@ -2273,21 +2286,22 @@  cp_parser_is_keyword (cp_token* token, enum rid keyword)
   return token->keyword == keyword;
 }
 
-/* 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
+/* If not parsing tenatively, issue a diagnostic of the form
+      FILE:LINE: GMSGID before TOKEN
+   where TOKEN is the next token in the input stream.  GMSGID
    (specified by the caller) is usually of the form "expected
-   OTHER-TOKEN".  */
+   OTHER-TOKEN".  LOC_TOKEN is a token from which the source
+   position of the lexer should be set.  */
 
 static void
-cp_parser_error (cp_parser* parser, const char* gmsgid)
+cp_parser_error_at_token (cp_parser* parser, const char* gmsgid,
+			  cp_token* loc_token)
 {
   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);
+
+      cp_lexer_set_source_position_from_token (loc_token);
 
       if (token->type == CPP_PRAGMA)
 	{
@@ -2306,6 +2320,23 @@  cp_parser_error (cp_parser* parser, const char* gmsgid)
     }
 }
 
+/* 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_token *token = cp_lexer_peek_token (parser->lexer);
+
+      cp_parser_error_at_token (parser, gmsgid, token);
+    }
+}
+
 /* 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
@@ -17627,6 +17658,8 @@  cp_parser_member_declaration (cp_parser* parser)
     }
   else
     {
+      bool assume_semicolon = false;
+
       /* See if these declarations will be friends.  */
       friend_p = cp_parser_friend_p (&decl_specifiers);
 
@@ -17820,11 +17853,18 @@  cp_parser_member_declaration (cp_parser* parser)
 	  else if (cp_lexer_next_token_is_not (parser->lexer,
 					       CPP_SEMICOLON))
 	    {
-	      cp_parser_error (parser, "expected %<;%>");
-	      /* Skip tokens until we find a `;'.  */
-	      cp_parser_skip_to_end_of_statement (parser);
+	      /* The next token might be a ways away from where the
+		 actual semicolon is missing.  Find the previous token
+		 and use that for our error position.  */
+	      cp_token *token = cp_lexer_previous_token (parser->lexer);
+	      error_at (token->location,
+			"expected %<;%> at end of member declaration");
 
-	      break;
+	      /* Assume that the user meant to provide a semicolon.  If
+		 we were to cp_parser_skip_to_end_of_statement, we might
+		 skip to a semicolon inside a member function definition
+		 and issue nonsensical error messages.  */
+	      assume_semicolon = true;
 	    }
 
 	  if (decl)
@@ -17836,6 +17876,9 @@  cp_parser_member_declaration (cp_parser* parser)
 	      if (TREE_CODE (decl) == FUNCTION_DECL)
 		cp_parser_save_default_args (parser, decl);
 	    }
+
+	  if (assume_semicolon)
+	    return;
 	}
     }
 
diff --git a/gcc/testsuite/g++.dg/ext/asmspec1.C b/gcc/testsuite/g++.dg/ext/asmspec1.C
index 3df2483..0661136 100644
--- a/gcc/testsuite/g++.dg/ext/asmspec1.C
+++ b/gcc/testsuite/g++.dg/ext/asmspec1.C
@@ -3,6 +3,6 @@ 
 
 struct A
 {
-  int i __asm__(int);         // { dg-error "before" }
-  static int j __asm__(int);  // { dg-error "before" }
+  int i __asm__(int);         // { dg-error "expected" }
+  static int j __asm__(int);  // { dg-error "expected" }
 };
diff --git a/gcc/testsuite/g++.dg/init/new13.C b/gcc/testsuite/g++.dg/init/new13.C
index 3563c48..2ced6e3 100644
--- a/gcc/testsuite/g++.dg/init/new13.C
+++ b/gcc/testsuite/g++.dg/init/new13.C
@@ -5,7 +5,7 @@ 
 
 struct A
 {
-  void* operator new(__SIZE_TYPE__) throw(X);  // { dg-error "" }
+  void* operator new(__SIZE_TYPE__) throw(X);  // { dg-error "expected|type" }
 };
 
-A* p = new A;                                  // { dg-error "no suitable" }
+A* p = new A;
diff --git a/gcc/testsuite/g++.dg/parse/ctor5.C b/gcc/testsuite/g++.dg/parse/ctor5.C
index 8194585..3ea2354 100644
--- a/gcc/testsuite/g++.dg/parse/ctor5.C
+++ b/gcc/testsuite/g++.dg/parse/ctor5.C
@@ -2,9 +2,9 @@ 
 
 struct A
 {
-  int i;
-  A() i() {}  // { dg-error "expected" }
-}; // { dg-error "expected" }
+  int i; // { dg-error "conflicts" }
+  A() i() {}  // { dg-error "declaration" }
+};
 
 struct B
 {
diff --git a/gcc/testsuite/g++.dg/parse/semicolon2.C b/gcc/testsuite/g++.dg/parse/semicolon2.C
new file mode 100644
index 0000000..50c3ce1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/semicolon2.C
@@ -0,0 +1,9 @@ 
+// PR c++/45332
+// { dg-do compile }
+
+class C
+{
+ int x				// { dg-error "expected" }
+
+ const int foo() { return x; }
+};