Re: push parser

[Bob Rossi]

On Tue, Oct 10, 2006 at 03:19:51PM -0700, Paul Eggert wrote:
> Bob Rossi <address@hidden> writes:
> 
> > --- yacc.c  2006-10-10 14:06:22.000000000 -0400
> > +++ push.c  2006-10-10 10:49:23.000000000 -0400
> >     @@ -152,6 +152,9 @@
> >      m4_if(b4_prefix, [yy], [],
> >      [/* Substitute the variable and function names.  */
> >      #define yyparse b4_prefix[]parse
> >     +#define yypushparse b4_prefix[]pushparse
> >     +#define yypvarsinit b4_prefix[]pvarsinit
> >     +#define yypvars b4_prefix[]pvars
> >      #define yylex   b4_prefix[]lex
> >      #define yyerror b4_prefix[]error
> >      #define yylval  b4_prefix[]lval
> >
> > These are 3 new symbols that get defined depending on the prefix.
> 
> Are these symbols needed if it's not a push parser?  If not, perhaps
> they should be protected with b4_push_if.  Similarly for any other
> changes needed only for the push parser.

No, they are not, I will wrap them in b4_push_if. I thought I was 
pretty careful about this sort of thing.

> >     @@ -1009,6 +1121,7 @@
> >      #if YYERROR_VERBOSE
> >        /* Buffer for error messages, and its allocated size.  */
> >        char yymsgbuf[128];
> >     +  char *yymsgbuf_ptr = yymsgbuf;
> >        char *yymsg = yymsgbuf;
> >        YYSIZE_T yymsg_alloc = sizeof yymsgbuf;
> >      #endif
> >
> > yymsgbuf_ptr is added. This is probably the first confusing difference
> > between the 2 patches. You will see this solution used in at least 2 
> > other places. Essentially, yacc.c was using yymsgbuf in yyparse. So,
> > the yypvars structure also has a 'char yymsgbuf[128];'. However, when
> > the pull parse is being used, it wants yymsgbuf to refer to the local 
> > stack variable, and when the push parser is being used, it wants 
> > yymsgbuf to refer to the variable in the yypvars structure.
> 
> I don't see why these changes are needed.  Why can't the push parser
> use a local variable, just as the pull parser does?  The buffer in
> question is needed only for the call to yysyntax_error.  Its storage
> does not need to survive until the next call to yypushparse.

Honestly, I did this out of ignorance. Which is why I gave it such a
long description above. Are you suggesting I could take the 
'char yymsgbuf[128];' out of the 'struct yypvars' and simply use the
one on the stack that appears each time yypushparse is called?
If so, I'll provide a patch with these changes.

> >     @@ -1023,6 +1136,7 @@
> >      
> >        /* The state stack.  */
> >        yytype_int16 yyssa[YYINITDEPTH];
> >     +  yytype_int16 *yyssa_ptr = yyssa;
> >        yytype_int16 *yyss = yyssa;
> >        yytype_int16 *yyssp;
> >
> > Same idea as above here.
> >  
> >     @@ -1037,7 +1151,9 @@
> >        YYLTYPE *yyls = yylsa;
> >        YYLTYPE *yylsp;
> >        /* The locations where the error started and ended.  */
> >     -  YYLTYPE yyerror_range[2];]])[
> >     +  YYLTYPE yyerror_range[2];
> >     +  YYLTYPE *yyerror_range_ptr = yyerror_range;
> >     +  ]])[
> >
> > Again, same idea.
> 
> I guess this is different from yymsgbuf_ptr, since this storage does
> need to survive from one call to the next.  But I don't see why the
> pointers are needed here, either.  If it's always the case that
> yyssa_ptr == pv->yyssa then you can simply use pv->yyssa rather than
> maintaining a separate pointer.  Likewise for yyerror_range_ptr.

I avoided using pv->yyssa for a good reason! In short, using a local 
variable instead of pv->yyssa really made the differences between 
yacc.c and push.c managable. My original patch did something like:
  ]b4_push_if([pv->yyssa],[yyssa])[
for each occurence of each variable that would be used. This made
the changes overwhemingly confusing. So, since I couldn't copy the
array 'yytype_int16 yyssa[YYINITDEPTH];' to a variable on the stack,
I simply created a new variable that I could simply copy.

I really hope I'm not being confusing here.

If you would like me to change this part of the patch, I'm very open to
suggestions.

Thanks,
Bob Rossi