[m-rev.] for review: io.stream_ops.m

[Julien Fischer]

Hi Zoltan,

On Sun, 13 Mar 2022, Zoltan Somogyi wrote:

> There are several places in the diff marked with "zs:" that I am
> particularly seeking feedback on.

Most of those places concern blocks of C# using directives; I would be
inclined to replace such blocks by fully qualifying everything.  While
this is not something you would want to do in a C# program, I think it
can be justified here on the grounds that:

- it simplifies any future refactoring
- it makes the code more self-documenting -- I can't speak for others
but I don't use C# on a daily basis, so having the fully qualified
names is useful anyway.
- we already fully qualify lots of stuff anyway, a bit isn't going
to hurt.

> Carve io.stream_ops.m out of io.m.
> 
> Also, move foreign code pieces still in io.m that belong in
> previously-carved-out modules to those modules.
> 
> library/io.m:
> library/io.stream_ops.m:
>     As above. The new private submodule contains the implementations
>     (helper predicates) of the operations that
>
>     - open and close streams,
>     - get and set offsets in those streams,
>     - get and set line numbers on streams,
>     - return the standard streams, and
>     - set streams as the current streams.
> 
> library/io.m:
>     Make some previously privaye includes public, to allow modules outside

s/privaye/private/

...

> diff --git a/library/io.primitives_read.m b/library/io.primitives_read.m
> index 1a6d97d9e..b725e5463 100644
> --- a/library/io.primitives_read.m
> +++ b/library/io.primitives_read.m

...

>  :- pragma foreign_decl("C", "
> +#ifdef MR_HAVE_UNISTD_H
> +    #include <unistd.h>
> +#endif
> +
> +#include ""mercury_types.h""            // for MR_Integer
> +#include ""mercury_int.h""              // for MR_*_reverse_bytes
> +
> +#include <inttypes.h>
> +
> +// zs: I am *guessing* this is needed for SSIZE_T?
> +#ifdef MR_WIN32
> +  #include ""mercury_windows.h""
> +#endif

It is definitely needed for SSIZE_T, the real issue is whether it is needed for
anything else.  If not we could restrict that #include to when MSVC is being
used.

I am probably in a better position to test Windows stuff, so you can leave
this one with me.

> +#if defined(MR_MSVC)
> +    typedef SSIZE_T     ML_ssize_t;
> +#else
> +    typedef ssize_t     ML_ssize_t;
> +#endif
> +
> +int     mercury_get_byte(MercuryFilePtr mf);
> +
> +///////////////////////////////////////////////////////////////////////////
> +//
> +// The C implementation of reading multibyte integers from binary streams.
> +//
> +

...

> diff --git a/library/io.stream_ops.m b/library/io.stream_ops.m
> index e69de29bb..d12653a3b 100644
> --- a/library/io.stream_ops.m
> +++ b/library/io.stream_ops.m
> @@ -0,0 +1,1817 @@
> +%---------------------------------------------------------------------------%
> +% vim: ft=mercury ts=4 sw=4 et
> +%---------------------------------------------------------------------------%
> +% Copyright (C) 1993-2012 The University of Melbourne.
> +% Copyright (C) 2013-2022 The Mercury team.
> +% This file is distributed under the terms specified in COPYING.LIB.
> +%---------------------------------------------------------------------------%
> +%
> +% File: io.stream_ops.m.
> +%
> +% This file handles the implementation of operations that operate
> +% on streams as a whole, such as opening and closing streams, reading
> +% or updating the current offset on streams, and finding some standard streams.
> +%
> +% We communicate results from foreign_procs as separate simple arguments
> +% so the C/Java/etc code does not depend on how Mercury stores its
> +% discriminated union data types. It also avoids memory allocation in
> +% inner loops.

...

> +:- func stdin_stream_2 = stream.
> +:- pred stdin_stream_2(stream::out, io::di, io::uo) is det.
> +:- pred stdin_binary_stream_2(stream::out, io::di, io::uo) is det.
> +
> +:- func stdout_stream_2 = stream.
> +:- pred stdout_stream_2(stream::out, io::di, io::uo) is det.
> +:- pred stdout_binary_stream_2(stream::out, io::di, io::uo) is det.
> +
> +:- func stderr_stream_2 = stream.
> +:- pred stderr_stream_2(stream::out, io::di, io::uo) is det.

Is there any point in keeping the predicate versions of these?

The rest looks fine.

Julien.