Re: [RFC] Allow specification of semantic predicates.

[Paul Hilfinger]

 > This looks like a nice thing to add, thanks!  I assume you still have
 > commit privileges?  I didn't look at the GLR implementation carefully,
 > but here are a few comments on the rest of it.

I haven't tried with the new git repository; I'll have to see.

 > > +evaluated immediately (even when in nondeterministic mode).  If the 
 > > +expression yields a non-true value, the clause is treated as a syntax 
 > > error,
 > 
 > That "yields a non-true value" grates.  More important, the
 > documentation doesn't say what happens when the value is nonzero.  I'd
 > change it to something like "If the expression yields a nonzero value,
 > it becomes the semantic value for that clause; otherwise, the clause
 > is treated as a syntax error."  Perhaps you can word this even better,
 > but I hope you get the idea.

Umphh.  Well, it's the fussy semanticist in me that wrote that.
"non-true value" here is supposed to mean "any value that C treats as
false in conditional or logical operations".  This includes non-null
pointers.  Now, yes, we WRITE the null pointer as `0', and we (perhaps)
universally REPRESENT the null pointer as 0, but the standard doesn't
(at least to me) clearly imply  that it IS 0, (and, for example, frowns
on 

   const int x = 0;
   void *y = x;

in C (not C++, but even there this is a conversion).  But I SUPPOSE I
can use your terminology to keep the peace.


 > 
 > > +actions in nondeterministic mode, since the latrer are deferred.
 > 
 > "latrer" -> "later".

"latter" actually, but thanks for pointing it out. 

 > 
 > > +this silently causes an alterative parse to die.  During deterministic
 > 
 > "alterative" -> "alternative"
 > 
 > > +Predicates actions, however, have no defined value, and may not be given
 > 
 > "Predicate actions", I guess?  Or did you mean "Predicates' actions"?
 > 
 > > +For example, trying to rewrite the previous example as 
 > > +
 > > address@hidden
 > > +widget :
 > > +          @{ if (!new_syntax) YYERROR; @} "widget" id new_args  @{ $$ = 
 > > f($3, $4); @}
 > > +       |  @{ if (new_syntax) YYERROR; @} "widget" id old_args   @{ $$ = 
 > > f($3, $4); @}
 > > +       ;
 > > address@hidden smallexample
 > 
 > The "(!new_syntax)" and "(new_syntax)" should be swapped here.
 > 

Correct.


 > > -  fprintf (out, "b4_case(%d, [b4_syncline(%d, ", r + 1,
 > > -           rules[r].action_location.start.line);
 > > +  if (rules[r].is_predicate)
 > > +    fprintf (out, "b4_predicate_case(%d, [b4_syncline(%d, ", r + 1,
 > > +             rules[r].action_location.start.line);
 > > +  else
 > > +    fprintf (out, "b4_case(%d, [b4_syncline(%d, ", r + 1,
 > > +             rules[r].action_location.start.line);
 > 
 > Shouldn't the "if" be factored?  Something like this:
 > 
 >      fprintf (out, "b4_%scase(%d, [b4_syncline(%d, ", r + 1,
 >               rules[r].is_predicate ? "predicate" : "",
 >               rules[r].action_location.start.line);
 > 

OK.


 > > -<SC_BRACED_CODE>
 > > +<SC_BRACED_CODE,SC_PREDICATE>
 > >  {
 > >    "{"|"<"{splice}"%"  STRING_GROW; nesting++;
 > >    "%"{splice}">"      STRING_GROW; nesting--;
 > > +
 > > +  /* Tokenize `<<%' correctly (as `<<' `%') rather than incorrrectly
 > > +     (as `<' `<%').  */
 > > +  "<"{splice}"<"  STRING_GROW;
 > > +
 > > +  <<EOF>> {
 > > +    unexpected_eof (code_start, "}");
 > > +    STRING_FINISH;
 > > +    loc->start = code_start;
 > > +    val->code = last_string;
 > > +    BEGIN INITIAL;
 > > +    return BRACED_CODE;
 > > +  }
 > > +}
 > 
 > That "return BRACED_CODE" looks wrong.  Surely it should be something
 > like this:
 > 
 >     return YY_START == SC_BRACED_CODE ? BRACED_CODE : BRACED_PREDICATE;

Well... This is an error condition, so the actual syntactic tag seems
largely irrelevant.  I couldn't use that particular 'return', BTW, because
YY_START is always INITIAL at that point.  I suppose I can do

     int tok = YY_START == SC_BRACED_CODE ? BRACED_CODE : BRACED_PREDICATE;
     ...
     return tok;

Paul H.