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 <[email protected]> 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 <[email protected]> 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 >
