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

Julien Fischer jfischer at opturion.com
Mon Nov 9 22:16:34 AEDT 2020


On Mon, 9 Nov 2020, Zoltan Somogyi wrote:

> 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.

Java and .NET are not dying platforms by any stretch of the imagination.
(Mercury int's are 32-bit on both of those platforms.)

I don't like this suggestion anyway, since I've been trying to avoid
having builtin types that are second class citizens (e.g. like unsigned
integer types in C#).

> - 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?

None of those, I would like string.format to handle 64-bit integers
correctly on 32-bit platforms ;-)

For now, I suggest going with your third option above and change
format_cast_int64_to_int etc to check if the platform is 32-bit and
abort if it is.  Supporting 32-bit platforms properly shouldn't be
too difficult, but will require further additions -- I can look into
that if you like.

>>> --- 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,

That seems fine.

Julien.


More information about the reviews mailing list