bison-patches
[Top][All Lists]
Advanced

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


reply via email to

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