On 19/07/19 23:37 +0200, François Dumont wrote:
It sounds reasonable to overload std::copy_n for istreambuf_iterator.
* include/bits/stl_algo.h (copy_n(istreambuf_iterator<>, _Size,
_OIte)):
New declaration.
* include/bits/streambuf_iterator.h (istreambuf_iterator<>): Declare
std::copy_n for istreambuf_iterator of char types to be friend.
(std::copy_n(istreambuf_iterator<>, _Size, _OIte)): New overload.
* include/std/streambuf(basic_streambuf<>): Declare std::copy_n for
istreambuf_iterator of char types to be friend.
* testsuite/25_algorithms/copy_n/istreambuf_iterator.cc: New.
* testsuite/25_algorithms/copy_n/istreambuf_iterator_neg.cc: New.
Tested under Linux x86_64, normal and debug modes.
Ok to commit ?
This is a nice improvement, I just have a few minor comments on it...
François
diff --git a/libstdc++-v3/include/bits/stl_algo.h
b/libstdc++-v3/include/bits/stl_algo.h
index 478f012def8..ec651e2cc45 100644
--- a/libstdc++-v3/include/bits/stl_algo.h
+++ b/libstdc++-v3/include/bits/stl_algo.h
@@ -771,6 +771,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_OutputIterator __result, random_access_iterator_tag)
{ return std::copy(__first, __first + __n, __result); }
+ template<typename _CharT, typename _Size, typename _OutputIterator>
+ typename enable_if<__is_char<_CharT>::__value,
+ _OutputIterator>::type
We have __enable_if_t that can be used to simplify this a bit:
__enable_if_t<__is_char<_CharT>::__value, _OutputIterator>
(The other declarations in bits/streambuf_iterator.h would need to be
changed to use that as well).
+ copy_n(istreambuf_iterator<_CharT, char_traits<_CharT> >,
std::copy_n is only declared for C++11 and later, so you don't need
the space between the closing angle brackets: >>
+ _Size __n, _OutputIterator __result);
+
/**
* @brief Copies the range [first,first+n) into [result,result+n).
* @ingroup mutating_algorithms
diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h
b/libstdc++-v3/include/bits/streambuf_iterator.h
index 2f4ff494a3a..c682fa91bde 100644
--- a/libstdc++-v3/include/bits/streambuf_iterator.h
+++ b/libstdc++-v3/include/bits/streambuf_iterator.h
@@ -80,6 +80,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__copy_move_a2(istreambuf_iterator<_CharT2>,
istreambuf_iterator<_CharT2>, _CharT2*);
+#if __cplusplus >= 201103L
+ template<typename _CharT2, typename _Size, typename _OutputIterator>
+ friend typename enable_if<__is_char<_CharT2>::__value,
+ _OutputIterator>::type
+ copy_n(istreambuf_iterator<_CharT2>, _Size, _OutputIterator);
+#endif
+
template<typename _CharT2>
friend typename __gnu_cxx::__enable_if<__is_char<_CharT2>::__value,
istreambuf_iterator<_CharT2> >::__type
@@ -367,6 +374,50 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return __result;
}
+#if __cplusplus >= 201103L
+ template<typename _CharT, typename _Size, typename _OutputIterator>
+ typename enable_if<__is_char<_CharT>::__value, _OutputIterator>::type
+ copy_n(istreambuf_iterator<_CharT> __it, _Size __n,
+ _OutputIterator __result)
+ {
+ if (__n == 0)
+ return __result;
+
+ __glibcxx_assert(__n > 0);
+ __glibcxx_requires_cond(!__it._M_at_eof(),
+ _M_message(__gnu_debug::__msg_inc_istreambuf)
+ ._M_iterator(__it));
Is this assertion necessary? Doesn't trying to read from the streambuf
also check the same condition, and so will already fail?
+
+ using traits_type = typename istreambuf_iterator<_CharT>::traits_type;
This is just char_traits<_CharT>, right?
+ const auto __eof = traits_type::eof();
+
+ auto __sb = __it._M_sbuf;
+ while (__n > 0)
+ {
+ streamsize __size = __sb->egptr() - __sb->gptr();
+ if (__size == 0)
+ {
+ if (traits_type::eq_int_type(__sb->underflow(), __eof))
+ {
+ __glibcxx_requires_cond(__n == 0,
+ _M_message(__gnu_debug::__msg_inc_istreambuf)
+ ._M_iterator(__it));
+ break;
+ }
+
+ __size = __sb->egptr() - __sb->gptr();
+ }
+
+ streamsize __xsize = std::min<streamsize>(__size, __n);
+ __result = std::copy(__sb->gptr(), __sb->gptr() + __xsize, __result);
+ __sb->__safe_gbump(__xsize);
+ __n -= __xsize;
+ }
+
+ return __result;
+ }
I wonder whether it's worth doing:
#if __cplusplus >= 201703L
if constexpr (is_same_v<_OutputIterator, _CharT*>)
return __result + __it._M_sbuf->sgetn(__result, __n);
else
{
#endif
...
#if __cplusplus >= 201703L
}
#endif
We could extend that to also work for basic_string<_CharT>::iterator
and vector<_CharT>::iterator too if we wanted.
I'm not sure if it will perform any better than the code below (it's
approximately equivalent) but it should result in smaller binaries, as we
wouldn't be instantiating the code below when outputting to a pointer
or contiguous iterator.
We don't need to do that now, it can be a separate patch later (if we
do it at all).
+#endif
Because the matching #if is more than 40 lines away, please add a
comment noting the condition that this corresponds to, i.e.
#endif // C++11
+
template<typename _CharT>
typename __gnu_cxx::__enable_if<__is_char<_CharT>::__value,
istreambuf_iterator<_CharT> >::__type
diff --git a/libstdc++-v3/include/std/streambuf
b/libstdc++-v3/include/std/streambuf
index d9ca981d704..4f62ebf4d95 100644
--- a/libstdc++-v3/include/std/streambuf
+++ b/libstdc++-v3/include/std/streambuf
@@ -155,6 +155,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__copy_move_a2(istreambuf_iterator<_CharT2>,
istreambuf_iterator<_CharT2>, _CharT2*);
+#if __cplusplus >= 201103L
+ template<typename _CharT2, typename _Size, typename _OutputIterator>
+ friend typename enable_if<__is_char<_CharT2>::__value,
+ _OutputIterator>::type
+ copy_n(istreambuf_iterator<_CharT2>, _Size, _OutputIterator);
+#endif
+
template<typename _CharT2>
friend typename __gnu_cxx::__enable_if<__is_char<_CharT2>::__value,
istreambuf_iterator<_CharT2> >::__type
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy_n/istreambuf_iterator.cc
b/libstdc++-v3/testsuite/25_algorithms/copy_n/istreambuf_iterator.cc
new file mode 100644
index 00000000000..ebd769cf7c0
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/copy_n/istreambuf_iterator.cc
@@ -0,0 +1,59 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do run { target c++11 } }
+
+#include <algorithm>
+#include <sstream>
+#include <iterator>
+
+#include <testsuite_hooks.h>
+
+void test01()
+{
+ std::stringstream ss("12345");
+
+ std::string ostr(5, '0');
+ typedef std::istreambuf_iterator<char> istrb_ite;
+ std::copy_n(istrb_ite(ss), 0, ostr.begin());
+ VERIFY( ostr.front() == '0' );
I'd like to see a check here that the value returned from copy_n is
equal to ostr.begin().
+
+ std::copy_n(istrb_ite(ss), 2, ostr.begin());
+ VERIFY( ostr == "12000" );
And equal to ostr.begin() + 2.
+
+ std::copy_n(istrb_ite(ss), 3, ostr.begin() + 2);
+ VERIFY( ostr == "12345" );
And equal to ostr.begin() + 5 here.
+}
+
+void test02()
+{
+ std::stringstream ss("12345");
+
+ std::string ostr(5, '0');
+ typedef std::istreambuf_iterator<char> istrb_ite;
+
+ istrb_ite ibfit(ss);
+ std::copy_n(ibfit, 3, std::copy_n(ibfit, 2, ostr.begin()));
+ VERIFY( ostr == "12345" );
+}
Ideally I'd also like to see tests where the input buffer is larger
than the size being read, e.g. read 5 chars from "123456" and verify
we don't read the '6'.
Also, these tests don't exercise the code path that causes an
underflow. It would be good to use an ifstream to read from one of the
files in the testsuite/data directory, and read a large amount of data
(more than fits in a filebuf's read area) so that the underflow logic
is tested.
+int main()
+{
+ test01();
+ test02();
+ return 0;
+}
diff --git
a/libstdc++-v3/testsuite/25_algorithms/copy_n/istreambuf_iterator_neg.cc
b/libstdc++-v3/testsuite/25_algorithms/copy_n/istreambuf_iterator_neg.cc
new file mode 100644
index 00000000000..42bc5b64306
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/copy_n/istreambuf_iterator_neg.cc
@@ -0,0 +1,40 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do run { target c++11 xfail *-*-* } }
+// { dg-require-debug-mode { } }
+
+#include <algorithm>
+#include <sstream>
+#include <iterator>
+
+#include <testsuite_hooks.h>
+
+void test01()
+{
+ std::stringstream ss("12345");
+
+ std::string ostr(10, '0');
+ typedef std::istreambuf_iterator<char> istrb_ite;
+ std::copy_n(istrb_ite(ss), 10, ostr.begin());
+}
+
+int main()
+{
+ test01();
+ return 0;
+}
