On 28/09/2017 23:56, Jonathan Wakely wrote:
On 28/09/17 21:59 +0200, François Dumont wrote:
The current istreambuf_iterator implementation capture the current
streambuf state each time it is tested for eof or evaluated. This is
why I considered those tests as fragile.
Yes, and I think that's non-conforming.
Good, then we have something to fix.
As I said in the other thread, I'd really like to see references to
the standard used to justify any changes to our istreambuf_iterator
I think _M_c has been introduced to cover this paragraph of the Standard:
24.6.3.1
"1. Class istreambuf_iterator<charT,traits>::proxy is for exposition
only. An implementation is permit-
ted to provide equivalent functionality without providing a class with
this name. Class istreambuf_-
iterator<charT, traits>::proxy provides a temporary placeholder as the
return value of the post-
increment operator (operator++). It keeps the character pointed to by
the previous value of the iterator for
some possible future access to get the character."
This is why it is being set in operator++(int):
istreambuf_iterator __old = *this;
__old._M_c = _M_sbuf->sbumpc();
It is also the reason why libstdc++ fails:
http://llvm.org/svn/llvm-project/libcxx/trunk/test/std/iterators/stream.iterators/istreambuf.iterator/istreambuf.iterator.cons/proxy.pass.cpp
It looks like this test is simply not Standard conformant.
I guess at some point _M_c started to be used also to cache the
streambuf::sgetc resulting in current situation.
In attached patch I limit usage of _M_c to cover 24.6.3.1.1 point and so
made additional changes to 2.cc test case to demonstrate it.
François
diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h b/libstdc++-v3/include/bits/streambuf_iterator.h
index f0451b1..94d8c7f 100644
--- a/libstdc++-v3/include/bits/streambuf_iterator.h
+++ b/libstdc++-v3/include/bits/streambuf_iterator.h
@@ -95,7 +95,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// NB: This implementation assumes the "end of stream" value
// is EOF, or -1.
mutable streambuf_type* _M_sbuf;
- mutable int_type _M_c;
+ int_type _M_c;
public:
/// Construct end of input stream iterator.
@@ -103,7 +103,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
: _M_sbuf(0), _M_c(traits_type::eof()) { }
#if __cplusplus >= 201103L
- istreambuf_iterator(const istreambuf_iterator&) noexcept = default;
+ istreambuf_iterator(const istreambuf_iterator&) = default;
~istreambuf_iterator() = default;
#endif
@@ -122,28 +122,29 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
char_type
operator*() const
{
+ int_type __c = _M_get();
+
#ifdef _GLIBCXX_DEBUG_PEDANTIC
// Dereferencing a past-the-end istreambuf_iterator is a
// libstdc++ extension
- __glibcxx_requires_cond(!_M_at_eof(),
+ __glibcxx_requires_cond(!_S_at_eof(__c),
_M_message(__gnu_debug::__msg_deref_istreambuf)
._M_iterator(*this));
#endif
- return traits_type::to_char_type(_M_get());
+ return traits_type::to_char_type(__c);
}
/// Advance the iterator. Calls streambuf.sbumpc().
istreambuf_iterator&
operator++()
{
- __glibcxx_requires_cond(!_M_at_eof(),
+ __glibcxx_requires_cond(_M_sbuf &&
+ (!_S_at_eof(_M_c) || !_S_at_eof(_M_sbuf->sgetc())),
_M_message(__gnu_debug::__msg_inc_istreambuf)
._M_iterator(*this));
- if (_M_sbuf)
- {
+
_M_sbuf->sbumpc();
_M_c = traits_type::eof();
- }
return *this;
}
@@ -151,16 +152,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
istreambuf_iterator
operator++(int)
{
- __glibcxx_requires_cond(!_M_at_eof(),
+ __glibcxx_requires_cond(_M_sbuf &&
+ (!_S_at_eof(_M_c) || !_S_at_eof(_M_sbuf->sgetc())),
_M_message(__gnu_debug::__msg_inc_istreambuf)
._M_iterator(*this));
istreambuf_iterator __old = *this;
- if (_M_sbuf)
- {
__old._M_c = _M_sbuf->sbumpc();
_M_c = traits_type::eof();
- }
return __old;
}
@@ -176,26 +175,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
int_type
_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
+ int_type __ret = _M_c;
+ if (_M_sbuf && _S_at_eof(__ret) && _S_at_eof(__ret = _M_sbuf->sgetc()))
_M_sbuf = 0;
- }
return __ret;
}
bool
_M_at_eof() const
+ { return _S_at_eof(_M_get()); }
+
+ static bool
+ _S_at_eof(int_type __c)
{
const int_type __eof = traits_type::eof();
- return traits_type::eq_int_type(_M_get(), __eof);
+ return traits_type::eq_int_type(__c, __eof);
}
};
@@ -373,13 +367,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
typedef typename __is_iterator_type::traits_type traits_type;
typedef typename __is_iterator_type::streambuf_type streambuf_type;
typedef typename traits_type::int_type int_type;
+ const int_type __eof = traits_type::eof();
if (__first._M_sbuf && !__last._M_sbuf)
{
const int_type __ival = traits_type::to_int_type(__val);
streambuf_type* __sb = __first._M_sbuf;
int_type __c = __sb->sgetc();
- while (!traits_type::eq_int_type(__c, traits_type::eof())
+ while (!traits_type::eq_int_type(__c, __eof)
&& !traits_type::eq_int_type(__c, __ival))
{
streamsize __n = __sb->egptr() - __sb->gptr();
@@ -396,11 +391,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__c = __sb->snextc();
}
- if (!traits_type::eq_int_type(__c, traits_type::eof()))
- __first._M_c = __c;
- else
- __first._M_sbuf = 0;
+ __first._M_c = __eof;
}
+
return __first;
}
diff --git a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc
index b81f4d4..572ea9f 100644
--- a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc
+++ b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc
@@ -25,9 +25,7 @@
void test02(void)
{
-
typedef std::istreambuf_iterator<char> cistreambuf_iter;
- typedef cistreambuf_iter::streambuf_type cstreambuf_type;
const char slit01[] = "playa hermosa, liberia, guanacaste";
std::string str01(slit01);
std::istringstream istrs00(str01);
@@ -35,10 +33,14 @@ void test02(void)
// ctor sanity checks
cistreambuf_iter istrb_it01(istrs00);
- cistreambuf_iter istrb_it02;
- std::string tmp(istrb_it01, istrb_it02);
+ cistreambuf_iter istrb_eos;
+ VERIFY( istrb_it01 != istrb_eos );
+
+ std::string tmp(istrb_it01, istrb_eos);
VERIFY( tmp == str01 );
+ VERIFY( istrb_it01 == istrb_eos );
+
cistreambuf_iter istrb_it03(0);
cistreambuf_iter istrb_it04;
VERIFY( istrb_it03 == istrb_it04 );
