[m-rev.] for review: extend the lexer to recognise additional integer literals

Zoltan Somogyi zoltan.somogyi at runbox.com
Wed Apr 26 01:24:53 AEST 2017



On Tue, 25 Apr 2017 21:39:33 +1000 (AEST), "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:

> 
> 
> On Tue, 25 Apr 2017 21:10:14 +1000 (AEST), Julien Fischer <jfischer at opturion.com> wrote:
> > For review by anyone.
> 
> I will review this later today.

Here is the review.

> --- a/library/lexer.m
> +++ b/library/lexer.m
> ...
> @@ -1969,7 +1969,7 @@ get_zero(Stream, Token, !IO) :-
>             get_number(Stream, LastDigit, [Char], Token, !IO)
>         else if Char = '_' then
>             LastDigit = last_digit_is_underscore,
> -            get_number(Stream, LastDigit, [], Token, !IO)
> +            get_number(Stream, LastDigit, ['0'], Token, !IO)
>         else if Char = '''' then
>             get_char_code(Stream, Token, !IO)
>         else if Char = 'b' then

Document why we pass ['0'] instead of []: get_zero is invoked
only if the last (non-underscore) char seen was a zero.

Maybe this would be easier to do if get_zero took a !.RevChars
argument, which already contained ['0'].

> @@ -1978,6 +1978,12 @@ get_zero(Stream, Token, !IO) :-
>             get_octal(Stream, Token, !IO)
>         else if Char = 'x' then
>             get_hex(Stream, Token, !IO)
> +        else if Char = 'u' then
> +            get_integer_size_suffix(Stream, ['0'], base_10, unsigned,
> +                Token, !IO)
> +        else if Char = 'i' then
> +            get_integer_size_suffix(Stream, ['0'], base_10, signed,
> +                Token, !IO)

Likewise: document why we pass ['0'] instead of [] in these places.

> @@ -2156,6 +2179,7 @@ get_binary_2(Stream, !.LastDigit, !.RevChars, Token, !IO) :-
>     posn::in, posn::out) is det.
>
> string_get_binary_2(String, !.LastDigit, Len, Posn1, Token, Context, !Posn) :-
> +    Posn2 = !.Posn,
>     ( if string_read_char(String, Len, Char, !Posn) then
>         ( if char.is_binary_digit(Char) then
>             !:LastDigit = last_digit_is_not_underscore,
> @@ -2165,12 +2189,21 @@ string_get_binary_2(String, !.LastDigit, Len, Posn1, Token, Context, !Posn
) :-
>             !:LastDigit = last_digit_is_underscore,
>             string_get_binary_2(String, !.LastDigit, Len, Posn1, Token,
>                 Context, !Posn)
> +        else if Char = 'u' then
> +            string_get_integer_size_suffix(String, Len, Posn1, Posn2, base_2,
> +                unsigned, Token, !Posn),
> +            string_get_context(Posn1, Context, !Posn)
> +        else if Char = 'i' then
> +            string_get_integer_size_suffix(String, Len, Posn1, Posn2, base_2,
> +                signed, Token, !Posn),
> +            string_get_context(Posn1, Context, !Posn)

With Posn1, Posn2, !.Posn and !:Posn, I lost track.
A comment or two might help, and so might a more expressive name
than "Posn2". (Not just here; in all the places that follow this
code schema.)

I am too tired now to review the diff of the test cases with sufficient
attention; I will do that tomorrow. However, since the rest of the diff
is fine, you can go ahead and commit it.

Zoltan.


More information about the reviews mailing list