Patchwork Better error reporting for missing semicolons after a struct definition

login
register
mail settings
Submitter Paolo Bonzini
Date Nov. 13, 2010, 10:25 a.m.
Message ID <4CDE678B.6060109@gnu.org>
Download mbox | patch
Permalink /patch/71042/
State New
Headers show

Comments

Paolo Bonzini - Nov. 13, 2010, 10:25 a.m.
On 11/10/2010 03:05 AM, Joseph S. Myers wrote:
> Perhaps you could post an updated version of your patch to improve
> diagnostics for missing semicolons, relative to current trunk?

Here it is, bootstrap/regtest in progress.

Paolo
2010-11-13  Paolo Bonzini  <bonzini@gnu.org>

        * c-parser.c (c_token_is_qualifier,
        c_parser_next_token_is_qualifier): New.
        (c_parser_declaration_or_fndef, c_parser_struct_declaration):
        Improve error message on ctsk_tagdef typespecs.
        (c_parser_struct_or_union_specifier): Improve error recovery.

2010-11-13  Paolo Bonzini  <bonzini@gnu.org>

        * gcc.dg/two-types-1.c: New test.
        * gcc.dg/two-types-2.c: New test.
        * gcc.dg/two-types-3.c: New test.
        * gcc.dg/two-types-4.c: New test.
        * gcc.dg/two-types-5.c: New test.
        * gcc.dg/two-types-6.c: New test.
        * gcc.dg/two-types-7.c: New test.
        * gcc.dg/two-types-8.c: New test.
        * gcc.dg/two-types-9.c: New test.
        * gcc.dg/two-types-10.c: New test.
        * objc.dg/two-types-1.m: New test.
Joseph S. Myers - Nov. 15, 2010, 4:37 p.m.
On Sat, 13 Nov 2010, Paolo Bonzini wrote:

> On 11/10/2010 03:05 AM, Joseph S. Myers wrote:
> > Perhaps you could post an updated version of your patch to improve
> > diagnostics for missing semicolons, relative to current trunk?
> 
> Here it is, bootstrap/regtest in progress.

This is OK if it passed the testing with no regressions.

Patch

Index: c-parser.c
===================================================================
--- c-parser.c	(revision 166700)
+++ c-parser.c	(working copy)
@@ -506,6 +506,47 @@  c_parser_next_token_starts_typename (c_p
   return c_token_starts_typename (token);
 }
 
+/* Return true if TOKEN is a type qualifier, false otherwise.  */
+static bool
+c_token_is_qualifier (c_token *token)
+{
+  switch (token->type)
+    {
+    case CPP_NAME:
+      switch (token->id_kind)
+	{
+	case C_ID_ADDRSPACE:
+	  return true;
+	default:
+	  return false;
+	}
+    case CPP_KEYWORD:
+      switch (token->keyword)
+	{
+	case RID_CONST:
+	case RID_VOLATILE:
+	case RID_RESTRICT:
+	case RID_ATTRIBUTE:
+	  return true;
+	default:
+	  return false;
+	}
+    case CPP_LESS:
+      return false;
+    default:
+      gcc_unreachable ();
+    }
+}
+
+/* Return true if the next token from PARSER is a type qualifier,
+   false otherwise.  */
+static inline bool
+c_parser_next_token_is_qualifier (c_parser *parser)
+{
+  c_token *token = c_parser_peek_token (parser);
+  return c_token_is_qualifier (token);
+}
+
 /* Return true if TOKEN can start declaration specifiers, false
    otherwise.  */
 static bool
@@ -1404,6 +1445,19 @@  c_parser_declaration_or_fndef (c_parser 
       c_parser_consume_token (parser);
       return;
     }
