[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for Dlang support 1/2] d: create the Parser.Context class
From: |
Akim Demaille |
Subject: |
Re: [PATCH for Dlang support 1/2] d: create the Parser.Context class |
Date: |
Tue, 27 Oct 2020 07:38:46 +0100 |
Hi Adela!
Thanks, installed.
> Le 25 oct. 2020 à 15:19, Adela Vais <adela.vais99@gmail.com> a écrit :
This patch was hard to install, because it contains special characters:
> +The kind of the lookahead. Return ‘null’ iff there is no lookahead.
instead of use the Texinfo directive (@code here), and the mail header use a
dubious charset:
Content-Type: text/plain; charset=y
You should fix your mailer to claim UTF-8.
> @@ -452,7 +452,7 @@ m4_popdef([b4_at_dollar])])dnl
>
> /* Take a decision. First try without lookahead. */
> yyn = yypact_[yystate];
> - if (yy_pact_value_is_default_ (yyn))
> + if (yyPactValueIsDefault(yyn))
It is good that move the code to comply with D's coding style. But please, in
the future, make these steps separate commits.
> + /**
> + * Information needed to get the list of expected tokens and to forge
> + * a syntax error diagnostic.
> + */
> + public static final class Context
> + {
> +
> + private YYStack yystack;
Is this a copy of the stack? It does not look like a reference/pointer to the
this. We must not copy it, it's big, and readonly here. It should be "const",
but I don't know how that is spelled in D.
> + private SymbolKind yytoken;]b4_locations_if([[
> + private ]b4_location_type[ yylocation;]])[
> +
> + this(YYStack stack, SymbolKind kind]b4_locations_if([[,
> ]b4_location_type[ loc]])[)
> + {
> + yystack = stack;
> + yytoken = kind;]b4_locations_if([[
> + yylocation = loc;]])[
> + }
> +
> + final SymbolKind getToken() const
> + {
> + return yytoken;
> + }]b4_locations_if([[
> +
> + final ]b4_location_type[ getLocation()
Why is this one not const?
> + {
> + return yylocation;
> + }]])[
> + /**
> + * Put in YYARG at most YYARGN of the expected tokens given the
> + * current YYCTX, and return the number of tokens stored in YYARG. If
> + * YYARG is null, return the number of expected tokens (guaranteed to
> + * be less than YYNTOKENS).
> + */
> + int getExpectedTokens(SymbolKind[] yyarg, int yyargn)
Likewise. I believe they can all be const.
> @@ -709,33 +766,28 @@ m4_popdef([b4_at_dollar])])dnl
> /* Stay within bounds of both yycheck and yytname. */
> int yychecklim = yylast_ - yyn + 1;
> int yyxend = yychecklim < yyntokens_ ? yychecklim : yyntokens_;
> - int count = 0;
> - for (int x = yyxbegin; x < yyxend; ++x)
> - if (yycheck_[x + yyn] == x && x != ]b4_symbol(1, kind)[
> - && !yy_table_value_is_error_ (yytable_[x + yyn]))
> - ++count;
> - if (count < 5)
> - {
> - count = 0;
> - for (int x = yyxbegin; x < yyxend; ++x)
> - if (yycheck_[x + yyn] == x && x != ]b4_symbol(1, kind)[
> - && !yy_table_value_is_error_ (yytable_[x + yyn]))
> - {
> - res ~= count++ == 0 ? ", expecting " : " or ";
> - res ~= format!"%s"(SymbolKind(x));
> - }
> - }
> + for (int yyx = yyxbegin; yyx < yyxend; ++yyx)
> + if (yycheck_[yyx + yyn] == yyx && yyx != ]b4_symbol(1, kind)[
> + && !yyTableValueIsError(yytable_[yyx + yyn]))
> + yycount++;
> + if (yycount < yyargn)
> + {
> + yycount = 0;
> + for (int x = yyxbegin; x < yyxend; ++x)
> + if (yycheck_[x + yyn] == x && x != ]b4_symbol(1, kind)[
> + && !yyTableValueIsError(yytable_[x + yyn]))
> + yyarg[yycount++] = SymbolKind(x);
> + }
> }
> - return res;
> - }]])[
> - return "syntax error";
> + return yycount - yyoffset;
> + }
> }
This looks good!
Installed, thanks!