Re: [PATCH] Better error messages for merge-conflict markers (v3)

[Martin Sebor <msebor@gmail.com>]

> @@ -471,6 +471,20 @@ c_parser_peek_2nd_token (c_parser *parser)
>     return &parser->tokens[1];
>   }
>
> +/* Return a pointer to the Nth token from PARSER, reading it
> +   in if necessary.  The N-1th token is already read in.  */
> +
> +static c_token *
> +c_parser_peek_nth_token (c_parser *parser, unsigned int n)
> +{
> +  if (parser->tokens_avail >= n)
> +    return &parser->tokens[n - 1];
> +  gcc_assert (parser->tokens_avail == n - 1);
> +  c_lex_one_token (parser, &parser->tokens[n - 1]);
> +  parser->tokens_avail = n;
> +  return &parser->tokens[n - 1];
> +}

David, I know little about the code in this area and so I looked
at the patch mostly out of curiosity.  This little function caught
my eye for some reason.  I see it's called only in two places and
in a safe way, but it also looks like it could easily be called
unsafely and cause either the assert to fire (when N is greater
than tokens_avail + 1), or a bad address to be returned (when N
is zero).  It's also a third peek function in the C parser,
making the choice not completely trivial (at least to those not
as familiar with the code).

When compared to the equivalent function in the C++ lexer, that
one is more like I would expect.  I.e., it asserts that N is
positive and doesn't assume any specific prior sequence of peeks.
I can see how someone familiar with the C++ lexer but not so well
with the C parser might inadvertently use the new C function
incorrectly.

May I suggest making the C function equivalent to the C++ one
in terms of its preconditions?

Martin