> I agree about yylookaheadStatuses. However, yylookaheadAvailable sounds > like it means yychar != YYEMPTY. The polarity of yylookaheadUnused is > confusing, and it sounds like it might indicate whether the lookahead has > been shifted. How about yylookaheadNeeds, which follows from my comments > above its declaration and maintains the plural? I'm open to other > suggestions of course.
That one's fine. > > Call me fussy, but I am not fond of this use of NULL pointers to mean > > false. Also, why the extra parentheses? Could we try > > > > yynewLookaheadUnused = > > (yybool*) YYREALLOC (yystackp->yytops.yylookaheadUnused, > > (yystackp->yytops.yycapacity > > * sizeof yynewLookaheadUnused[0])); > > if (yynewLookaheadUnused == NULL) > > yyMemoryExhausted (yystackp); > > > > instead? > > In this case, I duplicated the style of the lines immediately before this. > I agree that it's a bit hard to read. I'm not sure whose style that was, > but I'd prefer to leave the rewrite up to you. > > I believe gcc likes the extra parentheses when you have an assignment used > as a boolean expression. ... and in this particular assignment, you didn't have that situation. Anyway, following your suggestion, I have submitted the following: Index: data/glr.c =================================================================== RCS file: /sources/bison/bison/data/glr.c,v retrieving revision 1.157 diff -u -p -r1.157 glr.c --- data/glr.c 11 Jan 2006 23:08:49 -0000 1.157 +++ data/glr.c 12 Jan 2006 00:15:25 -0000 @@ -1534,18 +1534,27 @@ yysplitStack (yyGLRStack* yystackp, size { yyGLRState** yynewStates; yybool* yynewLookaheadStatuses; - if (! ((yystackp->yytops.yycapacity - <= (YYSIZEMAX / (2 * sizeof yynewStates[0]))) - && (yynewStates = - (yyGLRState**) YYREALLOC (yystackp->yytops.yystates, - ((yystackp->yytops.yycapacity *= 2) - * sizeof yynewStates[0]))))) + + yynewStates = NULL; + + if (yystackp->yytops.yycapacity + > (YYSIZEMAX / (2 * sizeof yynewStates[0]))) + yyMemoryExhausted (yystackp); + yystackp->yytops.yycapacity *= 2; + + yynewStates = + (yyGLRState**) YYREALLOC (yystackp->yytops.yystates, + (yystackp->yytops.yycapacity + * sizeof yynewStates[0])); + if (yynewStates == NULL) yyMemoryExhausted (yystackp); yystackp->yytops.yystates = yynewStates; - if (! (yynewLookaheadStatuses = - (yybool*) YYREALLOC (yystackp->yytops.yylookaheadStatuses, - ((yystackp->yytops.yycapacity) - * sizeof yynewLookaheadStatuses[0])))) + + yynewLookaheadStatuses = + (yybool*) YYREALLOC (yystackp->yytops.yylookaheadStatuses, + (yystackp->yytops.yycapacity + * sizeof yynewLookaheadStatuses[0])); + if (yynewLookaheadStatuses == NULL) yyMemoryExhausted (yystackp); yystackp->yytops.yylookaheadStatuses = yynewLookaheadStatuses; } > > This seems to be the only use of ARGSUSED in the directory. I believe > > that the custom in Bison has been to use YYUSE instead. > > Paul Eggert started using ARGSUSED together with YYUSE because, despite > everyone's best efforts, no YYUSE definition managed to pacify all the > lint implementations we tried without imposing limitations on bison. > There are several occurrences of ARGSUSED in "glr.c" and "c.m4" now. > > > [Aside: Are people still using lint? I haven't in well over a decade, > > now that we have ANSI-style C + gcc -Wall.] > > I believe the push to satisfy lint started with a user request. It's all > buried somewhere in the mailing lists, but Paul Eggert probably knows the > history best. Well, the GDB people have eliminated all uses at this point. Forgive my blasphemy, but we don't have to respond to all requests. Reporting unused function arguments is a dubious check at best, because unused arguments are common when a function header is dictated by interface requirements, in contrast to unused locals, which are harder to justify. It's plausible to complain in C++, where argument names are optional for precisely this reason. Paul Hilfinger _______________________________________________ Help-bison@gnu.org http://lists.gnu.org/mailman/listinfo/help-bison