[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for Dlang support] d: make enum SymbolKind idiomatic D
From: |
H. S. Teoh |
Subject: |
Re: [PATCH for Dlang support] d: make enum SymbolKind idiomatic D |
Date: |
Tue, 8 Sep 2020 12:53:39 -0700 |
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
>
>