[m-rev.] for review: formatting of uints using string.format etc.

[Julien Fischer]

Hi Zoltan,

On Sat, 23 May 2020, Zoltan Somogyi wrote:

> On Sat, 23 May 2020 02:12:43 +1000 (AEST), Julien Fischer <jfischer at opturion.com> wrote:
>> --- a/library/pprint.m
>> +++ b/library/pprint.m
>> @@ -463,6 +463,9 @@ group(X)                = 'GROUP'(doc(X)).
>>  poly(s(S))              = text(string.format("%s", [s(S)])).
>>  poly(c(C))              = text(string.format("%c", [c(C)])).
>>  poly(i(I))              = text(string.format("%d", [i(I)])).
>> +% XXX FIXME: replace the following line after the bootstrap compiler
>> +% understands how to specialise format calls with uints.
>> +poly(u(U))              = text(uint_to_string(U)).
>>  poly(f(F))              = text(string.format("%f", [f(F)])).
>
> This FIXME calls for a new synonym for compiler_sufficiently_recent
> in compiler/options.m.

Added.

>> --- a/library/string.format.m
>> +++ b/library/string.format.m
>> @@ -739,6 +822,9 @@ format_signed_int(Flags, MaybeWidth, MaybePrec, Int) = String :-
>>      % Format an unsigned int, unsigned octal, or unsigned hexadecimal
>>      % (u,o,x,X,p).
>>      %
>> +    % XXX we should replace most of this with a version that operates directly
>> +    % on uints.
>> +    %
>>  :- func format_unsigned_int(string_format_flags, string_format_maybe_width,
>>      string_format_maybe_prec, string_format_int_base, int) = string.
>> @@ -894,6 +980,15 @@ format_unsigned_int(Flags, MaybeWidth, MaybePrec, Base, Int) = String :-
>> +:- func format_uint(string_format_flags, string_format_maybe_width,
>> +    string_format_maybe_prec, string_format_int_base, uint) = string.
>> +
>> +format_uint(Flags, MaybeWidth, MaybePrec, Base, UInt) = String :-
>> +    Int = cast_to_int(UInt),
>> +    String = format_unsigned_int(Flags, MaybeWidth, MaybePrec, Base, Int).
>
> Do you intend to switch this around soon?

Yes.

>> index 080cb0b..1f026f7 100644
>> --- a/tests/hard_coded/stream_format.exp
>> +++ b/tests/hard_coded/stream_format.exp
>> @@ -1 +1 @@
>> -foo561a3.141000
>> +foo561a3.1410001111
>> diff --git a/tests/hard_coded/stream_format.m b/tests/hard_coded/stream_format.m
>> index f9e7061..9aa432b 100644
>> --- a/tests/hard_coded/stream_format.m
>> +++ b/tests/hard_coded/stream_format.m
>> @@ -18,5 +18,5 @@
>>
>>  main(!IO) :-
>>       io.stdout_stream(Stdout, !IO),
>> -     stream.string_writer.format(Stdout, "%s%d%c%f\n",
>> -          [s("foo"), i(561), c('a'), f(3.141)], !IO).
>> +     stream.string_writer.format(Stdout, "%s%d%c%f%u\n",
>> +          [s("foo"), i(561), c('a'), f(3.141), u(1111u)], !IO).
>
> Not related directly to this diff, but wouldn't it be easier to
> check the output if there was a space or some other divider between
> the values being printed?

I've added a space between them.

> I did not check the expected outputs of the string_format test cases,
> since I forgot the meaning of some of the flags. I hope you *did*
> check them.

Yes, by visual inspection for a sample of them and then by comparison
with what we produce when formatting signed integers as unsigned values.
(The latter was done using both the asm_fast.gc and java grades.)

If there is a problem, then it's an existing issue.

> Apart from the issues mentioned above, the diff is fine.

Thanks for that!

Julien.