Re: [PATCH for Dlang support 4/4] examples: d: started using location tracking

[Adela Vais]

Hello Akim,

În joi, 3 sept. 2020 la 09:45, Akim Demaille <akim@lrde.epita.fr> a scris:

> Hi Adela,
>
> > Le 2 sept. 2020 à 13:32, Adela Vais <adela.vais99@gmail.com> a écrit :
> >
> > diff --git a/examples/d/calc/README.md b/examples/d/calc/README.md
> > deleted file mode 100644
>
> This should have been part of the previous commit.  Use git rebase
> to clean up your commits before submitting them.
>
> > diff --git a/examples/d/calc/calc.test b/examples/d/calc/calc.test
> > index cf4f80a8..1650b220 100644
> > --- a/examples/d/calc/calc.test
> > +++ b/examples/d/calc/calc.test
> > @@ -23,4 +23,24 @@ run 0 7
> > cat >input <<EOF
> > 1 + 2 * * 3
> > EOF
> > -run 1 "err: syntax error, unexpected *, expecting + or - or ( or number"
> > +run 1 "err: 1.9: syntax error, unexpected *, expecting + or - or ( or
> number"
> > +
> > +cat >input <<EOF
> > +-8 * -8
> > +EOF
> > +run 0 64
> > +
> > +cat >input <<EOF
> > +1111 + 1111 2222
> > +EOF
> > +run 1 "err: 1.13-16: syntax error, unexpected number"
> > +
> > +cat >input <<EOF
> > +1 +
> > +EOF
> > +run 1 "err: 1.4-2.0: syntax error, unexpected end of line, expecting +
> or - or ( or number"
> > +
> > +cat >input <<EOF
> > +99 009
> > +EOF
> > +run 1 "err: 1.4-6: syntax error, unexpected number"
> > \ No newline at end of file
>
> Please make sure your text files end with the final eol.  And remove
> trailing blanks (there was one at the end of the "case '\n'" line).
>
> > @@ -103,6 +108,12 @@ class CalcLexer(R) : Lexer
> >     stderr.writeln (s);
> >   }
> >
> > +  void yyerror(YYLocation loc, string s)
> > +  {
> > +    stderr.write(loc.toString(), ": ");
> > +    yyerror(s);
> > +  }
>
> I believe we don't need the overload of yyerror that takes no location,
> so instead of a new one, the old one should be improved instead.
>
>
I will do this.


>
> >     // Numbers.
> >     if (input.front.isNumber)
> > +    {
> > +      int lenChars = 0;
>
> I wasn't aware the D style uses camelCase for variable names, I guess
> the skeleton is not clean on this regard.
>

I will clean up the skeleton in future commits, indeed D uses camelCase for
variables.


>
> > +      auto copy = input;
> > +      import std.conv : parse;
> > +      semanticVal_.ival = input.parse!int;
> > +      while (!input.empty && copy.front != input.front)
> >       {
> > -        import std.conv : parse;
> > -        semanticVal_.ival = input.parse!int;
> > -        return TokenKind.NUM;
> > +        lenChars++;
>
> This is really hairy...  Does it also apply cleanly on stdin?  There's
> no clean way to use parse!int _and_ get the number of bytes/characters
> that it consumed?
>

The function parse doesn't count the number of characters it consumes.
Sscanf was my best bet, but after a talk with Edi we realized that what I
did was working on very specific circumstances (I got lucky that the string
representation was at the beginning of the variable input).
He suggested this approach, for the moment at least.


> > +        copy.popFront;
> >       }
> > +      start = end;
> > +      end.column += lenChars;
> > +      return TokenKind.NUM;
> > +    }
>
>
> > diff --git a/examples/d/calc/local.mk b/examples/d/calc/local.mk
> > index 4497e5a3..5159c085 100644
> > --- a/examples/d/calc/local.mk
> > +++ b/examples/d/calc/local.mk
> > @@ -32,5 +32,5 @@ EXTRA_DIST += %D%/calc.test
> > %D%/calc: %D%/calc.d
> >       $(AM_V_GEN) $(DC) $(DCFLAGS) -of$@ %D%/calc.d
> >
> > -dist_d_DATA = %D%/calc.y %D%/Makefile %D%/README.md
> > -CLEANFILES += %D%/calc %D%/calc.[do]
> > +dist_d_DATA = %D%/calc.y %D%/Makefile
> > +CLEANFILES += %D%/calc %D%/calc.[do]
> > \ No newline at end of file
>
> Should have been part of the previous commit, and with the final eol.
>
> > diff --git a/examples/d/simple/README.md b/examples/d/simple/README.md
> > deleted file mode 100644
>
> Ditto.
>
> diff --git a/examples/d/simple/local.mk b/examples/d/simple/local.mk
> > index 40f8f63a..5159c085 100644
> > --- a/examples/d/simple/local.mk
> > +++ b/examples/d/simple/local.mk
> > @@ -27,10 +27,10 @@ EXTRA_DIST += %D%/calc.test
> >
> > %D%/calc.d: %D%/calc.y $(dependencies)
> >       $(AM_V_GEN)$(MKDIR_P) %D%
> > -     $(AM_V_at)$(BISON) $(srcdir)/%D%/calc.y -o $@
> > +     $(AM_V_at)$(BISON) -o $@ $(srcdir)/%D%/calc.y
>
> Likewise.
>
> > diff --git a/gnulib b/gnulib
> > index 175e0bc7..37b6f129 160000
> > --- a/gnulib
> > +++ b/gnulib
> > @@ -1 +1 @@
> > -Subproject commit 175e0bc72808d564074c4adcc72aeadb74adfcc6
> > +Subproject commit 37b6f1294620be849f951dcb2f505207b435f88d
>
> Should not be here.  Your copy of Bison is not updated to the current
> version of gnulib we use.  Be sure to run "git submodule init --update"
> on occasion when you see that gnulib was updated.  Possibly rerun
> bootstrap.  But don't force changes on gnulib by accident.
>
>
>
> So, these issues addressed, one gets the following commit, that I
> installed.  Cheers!
>
>
Thank you so much for your help and for your guidance!
I will make sure future commits will follow all the guidelines you provided.

