[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