std::istream and std::ostream derived wrappers for wxInputStream and wxOutputStream

[wxtrac]

Issue migrated from trac ticket # 10637

component: base | priority: normal | resolution: fixed | keywords: std streams

2009-03-24 08:23:17: @net147 created the issue

---

I've created wrappers for wxInputStream and wxOutputStream that derive from std::istream and std::ostream respectively. This is because I wanted to use the Boost serialisation library with wxWidgets stream classes.

Others may find it useful so i'm attaching my wrapper classes (wxStdInputStream and wxOutputStream) here. They use a custom std::streambuf implementation that act as a proxy to wxWidgets streams.

[wxtrac]

2009-03-24 23:46:04: @vadz changed status from new to confirmed

2009-03-24 23:46:04: @vadz commented

This would definitely be useful to have, thanks! Will try to look at it soon...

[wxtrac]

2009-03-25 01:41:13: @SpareSimian commented

---

I haven't chased down the problem, but I'm seeing this with Visual Studio 2008 (.cc changed to .cpp):

stdstream.cpp(84) : error C2512: 'std::basic_istream<_Elem,_Traits>' : no appropriate default constructor available

[wxtrac]

2009-03-25 04:42:08: @net147 commented

---

Replying to [comment:3 ScratchMonkey]:

I haven't chased down the problem, but I'm seeing this with Visual Studio 2008 (.cc changed to .cpp):

stdstream.cpp(84) : error C2512: 'std::basic_istream<_Elem,_Traits>' : no appropriate default constructor available

I've updated stdstream.cc. Check if the updated version works.

[wxtrac]

2009-03-28 05:38:36: @net147 commented

---

Wrappers updated with support for putting back data into the stream.

[wxtrac]

2009-04-26 14:59:21: @vadz changed status from confirmed to infoneeded_new

2009-04-26 14:59:21: @vadz commented

This would be a really useful addition to wxWidgets and I'd be very glad to have it in the library. However we absolutely need:

1. Documentation
2. Unit tests for the new classes. Would it be possible for you to update the patch to provide them?

Also, a couple of questions/comments about the patch:

• Why do you override setbuf() if it does nothing (just as in the base class) anyhow? I think it should be either not overridden at all or generate an assertion failure if you do override it.
• Several of the overridden functions are protected in std::streambuf and it would be more tidy to make them protected and not public in the derived class as well.
• It would be also better to use virtual keyword for the overridden virtual functions, as per wx convention. And in general use wx standard formatting for the code (e.g. braces placement).
• It would be nice to have a helper function mapping std::ios_base::seekdir to wx values instead of duplicating this code in input and output buffers.

I also wonder if it wouldn't be useful to have the support for bridging the streams in the other direction, i.e. have wxStdInput/OutputStream which would be constructed from a std::i/ostream. This shouldn't delay the application of this patch, of course, as it's quite independent of it.

[wxtrac]

2009-04-26 15:40:48: @net147 changed status from infoneeded_new to new

2009-04-26 15:40:48: @net147 commented

Replying to [comment:6 vadz]:

I've made the following modifications:

• Brace style fixed
• Add virtual destructors
• Keep virtual protected inherited members as virtual protected
• Add helper function to map std::ios_base::seekdir to wxSeekMode

