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

Some D idiom comments:

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