[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