[m-rev.] for review: make binary_{input, output}_stream_offset abort on overflow

Julien Fischer jfischer at opturion.com
Mon Oct 14 18:04:02 AEDT 2019


On Mon, 14 Oct 2019, Zoltan Somogyi wrote:

>
> On Mon, 14 Oct 2019 17:09:51 +1100 (AEDT), Julien Fischer <jfischer at opturion.com> wrote:
>> Make binary_{input,output}_stream_offset abort on overflow.
>>
>> Make binary{input,output}_stream_offset abort with an exception if the offset
>> they return cannot be respresented using Mercury's int type.  The current
>> impelmentation just silently casts the offset to an int.
>
> Fix spelling of implementation.

Done.

>>       % Returns the offset (in bytes) into the specified binary output stream.
>> +    % Throws an exception if the offset is outside the range that can be
>> +    % represented by the int type; the 64-bit version below should be used in
>> +    % that case.
>
> In that sentence, "that case" seems to refer to "when this throws an exception".
> I would suggest "To avoid this possibility, you can use the 64-bit offset version
> of this predicate below", for both predicates.

Done.

>> +    ( if int64.to_int(Offset64, OffsetPrime) then
>> +        Offset = OffsetPrime
>> +    else
>> +        error("io.binary_input_stream_offset: offset exceeds range of an int")
>> +    ).
>
> I think we should use error($pred, "...")  wherever possible.

Not done, since we currently don't do that in the standard library at
all.  I'm (mostly) not opposed to it, but doing it wholesale would alter
the contents of most of the standard library exceptions.  One reason we
may not want to do that for the stdlib is that $pred appends the arity
and sometimes the exception generated is applicable to both the
predicate and function versions of an operation.

Julien.


More information about the reviews mailing list