+
+  /* Provide better error recovery.  Note that a type name here is usually
+     better diagnosed as a redeclaration.  */
+  if (empty_ok
+      && specs->typespec_kind == ctsk_tagdef
+      && c_parser_next_token_starts_declspecs (parser)
+      && !c_parser_next_token_is (parser, CPP_NAME))
+    {
+      c_parser_error (parser, "expected %<;%>, identifier or %<(%>");
+      parser->error = false;
+      shadow_tag_warned (specs, 1);
+      return;
+    }
   else if (c_dialect_objc ())
     {
       /* Prefix attributes are an error on method decls.  */
@@ -1867,13 +1921,31 @@  c_parser_declspecs (c_parser *parser, st
 {
   bool attrs_ok = start_attr_ok;
   bool seen_type = specs->typespec_kind != ctsk_none;
-  while (c_parser_next_token_is (parser, CPP_NAME)
+  while ((c_parser_next_token_is (parser, CPP_NAME)
+	  && c_parser_peek_token (parser)->id_kind != C_ID_ID)
 	 || c_parser_next_token_is (parser, CPP_KEYWORD)
 	 || (c_dialect_objc () && c_parser_next_token_is (parser, CPP_LESS)))
     {
       struct c_typespec t;
       tree attrs;
       location_t loc = c_parser_peek_token (parser)->location;
+
+      if (!c_parser_next_token_is_qualifier (parser))
+        {
+	  /* Exit for TYPENAMEs after any type because they can appear as a
+	     field name.  */
+          if (seen_type && c_parser_next_token_is (parser, CPP_NAME))
+            break;
+
+          /* If we cannot accept a type, and the next token must start one,
+	     exit.  Do the same if we already have seen a tagged definition,
+	     since it would be an error anyway and likely the user has simply
+	     forgotten a semicolon.  */
+          if ((!typespec_ok || specs->typespec_kind == ctsk_tagdef)
+	      && c_parser_next_token_starts_typename (parser))
+            break;
+        }
+
       if (c_parser_next_token_is (parser, CPP_NAME))
 	{
 	  tree value = c_parser_peek_token (parser)->value;
@@ -1889,12 +1961,7 @@  c_parser_declspecs (c_parser *parser, st
 	      continue;
 	    }
 
-	  /* This finishes the specifiers unless a type name is OK, it
-	     is declared as a type name and a type name hasn't yet
-	     been seen.  */
-	  if (!typespec_ok || seen_type
-	      || (kind != C_ID_TYPENAME && kind != C_ID_CLASSNAME))
-	    break;
+	  /* Now at a C_ID_TYPENAME or C_ID_CLASSNAME.  */
 	  c_parser_consume_token (parser);
 	  seen_type = true;
 	  attrs_ok = true;
@@ -2335,12 +2402,17 @@  c_parser_struct_or_union_specifier (c_pa
 	      if (c_parser_next_token_is (parser, CPP_CLOSE_BRACE))
 		pedwarn (c_parser_peek_token (parser)->location, 0,
 			 "no semicolon at end of struct or union");
-	      else
+	      else if (parser->error
+		       || !c_parser_next_token_starts_declspecs (parser))
 		{
 		  c_parser_error (parser, "expected %<;%>");
 		  c_parser_skip_until_found (parser, CPP_CLOSE_BRACE, NULL);
 		  break;
 		}
+
+	      /* If we come here, we have already emitted an error
+		 for an expected `;', identifier or `(', and we also
+	         recovered already.  Go on with the next field. */
 	    }
 	}
       postfix_attrs = c_parser_attributes (parser);
@@ -2456,6 +2528,19 @@  c_parser_struct_declaration (c_parser *p
 	}
       return ret;
     }
+
+  /* Provide better error recovery.  Note that a type name here is valid,
+     and will be treated as a field name.  */
+  if (specs->typespec_kind == ctsk_tagdef
+      && TREE_CODE (specs->type) != ENUMERAL_TYPE
+      && c_parser_next_token_starts_declspecs (parser)
+      && !c_parser_next_token_is (parser, CPP_NAME))
+    {
+      c_parser_error (parser, "expected %<;%>, identifier or %<(%>");
+      parser->error = false;
+      return NULL_TREE;
+    }
+
   pending_xref_error ();
   prefix_attrs = specs->attrs;
   all_prefix_attrs = prefix_attrs;
Index: testsuite/gcc.dg/two-types-4.c
===================================================================
--- testsuite/gcc.dg/two-types-4.c	(revision 0)
+++ testsuite/gcc.dg/two-types-4.c	(revision 0)
@@ -0,0 +1,9 @@ 
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+int f()
+{
+  struct f {
+  }
+  int z; /* { dg-error "expected ';', identifier or " "" } */
+}
Index: testsuite/gcc.dg/two-types-8.c
===================================================================
--- testsuite/gcc.dg/two-types-8.c	(revision 0)
+++ testsuite/gcc.dg/two-types-8.c	(revision 0)
@@ -0,0 +1,10 @@ 
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+enum x { XYZ }
+struct g { enum x a; }; /* { dg-error "expected ';', identifier or " "" } */
+
+int f(struct g *x)
+{
+  return x->a == XYZ; /* { dg-bogus " has no member " "" } */
+}
Index: testsuite/gcc.dg/two-types-1.c
===================================================================
--- testsuite/gcc.dg/two-types-1.c	(revision 0)
+++ testsuite/gcc.dg/two-types-1.c	(revision 0)
@@ -0,0 +1,5 @@ 
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+typedef int x, y;
+x y z;			/* { dg-error "" "" } */
Index: testsuite/gcc.dg/two-types-5.c
===================================================================
--- testsuite/gcc.dg/two-types-5.c	(revision 0)
+++ testsuite/gcc.dg/two-types-5.c	(revision 0)
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+struct f {}
+struct g {} /* { dg-error "expected ';', identifier or " "" } */
+int f(); /* { dg-error "expected ';', identifier or " "" } */
Index: testsuite/gcc.dg/two-types-9.c
===================================================================
--- testsuite/gcc.dg/two-types-9.c	(revision 0)
+++ testsuite/gcc.dg/two-types-9.c	(revision 0)
@@ -0,0 +1,10 @@ 
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+struct f {}
+static int a, b; /* { dg-error "expected ';', identifier or " "" } */
+
+int f()
+{
+	return a - b; /* { dg-bogus "invalid operands " "" } */
+}
Index: testsuite/gcc.dg/two-types-10.c
===================================================================
--- testsuite/gcc.dg/two-types-10.c	(revision 0)
+++ testsuite/gcc.dg/two-types-10.c	(revision 0)
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+typedef enum a { XYZ } a; /* { dg-message "previous declaration" } */
+enum a a;	/* { dg-error "redeclared" } */
+struct b { enum a a : 8; };
Index: testsuite/gcc.dg/two-types-2.c
===================================================================
--- testsuite/gcc.dg/two-types-2.c	(revision 0)
+++ testsuite/gcc.dg/two-types-2.c	(revision 0)
@@ -0,0 +1,5 @@ 
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+typedef int x, y;
+x struct f z; /* { dg-error "two or more " "" } */
Index: testsuite/gcc.dg/two-types-6.c
===================================================================
--- testsuite/gcc.dg/two-types-6.c	(revision 0)
+++ testsuite/gcc.dg/two-types-6.c	(revision 0)
@@ -0,0 +1,7 @@ 
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+struct s {
+  struct f {} /* dg-warning "does not declare anything" "" } */
+  struct g {} x; /* { dg-error "expected ';', identifier or " "" } */
+};
Index: testsuite/gcc.dg/two-types-3.c
===================================================================
--- testsuite/gcc.dg/two-types-3.c	(revision 0)
+++ testsuite/gcc.dg/two-types-3.c	(revision 0)
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+struct f {
+}
+int z(); /* { dg-error "expected ';', identifier or " ""  { target *-*-* }  } */
Index: testsuite/gcc.dg/two-types-7.c
===================================================================
--- testsuite/gcc.dg/two-types-7.c	(revision 0)
+++ testsuite/gcc.dg/two-types-7.c	(revision 0)
@@ -0,0 +1,8 @@ 
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+struct s {
+  struct f {}
+  enum a { X } /* { dg-error "expected ';', identifier or " "" } */
+  struct g {} /* { dg-error "expected identifier " "" } */
+}; /* { dg-warning "no semicolon" "" } */
Index: testsuite/objc.dg/two-types-1.m
===================================================================
--- testsuite/objc.dg/two-types-1.m	(revision 0)
+++ testsuite/objc.dg/two-types-1.m	(revision 0)
@@ -0,0 +1,15 @@ 
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+@interface foo
+struct f {}
+struct g { int a; }; /* { dg-error "expected ';', identifier or " "" } */
+
+- (struct f *) a;
+- (struct g *) b;
+@end
+
+int f(struct g *x)
+{
+  return x->a; /* { dg-bogus " has no member " "" } */
+}