This patch changes the resturn type of istreambuf_iterator::operator++(int),
to dedicated __proxy type. This better reflect the standard, and allows
better optimization as _M_c is always set to EoF on normal path.
As _M_c may still have non-EoF value for the iterator returned by
post-increment, was produced and returned ifrom function defined in TU compiled
with any previous GCC version. Because of that, we also provide a implicit
conversion from __proxy to istream_iterator, that provides source compatibility.
Such patchs are considered unlikely, and thus marked as such.
We preserve extension allowing past-the-end iterator to be dereferenced,
and simplify the _M_at_eof value, by checking for _M_buf first.
libstdc++-v3/ChangeLog:
* include/bits/streambuf_iterator.h (istreambuf_iterator::__proxy):
Define.
(istreambuf_iterator::operator*): Inline _M_get, make path using
_M_c unlikely.
(istreambuf_iterator::operator++(int)): Return __proxy, and store
updated _M_sbuf pointer (null-for eof).
(istreambuf_iterator::_M_get): Remove.
(istreambuf_iterator::_M_at_eof): Inline _M_get and ignore _M_c.
* testsuite/24_iterators/istreambuf_iterator/sentinel.cc:
* testsuite/24_iterators/istreambuf_iterator/postinc.cc: New test.
Co-authored by: Jonathan Wakely <[email protected]>
Signed-off-by: Tomasz Kamiński <[email protected]>
---
Updated your implementation replacing __proxy for the return of
post-increment. This eliminate stores to _M_c during iterator.
The only case when it is used is when iterator is covnerted from
__proxy (or was returned from older TU), and we handle that in
dereference.
This provided further speedup (_sent test uses default_sentinel),
when after the change I see for same test:
-------------------------------------------------------------------
Benchmark Time CPU Iterations
-------------------------------------------------------------------
bench_empty 1.77 ns 1.76 ns 396575121
bench_stream_create 10.8 ns 10.7 ns 65465918
bench_preinc 329 ns 328 ns 2114616
bench_postinc 338 ns 337 ns 2076170
bench_deref_postinc 306 ns 305 ns 2301973
bench_preinc_sent 351 ns 349 ns 1997486
bench_postinc_sent 332 ns 331 ns 2118844
bench_deref_postinc_sent 308 ns 307 ns 2284426
Compared to results before:
-------------------------------------------------------------------
Benchmark Time CPU Iterations
-------------------------------------------------------------------
bench_empty 1.79 ns 1.79 ns 385470513
bench_stream_create 10.8 ns 10.7 ns 65436512
bench_preinc 445 ns 444 ns 1557361
bench_postinc 444 ns 442 ns 1588147
bench_deref_postinc 310 ns 309 ns 2261358
bench_preinc_sent 448 ns 447 ns 1568568
bench_postinc_sent 452 ns 450 ns 1548360
bench_deref_postinc_sent 311 ns 310 ns 2259266
Testing on x86_64-linux. Is this something we want to land
(technically breaking change)?
.../include/bits/streambuf_iterator.h | 46 +++++++++++++------
.../istreambuf_iterator/postinc.cc | 40 ++++++++++++++++
.../istreambuf_iterator/sentinel.cc | 20 ++++++++
3 files changed, 91 insertions(+), 15 deletions(-)
create mode 100644
libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/postinc.cc
diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h
b/libstdc++-v3/include/bits/streambuf_iterator.h
index 095928ca4d8..184bc69e0c5 100644
--- a/libstdc++-v3/include/bits/streambuf_iterator.h
+++ b/libstdc++-v3/include/bits/streambuf_iterator.h
@@ -115,6 +115,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
streambuf_type* _M_sbuf;
int_type _M_c;
+ class __proxy
+ {
+ streambuf_type* _M_sb;
+ char_type _M_keep;
+
+ explicit
+ __proxy(streambuf_type* __sb, char_type __c) _GLIBCXX_NOEXCEPT
+ : _M_sb(__sb), _M_keep(__c) { }
+
+ public:
+ char_type operator*() const _GLIBCXX_NOEXCEPT
+ { return _M_keep; }
+
+ friend class istreambuf_iterator;
+ };
+
public:
/// Construct end of input stream iterator.
_GLIBCXX_CONSTEXPR istreambuf_iterator() _GLIBCXX_USE_NOEXCEPT
@@ -139,6 +155,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
istreambuf_iterator(streambuf_type* __s) _GLIBCXX_USE_NOEXCEPT
: _M_sbuf(__s), _M_c(traits_type::eof()) { }
+ istreambuf_iterator(__proxy __p) _GLIBCXX_USE_NOEXCEPT
+ : _M_sbuf(__p._M_sb), _M_c(__p._M_keep) { }
+
#if __cplusplus >= 201103L
istreambuf_iterator&
operator=(const istreambuf_iterator&) noexcept = default;
@@ -151,7 +170,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
char_type
operator*() const
{
- int_type __c = _M_get();
+ int_type __c = traits_type::eof();
+ if (!_S_is_eof(_M_c)) [[unlikely]]
+ return traits_type::to_char_type(_M_c);
+ if (_M_sbuf) [[likely]]
+ __c = _M_sbuf->sgetc();
#ifdef _GLIBCXX_DEBUG_PEDANTIC
// Dereferencing a past-the-end istreambuf_iterator is a
@@ -178,16 +201,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return *this;
}
- /// Advance the iterator. Calls streambuf.sbumpc().
- istreambuf_iterator
+ /// Advance the iterator.
+ __attribute__((__always_inline__))
+ __proxy
operator++(int)
{
- istreambuf_iterator __old = *this;
- __old._M_c = _M_sbuf->sgetc();
+ int_type __c = _M_sbuf->sgetc();
++*this;
- return __old;
+ return __proxy(_M_sbuf, __c);
}
+
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// 110 istreambuf_iterator::equal not const
// NB: there is also number 111 (NAD) relevant to this function.
@@ -198,18 +222,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{ return _M_at_eof() == __b._M_at_eof(); }
private:
- int_type
- _M_get() const
- {
- int_type __ret = _M_c;
- if (_M_sbuf && _S_is_eof(__ret))
- __ret = _M_sbuf->sgetc();
- return __ret;
- }
bool
_M_at_eof() const
- { return _S_is_eof(_M_get()); }
+ { return !_M_sbuf || _S_is_eof(_M_sbuf->sgetc()); }
static bool
_S_is_eof(int_type __c)
diff --git a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/postinc.cc
b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/postinc.cc
new file mode 100644
index 00000000000..0aab7266349
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/postinc.cc
@@ -0,0 +1,40 @@
+// { dg-do run { target c++11 } }
+#include <iterator>
+#include <sstream>
+#include <testsuite_hooks.h>
+
+template<typename T, typename = void>
+struct can_increment
+: std::false_type { };
+
+template<typename T>
+struct can_increment<T, decltype(++std::declval<T&>(), void())>
+: std::true_type
+{ };
+
+void
+test_postinc()
+{
+ using iter_type = std::istreambuf_iterator<char>;
+
+ std::istringstream in("str");
+ iter_type iter(in.rdbuf());
+ static_assert( can_increment<decltype(iter)>::value, "sanity check" );
+ auto&& post = iter++;
+ static_assert( ! can_increment<decltype(post)>::value,
+ "cannot increment result of post-increment" );
+ static_assert( std::is_convertible<decltype(post), iter_type>::value,
+ "can convert result of post-increment back to iterator" );
+ VERIFY( *post == 's' );
+ VERIFY( *iter++ == 't' );
+ std::istreambuf_iterator<char> copy = iter++;
+ VERIFY( *copy == 'r' );
+ VERIFY( iter == iter_type() );
+ VERIFY( copy == iter_type() );
+}
+
+int main()
+{
+ test_postinc();
+}
+
diff --git
a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/sentinel.cc
b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/sentinel.cc
index 46ccad26c9b..ab8c529b258 100644
--- a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/sentinel.cc
+++ b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/sentinel.cc
@@ -45,8 +45,28 @@ test02()
VERIFY( std::default_sentinel == iter );
}
+
+void
+test03()
+{
+ std::istringstream in("abc");
+ std::istreambuf_iterator<char> iter(in);
+ VERIFY( iter != std::default_sentinel );
+ VERIFY( std::default_sentinel != iter );
+
+ auto iter2 = std::next(iter, 3);
+ VERIFY( iter2 == std::default_sentinel );
+ VERIFY( std::default_sentinel == iter2 );
+
+ // We should not use the original object after we've incremented a copy,
+ // so the standard does not require this to work.
+ VERIFY( iter == std::default_sentinel );
+ VERIFY( std::default_sentinel == iter );
+}
+
int main()
{
test01();
test02();
+ test03();
}
--
2.54.0
