www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - std.stream bugfixes

reply Ben Hinkle <bhinkle4 juno.com> writes:
I've made some bug fixes to std.stream and updated the code to take
advantage of some recent D features and put the file at
http://home.comcast.net/~benhinkle/stream.d
I'd also like to send Walter an updated section in phobos.html with these
modifications and documenting the stuff that had gotten in around 0.89.

Some of the changes are not backwards compatible so I'd like to get some
feedback before pestering Walter to accept it. I've been testing it on some
of my code that uses streams and if possible I'd like other people to see
if the non-backwards-compatible changes break code that's out there. The
changes are:

1) fixed scanf bug where it would error if the content ended with EOF. This
is backwards compatible

2) added uint available() function to InputStream and added implementations
to Stream and BufferedStream. Stream.available return 0 and BufferedStream
returns the amount of unread data in the buffer. This is not backwards
compatible since anyone implementing InputStream will have to add an
implementation of available(). Subclasses of Stream are ok. Since there
probably aren't many (if any) InputStreams that aren't Streams this isn't a
big problem.

3) changed all classes FooError:Error to FooException:Exception and added
aliases for FooError to map to the same thing as FooException. There are a
few files in phobos that catch or throw StreamError so removing the symbol
entirely would mean updating other files in phobos and probably some user
code. The aliases can be removed after a few releases. This should be
backwards compatible.

4) changed File open/create flags and one behavior. First, added a
FileMode.Trunc to say a file should be truncated when opened. Also made the
behavior on Windows match the behavior on linux when calling
 new File("foo",FileMode.In)
and the file didn't exist. On Windows it would create an empty file in
read-mode and on Linux it would error. Now it will error on both platforms.
To open or create a file in write-mode and zero out the length run
 new File("foo",FileMode.Out | FileMode.Trunc);

5) Changed various overloaded functions and constructors to use default
values. For example the three constructors for BufferedFile
 this(filename)
 this(filename,mode)
 this(filename,mode,buffersize)
have been replaced with one constructor with default mode and buffersize.
The functions readLine and readLine(char[] buffer) could be merged but it
would break socketstream.d so I didn't do that. In a perfect world I'd like
to merge those two functions as well as the other that seemed to be ok. The
only problem with backwards compatibility is with subclasses that override
parent functions.

6) Added TArrayStream(Buffer) which wraps an array-like type with a stream
(ok- this is actually a feature and not a bug-fix). This makes it easy to
wrap arrays and the idea was to use it to wrap MmFiles in a stream but
since MmFile is "auto" that part is commented out. I would really like
MmFile to not be auto so that they can be wrapped in a stream interface.
Made MemoryStream subclass TArrayStream!(ubyte[]). This should be backwards
compatible.

7) added a public bool isOpen() function to stream to allow users to check
if a stream is open or not. It can be closed either by failing in a call to
open() or by calling close(). This should be backwards compatible.

8) Two changes I'm not making but would like to if there weren't significant
issues with them:
8a) add a destructor to BufferedStream that flushed the buffer and closed
the source stream. Right now File has a destructor that closes the file but
that won't flush the buffer if there is one. A previous post about
destructors and the GC is causing this change to go on the back-burner
8b) add mixin templates for DefaultInputStream and DefaultOutputStream so
that people don't have to subclass all of Stream to get some default
behavior. The problem with this change is that templates containing private
data run into scoping issues and functions in the templates can conflict
with top-level functions from other module. These problems are nasty mixin
issues that I haven't found ways of dealing with. I thought I'd be able to
cut-and-paste the two chunks of Stream and wrap them in templates but it
ain't so easy.

That's it. Thanks for reading this far!
-Ben
Aug 18 2004
next sibling parent reply "Vathix" <vathixSpamFix dprogramming.com> writes:
Nice! How about adding writef() to OutputStream?
Aug 18 2004
next sibling parent Ben Hinkle <bhinkle4 juno.com> writes:
Vathix wrote:


 Nice! How about adding writef() to OutputStream?


good point. I'm trying to add it now and getting some conflicts with
printf's va_list imports. I think I'll be able to get that working, though.
Aug 19 2004
prev sibling parent reply Ben Hinkle <bhinkle4 juno.com> writes:
Vathix wrote:


 Nice! How about adding writef() to OutputStream?


ok, got it working and I've updated my web site. Sample usage:

stdout.writefln("output is ", 100, " and then ", 200);

produces

output is 100 and then 200

I added a writef and writefln to the interface InputStream and default
implementations to Stream.
Aug 19 2004
parent reply Nick <Nick_member pathlink.com> writes:
In article <cg3c5s$2knn$1 digitaldaemon.com>, Ben Hinkle says...

Vathix wrote:


 Nice! How about adding writef() to OutputStream?


ok, got it working and I've updated my web site.


Thank you! This is really starting to look good.

