Re: [PATCH,c] better recovery from missing semicolon after declaration

[Nathan Froyd <froydnj at codesourcery dot com>]

On Wed, Nov 17, 2010 at 05:17:20PM +0100, Paolo Bonzini wrote:
> On 11/17/2010 04:33 PM, Nathan Froyd wrote:
> >I forgot that one testcase needed some adjustments.  The patch below
> >deletes some dg-error messages since for the opening line of the
> >testcase:
> >
> >typedef BYTE unsigned char;	/* { dg-error "expected" } */
> >
> >we now treat BYTE as a type.  I think this is an improvement, albeit an
> >unexpected one.
> 
> In other words, I'm worried that it becomes harder to improve this
> tescase:
> 
>   typedef unsigned short uint16_t;
> 
>   /* should warn about unknown type name "uintt16_t", currently gives
>      a parse error ("expected ... before pid_t") because unknown type
>      names are not guessed in c_parser_declspecs.  */
>   typedef uintt16_t pid_t;
> 
>   /* no error expected about unknown type name; currently fails */
>   pid_t xyz;
> 
> Given my recent experience with adding unknown type name errors, it
> should not be hard to fix the above (just lack of time on my part),
> for example by moving this into c_parser_next_token_starts_typename:
> 
>   /* Try a bit harder to detect an unknown typename.  */
>   if (token->type == CPP_NAME
>       && token->id_kind == C_ID_ID
>       && (c_parser_peek_2nd_token (parser)->type == CPP_NAME
>           || c_parser_peek_2nd_token (parser)->type == CPP_MULT)
>       && !lookup_name (token->value)
> 
>       /* Do not try too hard when we could have "object in array".  */
>       && !parser->objc_could_be_foreach_context)
>     return true;
> 
> Maybe you can give this a shot before applying these patches?

I tried the example above with and without my patch and with and without
modifying c_parser_next_token_starts_typename like so:

static inline bool
c_parser_next_token_starts_typename (c_parser *parser)
{
  c_token *token = c_parser_peek_token (parser);

  /* Try a bit harder to detect an unknown typename.  */
  if (token->type == CPP_NAME
      && token->id_kind == C_ID_ID
      && (c_parser_peek_2nd_token (parser)->type == CPP_NAME
          || c_parser_peek_2nd_token (parser)->type == CPP_MULT)
      && !lookup_name (token->value)
      /* Do not try too hard when we could have "object in array".  */
      && !parser->objc_could_be_foreach_context)
    return true;

  return c_token_starts_typename (token);
}

and with:

static inline bool
c_parser_next_token_starts_typename (c_parser *parser)
{
  c_token *token = c_parser_peek_token (parser);

  if (c_token_starts_typename (token))
    return true;

  /* Try a bit harder to detect an unknown typename.  */
  if (token->type == CPP_NAME
      && token->id_kind == C_ID_ID
      && (c_parser_peek_2nd_token (parser)->type == CPP_NAME
          || c_parser_peek_2nd_token (parser)->type == CPP_MULT)
      && !lookup_name (token->value)
      /* Do not try too hard when we could have "object in array".  */
      && !parser->objc_could_be_foreach_context)
    return true;

  return false;
}

since I wasn't sure which version you meant.  (I think the latter, for
agreement with c_parser_next_tokens_start_declaration.)  The error
message in all cases is:

/home/froydnj/src/paolo.c:7:21: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'pid_t'
/home/froydnj/src/paolo.c:11:3: error: unknown type name 'pid_t'

so my patch doesn't seem to have much effect on this testcase.  I think
the saving grace on your testcase is that in:

  typedef uintt16_t pid_t;

`pid_t' is not a keyword, so we don't stop parsing early, but I haven't
traced through the parser to verify that.

-Nathan