[m-dev.] for discussion: stream library v2

[Peter Ross]

On Mon, Sep 25, 2000 at 10:49:03AM +1100, Fergus Henderson wrote:
> On 24-Sep-2000, Peter Ross <petdr at miscrit.be> wrote:
> > Please find attached my second attempt at a stream library.  This time
> > I have tried to define a typeclass for an impure lowlevel interface
> > for streams, and then defined a pure interface on top of that.
> 
> I think it's a good idea to build streams using a lowlevel interface
> for defining streams with a highlevel interface for using streams
> built on top of that, but it would be nicer if the lowlevel interface
> didn't need to be impure.
> 
I think that the low level interface has to be impure, that is why it is
lowlevel.

> > PS. I also attached the socket implementation which uses this new
> > interface.
> 
> It would also be a good idea to have an implementation of ordinary file
> I/O using this new interface.
> 
Yes I have done one but I haven't really tested it yet while I have used
the socket interface quite a bit, plus I didn't want to do to much
implementation work until the interface had settled down into its final
shape.

> > Any comments?
> > And should I place this stuff into the std library?
> 
> Not yet, IMHO -- see the comments below.
> 
> > :- module stream.
> > 

[snip]

> 
> > :- type stream(S) ---> stream(list(char)).
> 
> You should document what the list(char) represents,
> and why the `S' parameter is not used.
> 
The more I think about it the more I am convinced that this type should
change to

:- type stream(S)
    --->    stream(
                S,              % Handle on the stream
                list(char)      % putback characters
            ).

which means that we no longer have to pass the handle around, for
example stream__write_char becomes

:- pred stream__write_char(char::in, stream(S)::di, stream(S)::uo) is det.

> > :- pragma promise_pure(stream__read_char/4).
> > stream__read_char(Stream, Result, stream(PutbackChars), StreamOut) :-
> > 	(
> > 		PutbackChars = [],
> > 		NewPutbackChars = PutbackChars,
> > 		( impure lowlevel__read_char(Stream, Chr) ->
> > 			Result = ok(Chr)
> > 		;
> > 			( semipure lowlevel__eof(Stream) ->
> > 				Result = eof
> > 			;
> > 				semipure Err = lowlevel__error_message(Stream),
> > 				Result = error(Err)
> > 			)
> > 		)
> > 	;
> > 		PutbackChars = [Chr | NewPutbackChars],
> > 		Result = ok(Chr)
> > 	),
> > 	unsafe_promise_unique(stream(NewPutbackChars), StreamOut).
> 
> Here you're assuming that if the stream is at eof, then the
> previous operation succeeded.  This assumption is not documented
> in the lowlevel interface, and I don't think it is likely to hold in
> general.  Making this assumption may cause some errors to be ignored.
> 
You are correct that the assumption needs to be documented.

I arrived at this interface because fgetc return EOF on error or end of
file.  So first I check to see whether I am at EOF and if not it must be
an error.

> Also, why do you need `unsafe_promise_unique' here?
> 
I don't it is an artifact from my first implementation.

> > stream__putback_char(_Stream, Chr, stream(PutbackChars), StreamOut) :-
> > 	unsafe_promise_unique(stream([Chr | PutbackChars]), StreamOut).
> 
> Rather than calling unsafe_promise_unique here, it would be nicer to call
> copy/2 on Chr. 
> 
> Currently copy/2 is not as efficient as it could be, so for efficiency
> you might want to define
> 
> 	:- func copy_char(char::in) = (char::ui).
> 	copy_char(C0) = C :-
> 		unsafe_promise_unique(C0, C).
> 
> and use that instead.  But this is still an improvement, because calling
> unsafe_promise_unique on a char is nicer than calling it on a stream,
> since it's easier for a reader to understand why it is safe.
> 
Yes I agree wholeheartedly.

> > :- pragma promise_pure(stream__write_char/4).
> > stream__write_char(Stream, Chr, StreamIn, StreamOut) :-
> > 		% XXX A better design choice may be to throw the
> > 		% stream_error exception from inside
> > 		% lowlevel__write_char
> > 	( impure lowlevel__write_char(Stream, Chr) ->
> > 		true
> > 	;
> > 		semipure Err = lowlevel__error_message(Stream),
> > 		throw(stream_error(Err))
> > 	),
> > 	StreamOut = StreamIn.
> 
> I agree with the XXX comment here -- it would be better to throw
> the exception from inside lowlevel__write_char.
> 
> > %-----------------------------------------------------------------------%
> > % Copyright (C) 2000 The University of Melbourne.
> > % This file may only be copied under the terms of the GNU Library General
> > % Public License - see the file COPYING.LIB
> > %-----------------------------------------------------------------------%
> > %
> > % Module:	tcp
> > % Main Author:	peter.ross at miscrit.be (based on code written by pma at miscrit.be)
> > % Stability:	low
> > %
> > % An implementation of TCP streams.
> > %
> > %-----------------------------------------------------------------------%
> > %-----------------------------------------------------------------------%
> > 

Thanks for the review comments.  This module definately needs some work,
I attached it mainly as an example of using the stream interface.  I
think that it is best to seperate the review of this from the actual
stream implementation.

So I guess the next best step would be for me to submit the stream
implementation for review with an implementation of stdin/out stream,
read file stream and write file stream.

Agreed?

Pete
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------