At the end of your file you import std.string and std.file. Is this intentional?
If so shouldn't they be private and at the top of the file?

By the way, the streams do not have the writef-doesn't-flush problem:
( http://www.digitalmars.com/drn-bin/wwwnews?digitalmars.D.bugs/1352 )
so if your std.stream goes into phobos, I will consider this problem solved. 

However, writef and stdout.writef will then be incompatible, in that combining
them might give unexpected behavior (in terms of output order). So I still think
the std.stdio functions should be rewritten to use stdout.

Nick
Aug 20 2004
parent "Ben Hinkle" <bhinkle mathworks.com> writes:
"Nick" <Nick_member pathlink.com> wrote in message
news:cg56v3$f27$1 digitaldaemon.com...

 In article <cg3c5s$2knn$1 digitaldaemon.com>, Ben Hinkle says...

Vathix wrote:


 Nice! How about adding writef() to OutputStream?


ok, got it working and I've updated my web site.


 Thank you! This is really starting to look good.

 At the end of your file you import std.string and std.file. Is this


intentional?

 If so shouldn't they be private and at the top of the file?


It was like that before my changes and I wondered the same thing. It might
have been in there even before the D language changes that recursively
searched imported modules - without which they would be no-ops. I agree
those imports should be removed. It would impact backwards-compatibility
since people would have to import std.string and file explicitly but that is
probably a good thing to do long-term.


 By the way, the streams do not have the writef-doesn't-flush problem:
 ( http://www.digitalmars.com/drn-bin/wwwnews?digitalmars.D.bugs/1352 )
 so if your std.stream goes into phobos, I will consider this problem


solved.

The likely reason is that stream.stdout is not buffered - it is a direct
feed to the OS console. The std.c.stdio.stdout is buffered.


 However, writef and stdout.writef will then be incompatible, in that


combining

 them might give unexpected behavior (in terms of output order). So I still


think

 the std.stdio functions should be rewritten to use stdout.


It would be nice to try to use either std.c.stdio.stdout or
std.stream.stdout consistently within phobos. Mixing them seems a bit
schizophrenic.


 Nick
Aug 20 2004
prev sibling next sibling parent reply Arcane Jill <Arcane_member pathlink.com> writes:
In article <cg1284$2e2d$1 digitaldaemon.com>, Ben Hinkle says...

2) added uint available() function to InputStream and added implementations
to Stream and BufferedStream. Stream.available return 0 and BufferedStream
returns the amount of unread data in the buffer.


Yes! Thank you thank you thank you.


This is not backwards
compatible since anyone implementing InputStream will have to add an
implementation of available().


Perhaps we should mention that { return 0; } is a perfectly good default
implementation.



4) changed File open/create flags and one behavior. First, added a
FileMode.Trunc to say a file should be truncated when opened. Also made the
behavior on Windows match the behavior on linux when calling
 new File("foo",FileMode.In)
and the file didn't exist. On Windows it would create an empty file in
read-mode and on Linux it would error. Now it will error on both platforms.
To open or create a file in write-mode and zero out the length run
 new File("foo",FileMode.Out | FileMode.Trunc);


Excellent! This has really caused me problems in the past.



6) Added TArrayStream(Buffer) which wraps an array-like type with a stream


I didn't completely understand that. Will there be documentation?



7) added a public bool isOpen() function to stream to allow users to check
if a stream is open or not. It can be closed either by failing in a call to
open() or by calling close(). This should be backwards compatible.


Cool. Two questions:
(1) is it safe to call close() on a stream which is already closed?, and
(2) does the destructor call close()?




8) Two changes I'm not making but would like to if there weren't significant
issues with them:
8a) add a destructor to BufferedStream that flushed the buffer and closed
the source stream. Right now File has a destructor that closes the file but
that won't flush the buffer if there is one. A previous post about
destructors and the GC is causing this change to go on the back-burner


Ah right - you've answered already. I can see this is complicated.



8b) add mixin templates for DefaultInputStream and DefaultOutputStream so
that people don't have to subclass all of Stream to get some default
behavior.


Useful for available(), for example.


The problem with this change is ...


Eech. Sounds complicated.



That's it. Thanks for reading this far!


I have one more comment. When will std.Stream be fully documented on the web
page http://www.digitalmars.com/d/phobos.html?

Arcane Jill
Aug 19 2004
parent Ben Hinkle <bhinkle4 juno.com> writes:
6) Added TArrayStream(Buffer) which wraps an array-like type with a stream


 
 I didn't completely understand that. Will there be documentation?


I'll also send Walter some updated phobos.html doc. The basic idea is that
the Buffer type only needs to support a length property, slicing and
indexing and it can be wrapped in a TArrayStream. Memory-mapped files work
this way so to read an MmFile as a stream one would write
 MmFile f = new MmFile("foo");
 Stream s = new TArrayStream!(MmFile)(f);
 char[] a = s.readLine();
 ...
