Re: incorrect yychar for unambiguous GLR

[Paul Hilfinger]

 > This is a complex patch.  More pairs of eyes would be helpful.

Always a good idea.

> @@ -761,6 +761,11 @@ struct yyGLRState {
>  
>  struct yyGLRStateSet {
>    yyGLRState** yystates;
> +  /** During nondeterministic operation, yylookaheadStatuses tracks which
> +   *  stacks have actually needed the current lookahead.  During 
> deterministic
> +   *  operation, yylookaheadStatuses[0] is not maintained since it would 
> merely
> +   *  duplicate yychar != YYEMPTY.  */
> +  yybool* yylookaheadStatuses;
>    size_t yysize, yycapacity;
>  };
>  

I am not fond of the name 'yylookaheadStatuses'; doesn't seem too
descriptive, and one doesn't often say that "the status of X is
true".  How about yylookaheadUnused? yylookaheadAvailable?


 > @@ -771,6 +776,10 @@ struct yySemanticOption {
 >    yyRuleNum yyrule;
 >    /** The last RHS state in the list of states to be reduced.  */
 >    yyGLRState* yystate;
 > +  /** The lookahead for this reduction.  */
 > +  int yyrawchar;
 > +  YYSTYPE yyval;
 > +  YYLTYPE yyloc;
 >    /** Next sibling in chain of options.  To facilitate merging,
 >     *  options are chained in decreasing order by address.  */
 >    yySemanticOption* yynext;

As things stand, this organization will slightly expand the size of a
yyGLRStackItem in some cases.  How about the following definition of 
yySemanticOption instead?

    struct yySemanticOption {
      /** Type tag: always false. */
      yybool yyisState;
      /** The lookahead symbol for this reduction.  */
      short yyrawchar;
      /** Semantic value of the lookahead for this reduction. */
      YYSTYPE yyval;
      /** Location of the lookahead for this reduction. */
      YYLTYPE yyloc;
      /** The last RHS state in the list of states to be reduced. */
      yyGLRState* yystate;
      /** Next sibling in chain of options. To facilitate merging,
       *  options are chained in decreasing order by address. */
      yySemanticOption* yynext;
    };

 > +/** Stack #K = the stack from which RHS is taken.  This might not be the 
 > stack
 > + *  containing STATE, to which the deferred action is added.  */
 >  static void
 > -yyaddDeferredAction (yyGLRStack* yystackp, yyGLRState* yystate,
 > +yyaddDeferredAction (yyGLRStack* yystackp, size_t yyk, yyGLRState* yystate,
 >                   yyGLRState* rhs, yyRuleNum yyrule)

As long as we're adding a comment, we might as well make it complete.
How about:

   /** Add a new semantic action that will execute the action for rule
    *  RULENUM on the semantic values in RHS to the list of
    *  alternative actions for STATE.  Assumes that RHS comes from 
    *  stack #K of *STACKP. */

 > @@ -1505,9 +1540,17 @@ yysplitStack (yyGLRStack* yystackp, size
 >                                          * sizeof yynewStates[0])))))
 >      yyMemoryExhausted (yystackp);
 >        yystackp->yytops.yystates = yynewStates;
 > +      if (! (yynewLookaheadStatuses =
 > +         (yybool*) YYREALLOC (yystackp->yytops.yylookaheadStatuses,
 > +                              ((yystackp->yytops.yycapacity)
 > +                               * sizeof yynewLookaheadStatuses[0]))))
 > +    yyMemoryExhausted (yystackp);

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?

 > +/*ARGSUSED*/ static void
 >  yyreportAmbiguity (yySemanticOption* yyx0, yySemanticOption* yyx1,

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.

[Aside:  Are people still using lint?  I haven't in well over a decade,
 now that we have ANSI-style C + gcc -Wall.] 

Paul Hilfinger