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

[Adela Vais]

Hello!

I noticed that patch #4 was never merged, is there something I need to
modify for it to be accepted?

Thank you,
Adela

În vin., 18 dec. 2020 la 20:37, Adela Vais <adela.vais99@gmail.com> a scris:

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