On Tue, Oct 10, 2006 at 03:19:51PM -0700, Paul Eggert wrote: > Bob Rossi <[EMAIL PROTECTED]> 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
