[PATCH] streambuf_iterator: avoid debug-dependent behaviour

[Petr Ovtchenkov]

On Tue, 29 Aug 2017 22:02:14 +0200
François Dumont <frs.dumont@gmail.com> wrote:

> Hi
> 
>      You should perhaps justify why you want to do those changes.
> 

Because behaviour under debug should correspond to non-debug variant
as close as possible, if you mean this part:

@@ -136,9 +136,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       istreambuf_iterator&
       operator++()
       {
-	__glibcxx_requires_cond(!_M_at_eof(),
-				_M_message(__gnu_debug::__msg_inc_istreambuf)
-				._M_iterator(*this));
 	if (_M_sbuf)
 	  {
 	    _M_sbuf->sbumpc();

and this:
@@ -151,14 +148,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       istreambuf_iterator
       operator++(int)
       {
-	__glibcxx_requires_cond(!_M_at_eof(),
-				_M_message(__gnu_debug::__msg_inc_istreambuf)
-				._M_iterator(*this));
+        _M_get();
 
 	istreambuf_iterator __old = *this;
 	if (_M_sbuf)
 	  {
-	    __old._M_c = _M_sbuf->sbumpc();
+	    _M_sbuf->sbumpc();
 	    _M_c = traits_type::eof();
 	  }
 	return __old;
The second part explicit return __old in "old" state.
The
-	    __old._M_c = _M_sbuf->sbumpc();
work correctly because __old and "current" use same _M_sbuf.

This part
@@ -177,18 +172,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _M_get() const
       {
 	const int_type __eof = traits_type::eof();
-	int_type __ret = __eof;
-	if (_M_sbuf)
-	  {
-	    if (!traits_type::eq_int_type(_M_c, __eof))
-	      __ret = _M_c;
-	    else if (!traits_type::eq_int_type((__ret = _M_sbuf->sgetc()),
-					       __eof))
-	      _M_c = __ret;
-	    else
-	      _M_sbuf = 0;
-	  }
-	return __ret;
+	if (_M_sbuf && traits_type::eq_int_type(_M_c, __eof))
+          _M_c = _M_sbuf->sgetc();
+	return _M_c;
       }
remove drop _M_sbuf if EOF happens.
Illustration:
<snip>
  stringstream ss;

  ss << one;

  isrteambuf_iterator<char>(ss) i;
  copy_n( i, 10, b );
  ++i; // EOF reached

  ss << two;
  copy_n( i, 10, b ); // impossible, if _M_sbuf = 0; when EOF reached before
  ++i;
</snip>


> On 24/08/2017 11:57, Petr Ovtchenkov wrote:
> > Explicit do sgetc from associated streambuf. Avoid debug-dependent
> > sgetc (within _M_at_eof()):
> >
> >         __glibcxx_requires_cond(!_M_at_eof(),
> >                                 _M_message(__gnu_debug::__msg_inc_istreambuf)
> >                                 ._M_iterator(*this));
> >
> > Increment operators not require not-eof precoditions.
> >
> > Don't unlink associated streambuf if eof detected (in _M_get).
> 
> Why ? Are you working with a streambuf_type implementation that can 
> return eof at some moment and later return something else ?
> 
> I don't think istreambuf_iterator has been designed to work with such a 
> streambuf_type implementation. Your streambuf_type should block until it 
> fetches data, just do it in a background thread if you don't want to 
> block all your application while waiting for it.
> 
> However your request made me consider this implementation, I wonder why 
> it is done this way. Especially why the mutable keywords ?
> 
> I would have expected an implementation more consistent with the 
> istream_iterator implementation reading current character on 
> construction like in the attached patch.
> 
> Is it because of the _GLIBCXX_USE_NOEXCEPT constraint on the 
> constructors ? Can sgetc() throw ?
> 
> François

What attached file mean? Is it _you_ suggestion/point of view?
What code we discuss in this case?

--

  - ptr