Re: {maint} yacc.c: initialize yylval in pure-parser mode

[Akim Demaille]

Hi Paul,

Sorry for the delays on this patch.

Le 21 sept. 2012 à 19:13, Paul Eggert a écrit :

> On 09/21/2012 01:43 AM, Akim Demaille wrote:
>> The idea is nice, but there are several issues.
> 
> OK, how about the following patch to the trunk?
> It attempts to address the issues you mentioned.
> 
> diff --git a/data/yacc.c b/data/yacc.c
> index a15ef77..f498867 100644
> --- a/data/yacc.c
> +++ b/data/yacc.c
> @@ -1469,7 +1469,21 @@ m4_ifdef([b4_dollar_dollar_used],[[  yyvsp[0] = yylval;
> ]])dnl
> m4_ifdef([b4_at_dollar_used], [[  yylsp[0] = yylloc;
> ]])])dnl
> -[  goto yysetstate;
> +[  if (yychar != YYEMPTY || yyss + yystacksize - 1 <= yyssp)
> +    goto yysetstate;
> +
> +  *yyssp = yystate;
> +
> +  YYDPRINTF ((stderr, "Entering state %d\n", yystate));
> +
> +  if (yystate == YYFINAL)
> +    YYACCEPT;
> +
> +  yyn = yypact[yystate];
> +  if (yypact_value_is_default (yyn))
> +    goto yydefault;
> +
> +  goto yyread_token;
> 
> /*------------------------------------------------------------.
> | yynewstate -- Push a new state, which is found in yystate.  |
> @@ -1572,7 +1586,8 @@ yybackup:
> 
>   /* YYCHAR is either YYEMPTY or YYEOF or a valid lookahead symbol.  */
>   if (yychar == YYEMPTY)
> -    {]b4_push_if([[
> +    {
> +yyread_token:]b4_push_if([[

OMG, jumping inside an if :)  Not yet the Duff device,
but already quite, err, unusual.

>       if (!yyps->yynew)
>         {]b4_use_push_for_pull_if([], [[
>           YYDPRINTF ((stderr, "Return for a new token:\n"));]])[
> 

I confess I have the same resistance to this patch: it adds
complexity (a lot of complexity actually, these lines are far
from being trivial and are deeply connected to the interpretation
of the tables) to the parser, and makes it more different than
what lalr1.cc and lalr1.java are for instance.  Even glr.c
initializes yylval the same way as the one I used in my proposal
for yacc.c.

In fact, one of your initial goal was:

> And in the unusual case where
> where yylval really *isn't* initialized -- because the lexer is
> buggy -- GCC will issue the warning, which will be a good thing.

AFAICT, it is not the case, GCC does not complain in the following
attached test case where yylval is _not_ initialized.

foo.y
Description: Binary data

So really, I still prefer my proposal which is way simpler,
I think.  Again, _even_ if GCC were able to catch this first
initial yylval not being initialized, it is a very weak warning,
as there are many more tokens to come, whose value will not
be checked.  This is more a task for tools like valgrind.
What would be useful would be to add instrumentation
to "void" the content of yylval before we call yylex, so
that valgrind could check that the value is properly updated
(but that would not work for valueless tokens, so scratch it).