Thread overview
std.stream bugfixes
Aug 19, 2004
Ben Hinkle
Aug 19, 2004
Vathix
Aug 19, 2004
Ben Hinkle
Aug 19, 2004
Ben Hinkle
Aug 20, 2004
Nick
Aug 20, 2004
Ben Hinkle
Aug 19, 2004
Arcane Jill
Aug 19, 2004
Ben Hinkle
Aug 19, 2004
Nick
Aug 19, 2004
Ben Hinkle
August 19, 2004
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
August 19, 2004
Nice! How about adding writef() to OutputStream?


August 19, 2004
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



August 19, 2004
>>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().
August 19, 2004
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


August 19, 2004
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

August 19, 2004
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.
August 19, 2004
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.
August 20, 2004
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


August 20, 2004
"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
>
>