diff mbox

[C] Fix pr44517

Message ID 4C21D421.8070004@oracle.com
State New
Headers show

Commit Message

Shujing Zhao June 23, 2010, 9:30 a.m. UTC 
On 06/23/2010 04:56 PM, Manuel López-Ibáñez wrote:
> Nice improvement! Some unimportant comments:
> 
> @@ -2843,8 +2828,12 @@ c_parser_parameter_declaration (c_parser
>      {
>        /* ??? In some Objective-C cases '...' isn't applicable so there
>  	 should be a different message.  */
> -      c_parser_error (parser,
> -		      "expected declaration specifiers or %<...%>");
> +      c_token *token = c_parser_peek_token (parser);
> +      if (parser->error)
> +	return NULL;
> +      parser->error = true;
> +      c_parser_set_source_position_from_token (token);
> +      error ("unknown type name %qE", token->value);
>        c_parser_skip_to_end_of_parameter (parser);
>        return NULL;
>      }
> 
> The ??? comment does not make sense anymore (and without any example
> wasn't very useful to start with), I would propose to remove it. 
Yes, you are right. Remove it from the updated patch.
BTW,
> do you know that there is a error_at (LOCATION) function? I am not
> sure whether here using that is more correct (or efficient) than
> c_parser_set_source_position_from_token but just to let you know for
> future patches.
> 
If it uses error_at, the code would be changed from

  +      c_parser_set_source_position_from_token (token);
  +      error ("unknown type name %qE", token->value);

to

+ if (token->type != CPP_EOF)
+  {
+    error_at (token->location, "unknown type name %qE", token->value);
+  }
+ else
+   error ("unknown type name %qE", token->value);

I think using c_parser_set_source_position_from_token is better.

> 
> +/* PR c/44517: Improve diagnostic for misspelled typename in function
> declaration. */
> +int foo(int x, pid_t y, long z, in t) {  /* { dg-error "unknown type
> name.*pid_t|unknown type name.*in" } */
> +  return x + y + z + t;
> +}
> +
> +int bar(int x, lon y, long z, ...){ /* { dg-error "unknown type name"
> { target *-*-* } 8 } */
> +  return;
> +}
> +
> +void foo(int n, int a[n], pid_t x); /* { dg-error "unknown type name"
> { target *-*-* } 12 } */
> +void bar() {};
> 
> What is the default dg action for tests without dg-do directive? Also,
> I am not sure how these directives are working correctly, because the
> complete format is:
> 
> { dg-error PATTERN COMMENT { TARGET_SPEC } LINE }
> 
> so the comment is missing. As far as I know, you can drop arguments
> from right to left but not, for example, put a LINE and not a TARGET
> or COMMENT. (Unfortunate, because LINE is probably the second most
> used after PATTERN). BTW, you do not need LINE if the error is given
> in the same line as the directive dg-error.

Thanks. I also wonder how they are passed? Maybe it is caused the ".*" is used 
at the first dg-error directive. Using "'" directly to quote the unknown type 
name solves this issue.

Thanks
Pearly
diff mbox

Patch

Index: c-parser.c
===================================================================
--- c-parser.c	(revision 160889)
+++ c-parser.c	(working copy)
@@ -2706,7 +2706,7 @@  c_parser_parms_declarator (c_parser *par
 static struct c_arg_info *
 c_parser_parms_list_declarator (c_parser *parser, tree attrs)
 {
-  bool good_parm = false;
+  bool bad_parm = false;
   /* ??? Following the old parser, forward parameter declarations may
      use abstract declarators, and if no real parameter declarations
      follow the forward declarations then this is not diagnosed.  Also
@@ -2758,11 +2758,10 @@  c_parser_parms_list_declarator (c_parser
       /* Parse a parameter.  */
       struct c_parm *parm = c_parser_parameter_declaration (parser, attrs);
       attrs = NULL_TREE;
-      if (parm != NULL)
-	{
-	  good_parm = true;
-	  push_parm_decl (parm);
-	}
+      if (parm == NULL)
+	bad_parm = true;
+      else
+	push_parm_decl (parm);
       if (c_parser_next_token_is (parser, CPP_SEMICOLON))
 	{
 	  tree new_attrs;
@@ -2774,20 +2773,13 @@  c_parser_parms_list_declarator (c_parser
       if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN))
 	{
 	  c_parser_consume_token (parser);
-	  if (good_parm)
-	    return get_parm_info (false);
-	  else
+	  if (bad_parm)
 	    {
-	      struct c_arg_info *ret
-		= XOBNEW (&parser_obstack, struct c_arg_info);
-	      ret->parms = 0;
-	      ret->tags = 0;
-	      ret->types = 0;
-	      ret->others = 0;
-	      ret->pending_sizes = 0;
-	      ret->had_vla_unspec = 0;
-	      return ret;
+	      get_pending_sizes ();
+	      return NULL;
 	    }
+	  else
+	    return get_parm_info (false);
 	}
       if (!c_parser_require (parser, CPP_COMMA,
 			     "expected %<;%>, %<,%> or %<)%>"))
@@ -2802,20 +2794,13 @@  c_parser_parms_list_declarator (c_parser
 	  if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN))
 	    {
 	      c_parser_consume_token (parser);
-	      if (good_parm)
-		return get_parm_info (true);
-	      else
+	      if (bad_parm)
 		{
-		  struct c_arg_info *ret
-		    = XOBNEW (&parser_obstack, struct c_arg_info);
-		  ret->parms = 0;
-		  ret->tags = 0;
-		  ret->types = 0;
-		  ret->others = 0;
-		  ret->pending_sizes = 0;
-		  ret->had_vla_unspec = 0;
-		  return ret;
+		  get_pending_sizes ();
+		  return NULL;
 		}
+	      else
+		return get_parm_info (true);
 	    }
 	  else
 	    {
@@ -2841,10 +2826,12 @@  c_parser_parameter_declaration (c_parser
   bool dummy = false;
   if (!c_parser_next_token_starts_declspecs (parser))
     {
-      /* ??? In some Objective-C cases '...' isn't applicable so there
-	 should be a different message.  */
-      c_parser_error (parser,
-		      "expected declaration specifiers or %<...%>");
+      c_token *token = c_parser_peek_token (parser);
+      if (parser->error)
+	return NULL;
+      parser->error = true;
+      c_parser_set_source_position_from_token (token);
+      error ("unknown type name %qE", token->value);
       c_parser_skip_to_end_of_parameter (parser);
       return NULL;
     }
Index: testsuite/gcc.dg/noncompile/pr44517.c
===================================================================
--- testsuite/gcc.dg/noncompile/pr44517.c	(revision 0)
+++ testsuite/gcc.dg/noncompile/pr44517.c	(revision 0)
@@ -0,0 +1,11 @@ 
+/* PR c/44517: Improve diagnostic for misspelled typename in function declaration. */
+int foo(int x, pid_t y, long z, in t) {  /* { dg-error "unknown type name 'pid_t'|unknown type name 'in'" } */
+  return x + y + z + t;
+}
+
+int bar(int x, lon y, long z, ...){ /* { dg-error "unknown type name 'lon'" } */
+  return;
+}
+
+void foo(int n, int a[n], pid_t x); /* { dg-error "unknown type name 'pid_t'" } */
+void bar() {};
Index: testsuite/gcc.dg/noncompile/990416-1.c
===================================================================
--- testsuite/gcc.dg/noncompile/990416-1.c	(revision 160889)
+++ testsuite/gcc.dg/noncompile/990416-1.c	(working copy)
@@ -2,11 +2,11 @@  extern void *memcpy (void *, const void 
 typedef int word_type;
    
 static void
-copy_reg (unsigned int reg, frame_state *udata,	/* { dg-error "parse|syntax|expected" } */
-	  frame_state *target_udata)	/* { dg-error "expected" } */
+copy_reg (unsigned int reg, frame_state *udata,	/* { dg-error "unknown type name" } */
+	  frame_state *target_udata)	/* { dg-error "unknown type name" } */
 {  
-  word_type *preg = get_reg_addr (reg, udata, 0);	/* { dg-error "undeclared|function|without a cast" } */
-  word_type *ptreg = get_reg_addr (reg, target_udata, 0); /* { dg-error "undeclared|without a cast" } */
+  word_type *preg = ge_reg_addr (reg, udata, 0);
+  word_type *ptreg = ge_reg_addr (reg, target_udata, 0);
    
   memcpy (ptreg, preg, __builtin_dwarf_reg_size (reg));
 }