bison-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH for Dlang support] d: make enum SymbolKind idiomatic D


From: Adela Vais
Subject: Re: [PATCH for Dlang support] d: make enum SymbolKind idiomatic D
Date: Thu, 1 Oct 2020 02:40:25 +0300

Hello,

I realized that I was still triggering the GC within the toString function.
Fixed now.

d: don't trigger GC in void toString

* data/skeletons/d.m4: Here.
---
 data/skeletons/d.m4 | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/data/skeletons/d.m4 b/data/skeletons/d.m4
index fb883df8..759cb071 100644
--- a/data/skeletons/d.m4
+++ b/data/skeletons/d.m4
@@ -257,7 +257,6 @@ m4_define([b4_declare_symbol_enum],

       if (yystr[0] == '"')
         {
-          string yyr;
         strip_quotes:
           for (int i = 1; i < yystr.length; i++)
             switch (yystr[i])
@@ -271,11 +270,10 @@ m4_define([b4_declare_symbol_enum],
                   break strip_quotes;
                 goto default;
               default:
-                yyr ~= yystr[i];
+                put(sink, yystr[i]);
                 break;

               case '"':
-                put(sink, yyr);
                 return;
               }
         }
--
2.20.1

În lun., 14 sept. 2020 la 18:36, Adela Vais <adela.vais99@gmail.com> a
scris:

>
>
> În sâm., 12 sept. 2020 la 18:10, Akim Demaille <akim@lrde.epita.fr> a
> scris:
>
>> Hi Adela,
>>
>> > Le 11 sept. 2020 à 14:53, Adela Vais <adela.vais99@gmail.com> a écrit :
>> >
>> > The enum is now wrapped in a structure that contains its string
>> representation.
>> >
>> > * data/skeletons/d.m4, data/skeletons/lalr1.d: here.
>>
>> Great, thanks!
>>
>>
>>
>> Please, make run you run the test suite before submitting a commit,
>> there were errors.  I had to install the following changes to your
>> commit (you were still using yytname_).
>>
>> diff --git a/data/skeletons/d.m4 b/data/skeletons/d.m4
>> index 2e086ab0..7a9d3efe 100644
>> --- a/data/skeletons/d.m4
>> +++ b/data/skeletons/d.m4
>> @@ -269,11 +269,7 @@ m4_define([b4_declare_symbol_enum],
>>        put(sink, yystr);
>>      }
>>    }
>> -  ]])])
>> -    [
>> -  }
>> -]])])
>> -
>> +]])
>>
>>
>>  # b4-case(ID, CODE, [COMMENTS])
>> diff --git a/data/skeletons/lalr1.d b/data/skeletons/lalr1.d
>> index b452c4e0..e879dabf 100644
>> --- a/data/skeletons/lalr1.d
>> +++ b/data/skeletons/lalr1.d
>> @@ -373,12 +373,12 @@ b4_locations_if([, ref ]b4_location_type[
>> yylocationp])[)
>>      if (0 < yydebug)
>>      {
>>        string message = s ~ (yykind < yyntokens_ ? " token " : " nterm ")
>> -              ~ yytname_[yykind] ~ " ("]b4_locations_if([
>> +              ~ format("%s", yykind) ~ " ("]b4_locations_if([
>>                ~ yylocationp.toString() ~ ": "])[;
>> -      static if (__traits(compiles, message ~= yyvaluep.toString ()))
>> -              message ~= yyvaluep.toString ();
>> +      static if (__traits(compiles, message ~= yyvaluep.toString()))
>> +              message ~= yyvaluep.toString();
>>        else
>> -              message ~= format ("%s", &yyvaluep);
>> +              message ~= format("%s", &yyvaluep);
>>        message ~= ")";
>>        yycdebugln (message);
>>      }
>>
>>
>> Does it really make sense to use yyvaluep.toString here, doesn't the
>> second case, via format, always encompasses the first one?
>>
>>
> Hello,
>
> At the moment I see that the code makes the difference between calling
> toString and outputting the address of the variable of type YYSemanticType
> union, which doesn't seem to be intended behavior.
> Erasing the '&' gives "YYSemanticType" (the name of the union) all the
> way, so I think it remained like that because the address was giving
> somewhat more information.
> I will fix it in a future commit.
>
>
>> So I'm installing the following revised version of your commit.
>> With TODO updated.
>>
>>
> Thank you!
> Adela
>
>
>> Cheers!
>>
>> commit 2bc886dc02f00e7ebbaa008af8fdfce45cebadcd
>> Author: Adela Vais <adela.vais99@gmail.com>
>> Date:   Fri Sep 11 15:53:45 2020 +0300
>>
>>     d: make enum SymbolKind idiomatic D
>>
>>     Taking into account comments from H. S. Teoh.
>>     https://lists.gnu.org/r/bison-patches/2020-09/msg00021.html
>>
>>     * data/skeletons/d.m4, data/skeletons/lalr1.d (SymbolKind): Wrap the
>>     enum in a structure that contains its string representation.
>>
>> diff --git a/TODO b/TODO
>> index 98849e7a..3f4d1978 100644
>> --- a/TODO
>> +++ b/TODO
>> @@ -242,90 +242,6 @@ are.  Keep the same variable names.  If you change
>> the wording in one place,
>>  do it in the others too.  In other words: make sure to keep the
>>  maintenance *simple* by avoiding any gratuitous difference.
>>
>> -** Rename the D example
>> -Move the current content of examples/d into examples/d/simple.
>> -
>> -** Create a second example
>> -Duplicate examples/d/simple into examples/d/calc.
>> -
>> -** Add location tracking to d/calc
>> -Look at the examples in the other languages to see how to do that.
>> -
>> -** yysymbol_name
>> -The SymbolKind is an enum.  For a given SymbolKind we want to get its
>> string
>> -representation.  Currently it's a separate table in the parser that does
>> -that:
>> -
>> -  /* Symbol kinds.  */
>> -  public enum SymbolKind
>> -  {
>> -    S_YYEMPTY = -2,  /* No symbol.  */
>> -    S_YYEOF = 0,                   /* "end of file"  */
>> -    S_YYerror = 1,                 /* error  */
>> -    S_YYUNDEF = 2,                 /* "invalid token"  */
>> -    S_EQ = 3,                      /* "="  */
>> -    ...
>> -    S_input = 14,                  /* input  */
>> -    S_line = 15,                   /* line  */
>> -    S_exp = 16,                    /* exp  */
>> -  };
>> -
>> -  ...
>> -
>> -  /* YYTNAME[SYMBOL-NUM] -- String name of the symbol SYMBOL-NUM.
>> -     First, the terminals, then, starting at \a yyntokens_,
>> nonterminals.  */
>> -  private static immutable string[] yytname_ =
>> -  [
>> -  "\"end of file\"", "error", "\"invalid token\"", "\"=\"", "\"+\"",
>> -  "\"-\"", "\"*\"", "\"/\"", "\"(\"", "\")\"", "\"end of line\"",
>> -  "\"number\"", "UNARY", "$accept", "input", "line", "exp", null
>> -  ];
>> -
>> -  ...
>> -
>> -So to get a symbol kind, one runs `yytname_[yykind]`.
>> -
>> -Is there a way to attach this conversion to string to SymbolKind?  In
>> Java
>> -for instance, we have:
>> -
>> -  public enum SymbolKind
>> -  {
>> -    S_YYEOF(0),                    /* "end of file"  */
>> -    S_YYerror(1),                  /* error  */
>> -    S_YYUNDEF(2),                  /* "invalid token"  */
>> -    ...
>> -    S_input(16),                   /* input  */
>> -    S_line(17),                    /* line  */
>> -    S_exp(18);                     /* exp  */
>> -
>> -    private final int yycode_;
>> -
>> -    SymbolKind (int n) {
>> -      this.yycode_ = n;
>> -    }
>> -    ...
>> -    /* YYNAMES_[SYMBOL-NUM] -- String name of the symbol SYMBOL-NUM.
>> -       First, the terminals, then, starting at \a YYNTOKENS_,
>> nonterminals.  */
>> -    private static final String[] yynames_ = yynames_init();
>> -    private static final String[] yynames_init()
>> -    {
>> -      return new String[]
>> -      {
>> -        i18n("end of file"), i18n("error"), i18n("invalid token"), "!",
>> "+", "-", "*",
>> -        "/", "^", "(", ")", "=", i18n("end of line"), i18n("number"),
>> "NEG",
>> -        "$accept", "input", "line", "exp", null
>> -      };
>> -    }
>> -
>> -    /* The user-facing name of this symbol.  */
>> -    public final String getName() {
>> -      return yynames_[yycode_];
>> -    }
>> -  };
>> -
>> -which allows to write more naturally `yykind.getName()` rather than
>> -`yytname_[yykind]`.  Is there something comparable in (idiomatic) D?
>> -
>>  ** Change the return value of yylex
>>  Historically people were allowed to return any int from the scanner
>> (which
>>  is convenient and allows `return '+'` from the scanner).  Akim tends to
>> see
>> diff --git a/data/skeletons/d.m4 b/data/skeletons/d.m4
>> index edb0c49e..7a9d3efe 100644
>> --- a/data/skeletons/d.m4
>> +++ b/data/skeletons/d.m4
>> @@ -204,13 +204,72 @@ m4_define([b4_symbol_enum],
>>  # to use a signed type, which matters for yytoken.
>>  m4_define([b4_declare_symbol_enum],
>>  [[  /* Symbol kinds.  */
>> -  public enum SymbolKind
>> +  struct SymbolKind
>>    {
>> +    enum
>> +    {
>>      ]b4_symbol(-2, kind_base)[ = -2,  /* No symbol.  */
>>  ]b4_symbol_foreach([b4_symbol_enum])dnl
>> -[  };
>> -]])])
>> -
>> +[    }
>> +
>> +    private int yycode_;
>> +    alias yycode_ this;
>> +
>> +    this(int code)
>> +    {
>> +      yycode_ = code;
>> +    }
>> +
>> +    /* YYTNAME[SYMBOL-NUM] -- String name of the symbol SYMBOL-NUM.
>> +       First, the terminals, then, starting at \a YYNTOKENS_,
>> nonterminals.  */
>> +    static immutable string[] yytname_ = @{
>> +  ]b4_tname[
>> +    @};
>> +
>> +    /* Return YYSTR after stripping away unnecessary quotes and
>> +     backslashes, so that it's suitable for yyerror.  The heuristic is
>> +     that double-quoting is unnecessary unless the string contains an
>> +     apostrophe, a comma, or backslash (other than backslash-backslash).
>> +     YYSTR is taken from yytname.  */
>> +    final void toString(W)(W sink) const
>> +    if (isOutputRange!(W, char))
>> +    {
>> +      string yystr = yytname_[yycode_];
>> +
>> +      if (yystr[0] == '"')
>> +        {
>> +          string yyr;
>> +        strip_quotes:
>> +          for (int i = 1; i < yystr.length; i++)
>> +            switch (yystr[i])
>> +              {
>> +              case '\'':
>> +              case ',':
>> +                break strip_quotes;
>> +
>> +              case '\\':
>> +                if (yystr[++i] != '\\')
>> +                  break strip_quotes;
>> +                goto default;
>> +              default:
>> +                yyr ~= yystr[i];
>> +                break;
>> +
>> +              case '"':
>> +                put(sink, yyr);
>> +                return;
>> +              }
>> +        }
>> +      else if (yystr == "$end")
>> +      {
>> +        put(sink, "end of input");
>> +        return;
>> +      }
>> +
>> +      put(sink, yystr);
>> +    }
>> +  }
>> +]])
>>
>>
>>  # b4-case(ID, CODE, [COMMENTS])
>> diff --git a/data/skeletons/lalr1.d b/data/skeletons/lalr1.d
>> index 761125cb..e879dabf 100644
>> --- a/data/skeletons/lalr1.d
>> +++ b/data/skeletons/lalr1.d
>> @@ -361,41 +361,6 @@ b4_user_union_members
>>      return YYNEWSTATE;
>>    }
>>
>> -  /* Return YYSTR after stripping away unnecessary quotes and
>> -     backslashes, so that it's suitable for yyerror.  The heuristic is
>> -     that double-quoting is unnecessary unless the string contains an
>> -     apostrophe, a comma, or backslash (other than backslash-backslash).
>> -     YYSTR is taken from yytname.  */
>> -  private final string yytnamerr_ (string yystr)
>> -  {
>> -    if (yystr[0] == '"')
>> -      {
>> -        string yyr;
>> -      strip_quotes:
>> -        for (int i = 1; i < yystr.length; i++)
>> -          switch (yystr[i])
>> -            {
>> -            case '\'':
>> -            case ',':
>> -              break strip_quotes;
>> -
>> -            case '\\':
>> -              if (yystr[++i] != '\\')
>> -                break strip_quotes;
>> -              goto default;
>> -            default:
>> -              yyr ~= yystr[i];
>> -              break;
>> -
>> -            case '"':
>> -              return yyr;
>> -            }
>> -      }
>> -    else if (yystr == "$end")
>> -      return "end of input";
>> -
>> -    return yystr;
>> -  }
>>  ]b4_parse_trace_if([[
>>    /*--------------------------------.
>>    | Print this symbol on YYOUTPUT.  |
>> @@ -408,12 +373,12 @@ b4_locations_if([, ref ]b4_location_type[
>> yylocationp])[)
>>      if (0 < yydebug)
>>      {
>>        string message = s ~ (yykind < yyntokens_ ? " token " : " nterm ")
>> -              ~ yytname_[yykind] ~ " ("]b4_locations_if([
>> +              ~ format("%s", yykind) ~ " ("]b4_locations_if([
>>                ~ yylocationp.toString() ~ ": "])[;
>> -      static if (__traits(compiles, message ~= yyvaluep.toString ()))
>> -              message ~= yyvaluep.toString ();
>> +      static if (__traits(compiles, message ~= yyvaluep.toString()))
>> +              message ~= yyvaluep.toString();
>>        else
>> -              message ~= format ("%s", &yyvaluep);
>> +              message ~= format("%s", &yyvaluep);
>>        message ~= ")";
>>        yycdebugln (message);
>>      }
>> @@ -732,7 +697,7 @@ m4_popdef([b4_at_dollar])])dnl
>>        // FIXME: This method of building the message is not compatible
>>        // with internationalization.
>>        string res = "syntax error, unexpected ";
>> -      res ~= yytnamerr_ (yytname_[tok]);
>> +      res ~= format!"%s"(tok);
>>        int yyn = yypact_[yystate];
>>        if (!yy_pact_value_is_default_ (yyn))
>>        {
>> @@ -757,7 +722,7 @@ m4_popdef([b4_at_dollar])])dnl
>>                     && !yy_table_value_is_error_ (yytable_[x + yyn]))
>>                 {
>>                    res ~= count++ == 0 ? ", expecting " : " or ";
>> -                  res ~= yytnamerr_ (yytname_[x]);
>> +                  res ~= format!"%s"(SymbolKind(x));
>>                 }
>>            }
>>        }
>> @@ -795,13 +760,6 @@ m4_popdef([b4_at_dollar])])dnl
>>
>>    ]b4_parser_tables_define[
>>
>> -  /* YYTNAME[SYMBOL-NUM] -- String name of the symbol SYMBOL-NUM.
>> -     First, the terminals, then, starting at \a yyntokens_,
>> nonterminals.  */
>> -  private static immutable string[] yytname_ =
>> -  @{
>> -  ]b4_tname[
>> -  @};
>> -
>>  ]b4_parse_trace_if([[
>>    /* YYRLINE[YYN] -- Source line where rule number YYN was defined.  */
>>    private static immutable ]b4_int_type_for([b4_rline])[[] yyrline_ =
>> @@ -831,11 +789,10 @@ m4_popdef([b4_at_dollar])])dnl
>>    }
>>  ]])[
>>
>> -  private static SymbolKind yytranslate_ (int t)
>> +  private static auto yytranslate_ (int t)
>>    {
>>  ]b4_api_token_raw_if(
>> -[[    import std.conv : to;
>> -    return to!SymbolKind (t);]],
>> +[[    return SymbolKind(t);]],
>>  [[    /* YYTRANSLATE(YYLEX) -- Bison symbol number corresponding to
>> YYLEX.  */
>>      immutable ]b4_int_type_for([b4_translate])[[] translate_table =
>>      @{
>> @@ -848,10 +805,7 @@ m4_popdef([b4_at_dollar])])dnl
>>      if (t <= 0)
>>        return ]b4_symbol(0, kind)[;
>>      else if (t <= code_max)
>> -      {
>> -        import std.conv : to;
>> -        return to!SymbolKind (translate_table[t]);
>> -      }
>> +      return SymbolKind(translate_table[t]);
>>      else
>>        return ]b4_symbol(2, kind)[;]])[
>>    }
>>
>>


reply via email to

[Prev in Thread] Current Thread [Next in Thread]