Re: incorrect yychar for unambiguous GLR

[Paul Hilfinger]

> 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