[m-rev.] for review: allow sized integers as format arguments

[Zoltan Somogyi]

2020-11-09 18:00 GMT+11:00 "Julien Fischer" <jfischer at opturion.com>:
>>     The idea is that each of these poly_type values works exactly
>>     the same way as the i(_) poly_type (if signed) or the u(_) poly_type
>>     (if unsigned), with the exception that the value specified by the call
>>     is cast to int or uint before being processed.
> 
> That's fine for 8-, 16- and 32-bit integer types.  It won't work for 64-bit integer
> types on systems where int / uint are 32-bit.

True; the question is what we can do about it. I see several options.

- Just delete the i64 and u64 alternatives. I don't like this, because it
  limits all platforms for the sake of a dying subset of platforms.

- Just call {,u}int64.cast_to_{,u}int as this diff does, even on 32 bit platforms.
  This won't work, and the result may be garbage, but we can document this fact,
  and leave it up to users to avoid this scenario.

- Change format_cast_int64_to_int and its uint counterpart to check
  whether the platform is 32 bit, and abort with an explicit, specific
  error message if it is.

I would vote for the third option. What would you guys prefer?

>> --- a/library/string.m
>> +++ b/library/string.m
>> @@ -1556,7 +1556,15 @@
>>  :- type poly_type
>>      --->    f(float)
>>      ;       i(int)
>> +    ;       i8(int8)
>> +    ;       i16(int16)
>> +    ;       i32(int32)
>> +    ;       i64(int64)
>>      ;       u(uint)
>> +    ;       u8(uint8)
>> +    ;       u16(uint16)
>> +    ;       u32(uint32)
>> +    ;       u64(uint64)
>>      ;       s(string)
>>      ;       c(char).
> 
> The documentation directly below this type declaration (for string.format) should
> also be updated.

I can do this by

- adding a note such as "the valid conversion specifiers for int{8,16,32,64},
  are the same as for int", and likewise for uint,

- by replacing all int entries in the table with int*, and likewise for uint, or

- doing both.

Any preferences?

Thanks for the review.

Zoltan.