Re: bison-patches Digest, Vol 174, Issue 10

[Adela Vais]

În mie., 9 sept. 2020 la 19:00, <bison-patches-request@gnu.org> a scris:

> Send bison-patches mailing list submissions to
>         bison-patches@gnu.org
>
> To subscribe or unsubscribe via the World Wide Web, visit
>         https://lists.gnu.org/mailman/listinfo/bison-patches
> or, via email, send a message with subject or body 'help' to
>         bison-patches-request@gnu.org
>
> You can reach the person managing the list at
>         bison-patches-owner@gnu.org
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of bison-patches digest..."
>
>
> Today's Topics:
>
>    1. [PATCH for Dlang support] d: make enum SymbolKind idiomatic D
>       (Adela Vais)
>    2. Re: [PATCH for Dlang support] d: make enum SymbolKind
>       idiomatic D (H. S. Teoh)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Tue,  8 Sep 2020 22:31:52 +0300
> From: Adela Vais <adela.vais99@gmail.com>
> To: bison-patches@gnu.org
> Cc: Adela Vais <adela.vais@yahoo.com>
> Subject: [PATCH for Dlang support] d: make enum SymbolKind idiomatic D
> Message-ID: <20200908193152.4818-1-adela.vais@yahoo.com>
>
> The enum is now wrapped in a structure that contains its string
> representation.
>
> * data/skeletons/d.m4, data/skeletons/lalr1.d: here.
> ---
>  data/skeletons/d.m4    | 29 +++++++++++++++++++++++++++--
>  data/skeletons/lalr1.d | 21 +++++----------------
>  2 files changed, 32 insertions(+), 18 deletions(-)
>
> diff --git a/data/skeletons/d.m4 b/data/skeletons/d.m4
> index edb0c49e..863484f4 100644
> --- a/data/skeletons/d.m4
> +++ b/data/skeletons/d.m4
> @@ -204,11 +204,36 @@ 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
>    {
> +    public enum SymbolKindEnum
> +    {
>      ]b4_symbol(-2, kind_base)[ = -2,  /* No symbol.  */
>  ]b4_symbol_foreach([b4_symbol_enum])dnl
> -[  };
> +[    }
> +
> +    private SymbolKindEnum yycode_;
> +    alias yycode_ this;
> +
> +    this(int code)
> +    {
> +      yycode_ = cast(SymbolKindEnum) code;
> +    }
> +
> +    /* 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[
> +    @};
> +
> +    public string toString() const
> +    {
> +      return yytname_[yycode_];
> +    }
> +  }
> +  ]])])
> +    [
> +  }
>  ]])])
>
>
> diff --git a/data/skeletons/lalr1.d b/data/skeletons/lalr1.d
> index 761125cb..59547b06 100644
> --- a/data/skeletons/lalr1.d
> +++ b/data/skeletons/lalr1.d
> @@ -732,7 +732,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 ~= yytnamerr_ (tok.toString);
>        int yyn = yypact_[yystate];
>        if (!yy_pact_value_is_default_ (yyn))
>        {
> @@ -757,7 +757,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 ~= yytnamerr_ (SymbolKind(x).toString);
>                 }
>            }
>        }
> @@ -795,13 +795,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 +824,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 +840,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)[;]])[
>    }
> --
> 2.17.1
>
>
>
>
> ------------------------------
>
> Message: 2
> Date: Tue, 8 Sep 2020 12:53:39 -0700
> From: "H. S. Teoh" <hsteoh@quickfur.ath.cx>
> To: bison-patches@gnu.org
> Subject: Re: [PATCH for Dlang support] d: make enum SymbolKind
>         idiomatic D
> Message-ID: <20200908195339.GA573429@quickfur.ath.cx>
> Content-Type: text/plain; charset=us-ascii
>
> Some D idiom comments:
>

Thank you for your feedback! I will address these comments and resend the
patch.

Adela


>
> 1) It's unusual to specifically annotate D declarations with `public`;
>    usually it's only used for countermanding a `private:` directive. I'd
>    drop the `public` from declarations.
>
> 2) The preferred toString signature is the one that takes a sink as
>    parameter and returns void; the overload that returns string is for
>    backward compatibility and is not as preferred because it forces GC
>    dependence. I'd write it like this:
>
>         import std.range.primitives;
>
>         ...
>         void toString(W)(W sink)
>                 if (isOutputRange!(W, char))
>         {
>                 put(sink, yytname_[yycode_];
>         }
>
>    This way, you can pass a delegate that receives character arrays to
>    toString and be able to get the string value without GC allocations.
>    This signature is also recognized by std.format, which will
>    automatically pass a string appender sink of the right type, so you
>    could just call `format("%s", x)` or `writeln` and pass the wrapped
>    enum to it and std.format takes care of the rest.
>
> 3) Nesting SymbolKindEnum inside SymbolKind makes for rather verbose
>    access syntax: you have to spell out SymbolKindEnum.SymbolKind.xyz...
>    every time you wish to refer to an enum value.  It might be better to
>    use an unnamed enum inside SymbolKind, e.g.:
>
>         struct SymbolKind
>         {
>                 enum {   // N.B. anonymous
>                         abc, def, ...
>                 }
>         }
>
>    Then you can refer to enum values as SymbolKind.abc, SymbolKind.def,
>    which is much more user-friendly as a wrapped enum.
>
>
> --T
>
> On Tue, Sep 08, 2020 at 10:31:52PM +0300, Adela Vais wrote:
> > The enum is now wrapped in a structure that contains its string
> representation.
> >
> > * data/skeletons/d.m4, data/skeletons/lalr1.d: here.
> > ---
> >  data/skeletons/d.m4    | 29 +++++++++++++++++++++++++++--
> >  data/skeletons/lalr1.d | 21 +++++----------------
> >  2 files changed, 32 insertions(+), 18 deletions(-)
> >
> > diff --git a/data/skeletons/d.m4 b/data/skeletons/d.m4
> > index edb0c49e..863484f4 100644
> > --- a/data/skeletons/d.m4
> > +++ b/data/skeletons/d.m4
> > @@ -204,11 +204,36 @@ 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
> >    {
> > +    public enum SymbolKindEnum
> > +    {
> >      ]b4_symbol(-2, kind_base)[ = -2,  /* No symbol.  */
> >  ]b4_symbol_foreach([b4_symbol_enum])dnl
> > -[  };
> > +[    }
> > +
> > +    private SymbolKindEnum yycode_;
> > +    alias yycode_ this;
> > +
> > +    this(int code)
> > +    {
> > +      yycode_ = cast(SymbolKindEnum) code;
> > +    }
> > +
> > +    /* 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[
> > +    @};
> > +
> > +    public string toString() const
> > +    {
> > +      return yytname_[yycode_];
> > +    }
> > +  }
> > +  ]])])
> > +    [
> > +  }
> >  ]])])
> >
> >
> > diff --git a/data/skeletons/lalr1.d b/data/skeletons/lalr1.d
> > index 761125cb..59547b06 100644
> > --- a/data/skeletons/lalr1.d
> > +++ b/data/skeletons/lalr1.d
> > @@ -732,7 +732,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 ~= yytnamerr_ (tok.toString);
> >        int yyn = yypact_[yystate];
> >        if (!yy_pact_value_is_default_ (yyn))
> >        {
> > @@ -757,7 +757,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 ~= yytnamerr_ (SymbolKind(x).toString);
> >                 }
> >            }
> >        }
> > @@ -795,13 +795,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 +824,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 +840,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)[;]])[
> >    }
> > --
> > 2.17.1
> >
> >
>
>
>
>
> ------------------------------
>
> Subject: Digest Footer
>
> _______________________________________________
> bison-patches mailing list
> bison-patches@gnu.org
> https://lists.gnu.org/mailman/listinfo/bison-patches
>
>
> ------------------------------
>
> End of bison-patches Digest, Vol 174, Issue 10
> **********************************************
>