This would work except for one small problem: MmFile is auto.
 

7) added a public bool isOpen() function to stream to allow users to check
if a stream is open or not. It can be closed either by failing in a call
to open() or by calling close(). This should be backwards compatible.


 
 Cool. Two questions:
 (1) is it safe to call close() on a stream which is already closed?, and


yes - for those implemented in std.stream. Other stream classes would still
do whatever they want in their close().
Aug 19 2004
prev sibling parent reply Nick <Nick_member pathlink.com> writes:
In article <cg1284$2e2d$1 digitaldaemon.com>, Ben Hinkle says...

I've made some bug fixes to std.stream and updated the code to take
advantage of some recent D features and put the file at
http://home.comcast.net/~benhinkle/stream.d
I'd also like to send Walter an updated section in phobos.html with these
modifications and documenting the stuff that had gotten in around 0.89.


Great work! But I have a few comments, if you don't mind:


To open or create a file in write-mode and zero out the length run
 new File("foo",FileMode.Out | FileMode.Trunc);


Could it be possible to have Trunc include Out implicitly? I mean there's very
few situations where you would use Trunc without Out. Also, perhaps there could
be an Append mode, which did the same as Out except also automatically seek to
the end of the file.


5) Changed various overloaded functions and constructors to use default
values. For example the three constructors for BufferedFile


After seeing the discussion of this topic on this NG, I have mostly been
convinced that default arguments in virtual functions is a bad idea in general.
(Ok for constructors, though.) This is because default values are not
inherrited, and which default value is used depend on how you call the members.
Eg.

class A ...
class B : A ...

B b = new B;
A a = b;
b.foo(); // Calls B.foo() with default values defined in B (if any)
a.foo(); // Calls B.foo() with default values defined in A (if any)

I have not studied your code in particular in this respect, so ignore me if I'm
overreacting.

[snip]

I would really like MmFile to not be auto so that they can be wrapped in a
stream interface.


Yes, I agree. In fact I am starting to think that auto classes are for the most
part useless, since you cannot pass them around anywhere except as function
parameters. Very few classes needs to be auto, IMO. Besides, there is no reason
MmFile should be auto when File is not. But this is just my opinion.

Nick
Aug 19 2004
parent Ben Hinkle <bhinkle4 juno.com> writes:
Nick wrote:


 In article <cg1284$2e2d$1 digitaldaemon.com>, Ben Hinkle says...

I've made some bug fixes to std.stream and updated the code to take
advantage of some recent D features and put the file at
http://home.comcast.net/~benhinkle/stream.d
I'd also like to send Walter an updated section in phobos.html with these
modifications and documenting the stuff that had gotten in around 0.89.


 
 Great work! But I have a few comments, if you don't mind:


don't mind at all - that's why I posted.


 

To open or create a file in write-mode and zero out the length run
 new File("foo",FileMode.Out | FileMode.Trunc);


 
 Could it be possible to have Trunc include Out implicitly? I mean there's
 very few situations where you would use Trunc without Out. Also, perhaps
 there could be an Append mode, which did the same as Out except also
 automatically seek to the end of the file.


That makes sense. Maybe instead of using Trunc it should be called something
with the word Out in it ... like OutNew or something (like mmfile). Append
by itself implies it is going to be writing so that sounds fine to me. So
the code above would become
 new File("foo",FileMode.OutNew);
Testing for (OutNew & Out) would return true. 


5) Changed various overloaded functions and constructors to use default
values. For example the three constructors for BufferedFile


 
 After seeing the discussion of this topic on this NG, I have mostly been
 convinced that default arguments in virtual functions is a bad idea in
 general. (Ok for constructors, though.) This is because default values are
 not inherrited, and which default value is used depend on how you call the
 members. Eg.
 
 class A ...
 class B : A ...
 
 B b = new B;
 A a = b;
 b.foo(); // Calls B.foo() with default values defined in B (if any)
 a.foo(); // Calls B.foo() with default values defined in A (if any)
 
 I have not studied your code in particular in this respect, so ignore me
 if I'm overreacting.


Looking over the code the functions File.open and File.create (and same for
BufferedFile) are candidates for default parameters. Will it be common to
define subclasses of File? Will subclasses want to use different defaults?
It seems fairly rare to do that. I'm tempted to go ahead with the default
parameters, but I'll ponder that one more.


 [snip]

I would really like MmFile to not be auto so that they can be wrapped in a
stream interface.


 
 Yes, I agree. In fact I am starting to think that auto classes are for the
 most part useless, since you cannot pass them around anywhere except as
 function parameters. Very few classes needs to be auto, IMO. Besides,
 there is no reason MmFile should be auto when File is not. But this is
 just my opinion.
 
 Nick
Aug 19 2004