Re: [patch #9620] spurious compiler warning: "potential null pointer dereference"

[Frank Heckenbach]

Akim Demaille wrote:

> > The generated parser produces a GCC warning with "-Wnull-dereference":
> > 
> >  parse.cpp: error: potential null pointer dereference
> > [-Werror=null-dereference]
> > 
> > The reason is:
> > 
> >    char const* yyformat = YY_NULLPTR;
> > 
> > before it's set in a switch for cases 0 to 5. Now, this seems in fact 
> > correct
> > due to "YYERROR_VERBOSE_ARGS_MAXIMUM = 5" and the way the code finally gets
> > there. But that's a bit much to expect the compiler to recognize.
> > 
> > Actually I do find the code rather fragile; the definition of
> > YYERROR_VERBOSE_ARGS_MAXIMUM does not even have a comment pointing out the
> > ramifications of changing it.
> 
> Agreed.  That's also why actually instead of your approach, I would
> prefer a `default: abort()`.
> 
> > I also find the code a bit strange at all; why have a number of format 
> > strings
> > that differ only in the number of "or %s" parts, and which must all be
> > translated individually? Rather than adding repeated parts in a loop, or 
> > using
> > a more flexible wording such as "expecing one of the following: « ?
> 
> Because of internationalization: you don't know how it would be
> translated, how punctuation would be, etc.

In my suggestion only the text "expecing one of the following:"
would need to be translated. It would be followed by a list of
tokens only, with no more text. Since token names themselves are not
translated either AFAIK, the separator doesn't need to be (i.e.,
even if some language uses some other than commas to separate lists,
it would be strange to use them in a list of English token names).
If that's really problematic, it will be hard for internationalized
programs to output any lists at all ...

> > Anyway, this patch does just the minimum necessary to avoid the warning (and
> > make the code more robust in case someone changes
> > YYERROR_VERBOSE_ARGS_MAXIMUM), by using "default" instead of "case 0 ».
> 
> But it would be incorrect anyway.

I think not strictly incorrect. It would just default to outputting
no token names (as it's currently supposed to do in this case,
anyway).

> Would `default: abort` suit you?

Mostly what bothers me is the compiler warning, so anything that
avoids that is basically fine with me.

Regards,
Frank