Re: [PATCH for Dlang support 1/2] d: change the return value of yylex from TokenKind to YYParser.Symbol

[Adela Vais]

Hello Akim,

I addressed the suggested modifications and removed the 3 unused methods
from the Lexer.
Thank you for the feedback and help!

Adela

În lun., 21 dec. 2020 la 08:34, Akim Demaille <akim@lrde.epita.fr> a scris:

> Hi Adela,
>
> This is a great piece of work!  The overall result is much better,
> well done!
>
> > Le 18 déc. 2020 à 19:37, Adela Vais <adela.vais99@gmail.com> a écrit :
> >
> > Hello!
> >
> > I made the suggested modifications.
> >
> > I have a question: given that the user will provide all the needed
> > information through the return value, should semanticVal(), startPos()
> and
> > endPos() still be part of the Lexer interface?
>
> No, they are not useful.  They make sense only with split symbols.
>
> Besides, in that case (split symbols) I personally think the right
> interface is that of locations, not that of positions.  IMHO it was
> an error in Java to put forward positions, and leave locations to
> the parser only.  C and C++ show only location-based interfaces.
> It's up to the user to decide that a location is nothing but a
> position if she thinks two positions is too costly (but that would
> be mean to the users).
>
>
> > În vin., 20 nov. 2020 la 20:18, Akim Demaille <akim@lrde.epita.fr> a
> scris:
> >
> >> You should introduce type aliases for b4_yystype and YYLocation.
> >> In the C++ parser, you have value_type and location_type which
> >> are defined to whatever they are actually.  The code is nicer
> >> to read, with fewer occurrences of ugly YYnames.
> >>
> >>
> > I named them Location and Value.
>
> Excellent.
>
> > Should I also change b4_location_type to
> > the new Location alias throughout lalr1.d? I know that the user should
> not
> > use yy names (and before these commits, the examples used YYLocation)
> but I
> > don't know how much of the backend I should change. I changed the
> > b4_yystype occurrences.
>
> I would also use Location everywhere.
>
>
> >>>        if (yychar == TokenKind.]b4_symbol(empty, id)[)
> >>>        {]b4_parse_trace_if([[
> >>>          yycdebugln ("Reading a token");]])[
> >>> -          yychar = yylex ();]b4_locations_if([[
> >>> -          static if (yy_location_is_class) {
> >>> -            yylloc = new ]b4_location_type[(yylexer.startPos,
> >> yylexer.endPos);
> >>> -          } else {
> >>> -            yylloc = ]b4_location_type[(yylexer.startPos,
> >> yylexer.endPos);
> >>> -          }]])
> >>> -          yylval = yylexer.semanticVal;[
> >>> +          Symbol yysymbol = yylex();
> >>> +          yychar = yysymbol.token();
> >>
> >> Maybe you don't need yychar, but only need yytoken.  You probably
> >> can avoid dealing with the token-kinds here, and deal only with
> >> the symbol kinds.
> >>
> > Done.
>
> Great.  This part, however, is too complex and not very clear:
>
> > -          if (yychar <= TokenKind.]b4_symbol(eof, [id])[)
> > +          if (yytoken <= ]b4_symbol(eof, [kind])[)
> >            {
> >              /* Return failure if at end of input.  */
> > -            if (yychar == TokenKind.]b4_symbol(eof, [id])[)
> > +            if (yytoken == ]b4_symbol(eof, [kind])[)
> >               return false;
> >            }
> >            else
> > -            yychar = TokenKind.]b4_symbol(empty, id)[;
> > +            yytoken = ]b4_symbol(empty, kind)[;
>
> When accepting token kinds as return values from yylex, we tolerate
> any nonpositive number to denote EOF.  Hence the first if.  However
> when using symbol kinds, we should not accept any deviation from
> the symbols kinds.  So that should be a single check "== eof", not
> two.
>
> Maybe have a look at what was done in C++.
>
> Again, great work, thanks!
>
> Cheers!
>
>

0001-d-remove-unnecessary-comparison-from-YYParser.parse.patch
Description: Binary data

0003-d-remove-unnecessary-methods-from-the-Lexer-interfac.patch
Description: Binary data

0002-d-use-Location-and-Position-aliases-in-the-backend.patch
Description: Binary data