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

[Zoltan Somogyi]

2022-03-14 14:55 GMT+11:00 "Julien Fischer" <jfischer at opturion.com>:
> 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:

I  fully module qualified all references to entities that we define
that I moved. I also fully module qualified references to entities
that we define that I did not move, if they caught my eye.
I did not add any qualifications to anything in the standard
C# and Java libraries, because that would have required
knowing what the fully qualified names of those entities are.
It was a pointer to some documentation along those lines
that I was expecting in response to my earlier email asking
for education :-)

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

I agree, but this can and should be done later, after I am done
breaking up io.m, and preferably not by me.

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

Will do.

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

For users, no, but for us, there may be, at least in the short term.
The predicate versions are exported to C, and mkinit.c puts references
to those C functions into the autogenerated _init.c files. Offhand,
I don't know whether the signatures in C of the exported-to-C
versions of the func and pred versions of e.g. stdin_stream
are the same or different, but if they are different, then
eliminating the predicate version would require changing
the interface between mercury_wrapper.c and _init.c files.
In any case, conceptually, that is a wholly different change
than what this diff is about.

And we can't get rid of the function versions either,
because that would be a breaking change whose fix
is nontrivial for users.
 
> The rest looks fine.

Thank you.

Zoltan.