Adela


> commit 6e1d83c8a8d6f6af81ad948c9312d8964043284e
> Author: Adela Vais <adela.vais99@gmail.com>
> Date:   Thu Sep 3 08:42:06 2020 +0200
>
>     examples: d: demonstrate location tracking
>
>     * examples/d/calc/calc.y: Track locations.
>     * examples/d/calc/calc.test: Check locations.
>
> diff --git a/examples/d/README.md b/examples/d/README.md
> index 86cb10a8..eed3494c 100644
> --- a/examples/d/README.md
> +++ b/examples/d/README.md
> @@ -9,7 +9,7 @@ afterwards.
>  The usual calculator.
>
>  ## d/calc.y
> -A richer implementation of the calculator.
> +A richer implementation of the calculator, with location tracking.
>
>  <!---
>
> diff --git a/examples/d/calc/calc.test b/examples/d/calc/calc.test
> index 6b237592..b592d215 100644
> --- a/examples/d/calc/calc.test
> +++ b/examples/d/calc/calc.test
> @@ -33,4 +33,19 @@ run 0 5
>  cat >input <<EOF
>  1 + 2 * * 3
>  EOF
> -run 1 "err: syntax error, unexpected *, expecting + or - or ( or number"
> +run 1 "err: 1.9: syntax error, unexpected *, expecting + or - or ( or
> number"
> +
> +cat >input <<EOF
> +1111 + 1111 2222
> +EOF
> +run 1 "err: 1.13-16: syntax error, unexpected number"
> +
> +cat >input <<EOF
> +1 +
> +EOF
> +run 1 "err: 1.4-2.0: syntax error, unexpected end of line, expecting + or
> - or ( or number"
> +
> +cat >input <<EOF
> +99 009
> +EOF
> +run 1 "err: 1.4-6: syntax error, unexpected number"
> diff --git a/examples/d/calc/calc.y b/examples/d/calc/calc.y
> index 15b4f7c7..0aa96ee6 100644
> --- a/examples/d/calc/calc.y
> +++ b/examples/d/calc/calc.y
> @@ -22,6 +22,8 @@
>  %define api.parser.class {Calc}
>  %define parse.error verbose
>
> +%locations
> +
>  %union {
>    int ival;
>  }
> @@ -94,13 +96,16 @@ class CalcLexer(R) : Lexer
>
>    this(R r) { input = r; }
>
> +  YYPosition start;
> +  YYPosition end;
> +
>    // Should be a local in main, shared with %parse-param.
>    int exit_status = 0;
>
> -  public void yyerror (string s)
> +  void yyerror(YYLocation loc, string s)
>    {
>      exit_status = 1;
> -    stderr.writeln (s);
> +    stderr.writeln(loc.toString(), ": ", s);
>    }
>
>    YYSemanticType semanticVal_;
> @@ -116,24 +121,39 @@ class CalcLexer(R) : Lexer
>
>      // Skip initial spaces
>      while (!input.empty && input.front != '\n' && isWhite (input.front))
> +    {
> +      start = end;
> +      end.column++;
>        input.popFront;
> +    }
>
>      if (input.empty)
>        return TokenKind.YYEOF;
>
>      // Numbers.
>      if (input.front.isNumber)
> +    {
> +      int lenChars = 0;
> +      auto copy = input;
> +      import std.conv : parse;
> +      semanticVal_.ival = input.parse!int;
> +      while (!input.empty && copy.front != input.front)
>        {
> -        import std.conv : parse;
> -        semanticVal_.ival = input.parse!int;
> -        return TokenKind.NUM;
> +        lenChars++;
> +        copy.popFront;
>        }
> +      start = end;
> +      end.column += lenChars;
> +      return TokenKind.NUM;
> +    }
>
>      // Individual characters
>      auto ch = input.front;
>      input.popFront;
> +    start = end;
> +    end.column++;
>      switch (ch)
> -      {
> +    {
>        case '=':  return TokenKind.EQ;
>        case '+':  return TokenKind.PLUS;
>        case '-':  return TokenKind.MINUS;
> @@ -141,9 +161,24 @@ class CalcLexer(R) : Lexer
>        case '/':  return TokenKind.SLASH;
>        case '(':  return TokenKind.LPAR;
>        case ')':  return TokenKind.RPAR;
> -      case '\n': return TokenKind.EOL;
> -      default: assert(0);
> +      case '\n':
> +      {
> +        end.line++;
> +        end.column = 1;
> +        return TokenKind.EOL;
>        }
> +      default: assert(0);
> +    }
> +  }
> +
> +  YYPosition startPos() const
> +  {
> +    return start;
> +  }
> +
> +  YYPosition endPos() const
> +  {
> +    return end;
>    }
>  }
>
>