As for overriding setbuf(), I did it so that it returns NULL instead of the default of "this" to indicate failure instead of success (because wxWidgets streams don't allow setting a buffer as far as I know).

I'll add documentation and unit tests when I find time. If we were to include this in the library, what others things will need to be done? Do I you need me to modify any Makefiles to include it in the library or will you be okay with taking care of that?

[wxtrac]

2009-04-26 15:56:28: @vadz commented

---

Thanks for your changes (wow, this was lightning fast)!

Documentation should be pretty easy as you don't need to document the overridden methods (unless you really want to, of course...), you just need to add these classes description and -- very desirably -- a short usage example (as IME people are not really that familiar with C++ streams so showing how you could redirect std::cout to a wxStream, for example, would be just great).

The unit tests are really needed though, even if it's something really simple like using wxStringStreams or wxMemoryStreams and checking that we really get what we expect when using standard streams with buffers using them (of course, it would be also nice that detecting EOF, seeking &c works correctly too as these are typical areas where bugs could arise).

Finally, other things to be done: we have a choice between putting all the code inline which might be not very efficient code-size-wise but very simple as we don't need anything else to do, in particular no guards for platforms without streams support (e.g. WinCE) are needed. But if we want to keep the code in separate files it should be surrounded by #if wxUSE_STD_IOSTREAM guards. And in any case the new files need to be added to build/bakefiles/files.bkl -- but there is no need to modify the makefiles, they will be regenerated automatically from this file.

Thanks again!

[wxtrac]

2009-05-01 11:07:11: @net147 commented

---

I've attached an initial patch to add the wrappers classes to the library with the following changes:

• Reformatted code style
• Add appropriate headers
• Update bakefiles
• Add interface documentation
• Add test stub

Aside from the tests which i've still yet to write, does the patch seem okay or are there still some things missing that should be in the patch?

[wxtrac]

2009-05-02 11:50:14: @net147 uploaded file stdstream.h (2.0 KiB)

Header.

[wxtrac]

2009-05-02 12:42:47: @net147 uploaded file stdstream.cc (3.9 KiB)

Implementation.

[wxtrac]

2009-05-02 13:31:13: @net147 commented

---

I've finished writing all the unit tests and fixed all the bugs I could find. The finished patch is attached. Let me know if you need anything else for the patch.

[wxtrac]

2009-05-02 13:43:45: @net147 uploaded file stdstream.patch (32.4 KiB)

Initial patch to add wrappers classes to library.

[wxtrac]

2009-05-02 20:29:36: @vadz changed status from new to closed

2009-05-02 20:29:36: @vadz changed resolution from * to fixed*

2009-05-02 20:29:36: @vadz commented

(In [60483]) added wxStd{In,Out}putStream classes (closes #10637)

[wxtrac]

2009-05-02 20:31:37: @vadz commented

---

Excellent, thanks, committed now! I had to modify the unit tests as it was failing for me:

/usr/local/src/wx/HEAD/tests/streams/stdstream.cpp:186:Assertion
Test name: StdStreamTestCase::InputBuffer_pubseekpos
equality assertion failed
- Expected: 47
- Actual  : 49

/usr/local/src/wx/HEAD/tests/streams/stdstream.cpp:388:Assertion
Test name: StdStreamTestCase::OutputBuffer_pubseekpos
equality assertion failed
- Expected: -2
- Actual  : -1

but I think it was just because of a wrongly written infinite loop condition in

    for ( int i = 9; i < 10; --i )

loops so I simply corrected it.

I also had to fix compilation under 64 bit Linux (as this is what I use) and we might need to tweak it further but globally everything looks fine to me, thanks a lot!

[wxtrac]

2009-05-02 23:50:11: @net147 uploaded file stdstream_doc_fix_r60483.patch (0.5 KiB)

Fix typo.

[wxtrac]

2009-05-02 23:50:43: @net147 changed status from closed to reopened

2009-05-02 23:50:43: @net147 changed resolution from fixed to **

2009-05-02 23:50:43: @net147 commented

Just a small typo in the documentation. I've attached patch to fix it.

[wxtrac]

2009-05-02 23:59:05: @vadz commented

---

(In [60488]) fix typo (see #10637)

[wxtrac]

2009-05-03 00:02:46: @vadz changed status from reopened to closed

2009-05-03 00:02:46: @vadz changed resolution from * to fixed*

2009-05-03 00:02:46: @vadz commented

Thanks, corrected.

[wxtrac]

2009-05-03 01:05:57: @net147 uploaded file stdstream_doc_fix_r60488.patch (0.5 KiB)

Fix typo.

[wxtrac]

2009-05-03 01:06:36: @net147 changed status from closed to reopened

2009-05-03 01:06:36: @net147 changed resolution from fixed to **

2009-05-03 01:06:36: @net147 commented

Another patch to fix typo attached.

[wxtrac]

2009-05-03 11:29:15: @vadz commented

---

(In [60500]) correct a typo in a comment in the example (see #10637)

[wxtrac]

2009-05-03 11:30:32: @vadz changed status from reopened to closed

2009-05-03 11:30:32: @vadz changed resolution from * to fixed*

2009-05-03 11:30:32: @vadz commented

Thanks, corrected too.

On a separate note, to my surprise, it even works with VC6 after some tweaks.

[wxtrac]

2009-05-03 11:42:47: @net147 commented

---

Regarding the std::streamoff data type, i've found the following:

• GCC 4.3.3 (x86 32-bit and 64-bit)

  #ifdef _GLIBCXX_HAVE_INT64_T
  typedef int64_t       streamoff;
  #else
  typedef long long     streamoff;
  #endif

• Visual C++ 6

  typedef long streamoff;

• Visual C++ 9

  #ifdef _WIN64
  typedef __int64 streamoff;
  typedef __int64 streamsize;
  
  #else /* _WIN64 */
  typedef long streamoff;
  typedef int streamsize;
  #endif /* _WIN64 */

That explains the errors compiling the test as the WX_CPPUNIT_ALLOW_EQUALS_TO_INT macro is being expanded twice for long on Visual C++ 6. Would it be appropriate to support 64-bit integer types in general with WX_CPPUNIT_ALLOW_EQUALS_TO_INT in include/wx/cppunit.h?

[wxtrac]

2009-05-03 11:44:57: @net147 commented

---

We can't assume std::streamoff is 32-bit for all compilers on Windows as GCC 4.3.3 MinGW TDM-1 has std::streamoff as 64-bit data type even on 32-bit systems.

[wxtrac]

2009-05-03 11:59:33: @vadz commented

---

(In [60502]) allow comparison of int with 64 bit integer type (see #10637)