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

[Adela Vais]

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?

Î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. 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 also created an alias for YYPosition, which was part of the user
interface.


> > +    SymbolKind token() { return kind; }
> > +    ]b4_yystype[ semanticValue() { return value; }]b4_locations_if([[
>
> Make this value instead of semanticValue.
>

Done.


> > @@ -75,7 +75,7 @@ public interface Lexer
> >    * to the next token and prepares to return the semantic value
> >    * ]b4_locations_if([and beginning/ending positions ])[of the token.
> >    * @@return the token identifier corresponding to the next token. */
> > -  TokenKind yylex ();
> > +  ]b4_parser_class[.Symbol yylex ();
>
> Can't you provide an alias to avoid the need for the full path?
>

Done, I called it simply 'Symbol'.


> > @@ -411,7 +411,9 @@ b4_locations_if([, ref ]b4_location_type[
> yylocationp])[)
> >       yycdebugln (message);
> >     }
> >   }
> > -]])[
> > +]])
> > +b4_symbol_type_define
> > +[
>
> I would prefer
>
> > ]])[
> > ]b4_symbol_type_define[
> >
>
> for consistency.


Done.


> >         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.

>
> > +          yylval = yysymbol.semanticValue();]b4_locations_if([[
> > +          yylloc = yysymbol.location();]])[
> >         }
>
> > +@deftypemethod {Lexer} {YYParser.Symbol} yylex()
> > +Return the next token. The return value is of type YYParser.Symbol,
>
> Use @code{YYParser.Symbol}.


Done.

@code for TokenKind too.


Done.

> +      case '+':  return Calc.Symbol(TokenKind.PLUS, new
> YYLocation(startPos, endPos));
> > +      case '-':  return Calc.Symbol(TokenKind.MINUS, new
> YYLocation(startPos, endPos));
> > +      case '*':  return Calc.Symbol(TokenKind.STAR, new
> YYLocation(startPos, endPos));
> > +      case '/':  return Calc.Symbol(TokenKind.SLASH, new
> YYLocation(startPos, endPos));
> > +      case '(':  return Calc.Symbol(TokenKind.LPAR, new
> YYLocation(startPos, endPos));
> > +      case ')':  return Calc.Symbol(TokenKind.RPAR, new
> YYLocation(startPos, endPos));
>
> This is super verbose.  Can't you factor some 'loc' variable and use it?
>

Done


> In modern C++ there's a feature I like: the constructor can be
> called implicitly.  So for instance
>
> MyStruct foo() { return 42; }
>
> actually means
>
> MyStruct foo() { return MyStruct(42); }
>
> If D has something equivalent, you can probably simplify all this.
>
>
I'm afraid there is no equivalent in D. I have to explicitly call the
constructor.


> > +      case '+':  return
> YYParser.Symbol(TokenKind.]AT_TOKEN_PREFIX[PLUS]AT_LOCATION_IF([[, new
> YYLocation(startPos, endPos)]])[);
> > +      case '-':  return
> YYParser.Symbol(TokenKind.]AT_TOKEN_PREFIX[MINUS]AT_LOCATION_IF([[, new
> YYLocation(startPos, endPos)]])[);
> > +      case '*':  return
> YYParser.Symbol(TokenKind.]AT_TOKEN_PREFIX[STAR]AT_LOCATION_IF([[, new
> YYLocation(startPos, endPos)]])[);
> > +      case '/':  return
> YYParser.Symbol(TokenKind.]AT_TOKEN_PREFIX[SLASH]AT_LOCATION_IF([[, new
> YYLocation(startPos, endPos)]])
>
> Same comments about verbosity and redundancy.
>

Done.

Adela

0001-d-change-name-of-YYParser.Symbol-s-semanticValue-to-.patch
Description: Binary data

0003-d-m4-style-consistency-fixup-around-b4_symbol_type_d.patch
Description: Binary data

0002-d-create-alias-Symbol-for-YYParse.Symbol.patch
Description: Binary data

0005-d-reduce-verbosity-for-returning-the-location-from-y.patch
Description: Binary data

0004-d-doc-add-code-annotation-for-yylex-definition.patch
Description: Binary data

0006-d-remove-yychar-from-YYParse.parse.patch
Description: Binary data

0007-d-create-alias-Location-for-YYLocation.patch
Description: Binary data

0008-d-create-alias-Value-for-YYSemanticType.patch
Description: Binary data

0009-d-create-alias-Position-for-YYPosition.patch
Description: Binary data