[m-rev.] for review: putback of 8-bit values to binary input streams

Peter Wang novalazy at gmail.com
Fri Oct 5 16:15:09 AEST 2018


On Fri, 5 Oct 2018 02:36:11 +0000 (UTC), Julien Fischer <jfischer at opturion.com> wrote:
> 
> For review by anyone.
> 
> -------------------------------------
> 
> Putback of 8-bit values to binary input streams.
> 
> Support direct putback of int8 and uint8 values to binary input streams
> instead of via the bottom 8 bits of an int value.
> 
> library/io.m:
>      Add putback_{int8,uint8}/[34].
> 
>      Add stream type class instances for putback streams binary_input_stream
>      with int8 and uint8.
> 
> tests/hard_coded/Mmakefile:
> tests/hard_coded/putback_binary_int8.{m,exp}:
> tests/hard_coded/putback_binary_uint8.{m,exp}:
>      Add tests of the above.
> 
> Julien.
> 
> diff --git a/library/io.m b/library/io.m
> index 20af25a..b3356ff 100644
> --- a/library/io.m
> +++ b/library/io.m
> @@ -1014,6 +1014,20 @@
>   :- pred putback_byte(io.binary_input_stream::in, int::in, io::di, io::uo)
>       is det.

The description for putback_byte itself does not describe the behaviour
for out-of-range values.

> +    % As above, but un-reads an int8 instead of a byte represented by
> +    % the bottom 8 bits of an int.
> +    %
> +:- pred putback_int8(int8::in, io::di, io::uo) is det.
> +:- pred putback_int8(io.binary_input_stream::in, int8::in, io::di, io::uo)
> +    is det.

The comment should be more specific, something like:

    Un-reads a byte, where the byte value is the 8 bits of the int8
    reinterpreted as a uint8.

(where the reference manual has defined int8 to use two's complement
representation)

> +    % As above, but un-reads a uint8 instead of a byte represented by
> +    % the bottom 8 bits of an int.
> +    %
> +:- pred putback_uint8(uint8::in, io::di, io::uo) is det.
> +:- pred putback_uint8(io.binary_input_stream::in, uint8::in, io::di, io::uo)
> +    is det.
> +

Better not have two "As above" in a row. I suggest "Like putback_byte"
for both.

The code looks fine.

Peter


More information about the reviews mailing list