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