On 9/27/19 1:00 PM, Jonathan Wakely wrote:
On 19/07/19 23:37 +0200, François Dumont wrote:
It sounds reasonable to overload std::copy_n for istreambuf_iterator.
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).

Very good remark, I hadn't check streambuf to find out if there were better to do. For me it is the streambuf method to target for an std::copy_n overload.

So here is a new proposal much simpler. I see no reason to enable it only for char types, is there ?

Once the other patch on copy/copy_backward... algos is in I'll provide what necessary to benefit from the same optimization for std::deque iterators and in Debug mode.


+#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
Ok, done even if there is no 40 lines anymore. And also added it in stl_algo.h.

+
  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.
Done.

+}
+
+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'.
In test01 I am doing something like that.

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.

With this new proposal I don't need to do it, I'am counting on sgetn tests.

    * include/bits/stl_algo.h
    (__copy_n_a(_IIte, _Size, _OIte)): New.
    (__copy_n_a(istreambuf_iterator<>, _Size, _CharT*)): New declaration.
    (__copy_n(_IIte, _Size, _OIte, input_iterator_tag)): Adapt to use
    latter.
    * include/bits/streambuf_iterator.h (istreambuf_iterator<>): Declare
    std::__copy_n_a friend.
    (__copy_n_a(istreambuf_iterator<>, _Size, _CharT*)): New.
    * testsuite/25_algorithms/copy_n/istreambuf_iterator/1.cc: New.
    * testsuite/25_algorithms/copy_n/istreambuf_iterator/1_neg.cc: New.
    * testsuite/25_algorithms/copy_n/istreambuf_iterator/2_neg.cc: New.

Tested under Linux x86_64.

Ok to commit ?

François

diff --git a/libstdc++-v3/include/bits/stl_algo.h b/libstdc++-v3/include/bits/stl_algo.h
index 078efc028cc..ae4f41c53ff 100644
--- a/libstdc++-v3/include/bits/stl_algo.h
+++ b/libstdc++-v3/include/bits/stl_algo.h
@@ -778,14 +778,28 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template<typename _InputIterator, typename _Size, typename _OutputIterator>
     _GLIBCXX20_CONSTEXPR
     _OutputIterator
-    __copy_n(_InputIterator __first, _Size __n,
-	     _OutputIterator __result, input_iterator_tag)
+    __copy_n_a(_InputIterator __first, _Size __n, _OutputIterator __result)
     {
       for (; __n > 0; --__n, (void)++__first, (void)++__result)
 	*__result = *__first;
       return __result;
     }
 
+  template<typename _CharT, typename _Traits, typename _Size>
+    _CharT*
+    __copy_n_a(istreambuf_iterator<_CharT, _Traits>, _Size, _CharT*);
+
+  template<typename _InputIterator, typename _Size, typename _OutputIterator>
+    _GLIBCXX20_CONSTEXPR
+    _OutputIterator
+    __copy_n(_InputIterator __first, _Size __n,
+	     _OutputIterator __result, input_iterator_tag)
+    {
+      return std::__niter_wrap(__result,
+			       __copy_n_a(__first, __n,
+					  std::__niter_base(__result)));
+    }
+
   template<typename _RandomAccessIterator, typename _Size,
 	   typename _OutputIterator>
     _GLIBCXX20_CONSTEXPR
@@ -870,7 +884,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       return pair<_OutputIterator1, _OutputIterator2>(__out_true, __out_false);
     }
-#endif
+#endif // C++11
 
   template<typename _ForwardIterator, typename _Predicate>
     _GLIBCXX20_CONSTEXPR
diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h b/libstdc++-v3/include/bits/streambuf_iterator.h
index 2f4ff494a3a..b1b6bc129ce 100644
--- a/libstdc++-v3/include/bits/streambuf_iterator.h
+++ b/libstdc++-v3/include/bits/streambuf_iterator.h
@@ -80,6 +80,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	__copy_move_a2(istreambuf_iterator<_CharT2>,
 		       istreambuf_iterator<_CharT2>, _CharT2*);
 
+#if __cplusplus >= 201103L
+      template<typename _CharT2, typename _Traits2, typename _Size>
+	friend _CharT2*
+	__copy_n_a(istreambuf_iterator<_CharT2, _Traits2>, _Size, _CharT2*);
+#endif
+
       template<typename _CharT2>
 	friend typename __gnu_cxx::__enable_if<__is_char<_CharT2>::__value,
 				    istreambuf_iterator<_CharT2> >::__type
@@ -367,6 +373,27 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       return __result;
     }
 
+#if __cplusplus >= 201103L
+  template<typename _CharT, typename _Traits, typename _Size>
+    _CharT*
+    __copy_n_a(istreambuf_iterator<_CharT, _Traits> __it,
+	       _Size __n, _CharT* __result)
+    {
+      if (__n == 0)
+	return __result;
+
+      __glibcxx_requires_cond(__it._M_sbuf,
+			      _M_message(__gnu_debug::__msg_inc_istreambuf)
+			      ._M_iterator(__it));
+      _CharT* __beg = __result;
+      __result += __it._M_sbuf->sgetn(__beg, __n);
+      __glibcxx_requires_cond(__result - __beg == __n,
+			      _M_message(__gnu_debug::__msg_inc_istreambuf)
+			      ._M_iterator(__it));
+      return __result;
+    }
+#endif // C++11
+
   template<typename _CharT>
     typename __gnu_cxx::__enable_if<__is_char<_CharT>::__value,
 		  		    istreambuf_iterator<_CharT> >::__type
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy_n/istreambuf_iterator/1.cc b/libstdc++-v3/testsuite/25_algorithms/copy_n/istreambuf_iterator/1.cc
new file mode 100644
index 00000000000..87d37b6b9ca
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/copy_n/istreambuf_iterator/1.cc
@@ -0,0 +1,73 @@
+// 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;
+  auto res = std::copy_n(istrb_ite(ss), 0, ostr.begin());
+  VERIFY( res == ostr.begin() );
+  VERIFY( ostr.front() == '0' );
+
+  res = std::copy_n(istrb_ite(ss), 2, ostr.begin());
+  VERIFY( res == ostr.begin() + 2 );
+  VERIFY( ostr == "12000" );
+
+  res = std::copy_n(istrb_ite(ss), 3, ostr.begin() + 2);
+  VERIFY( res == ostr.begin() + 5 );
+  VERIFY( ostr == "12345" );
+}
+
+void test02()
+{
+  std::stringstream ss("12345");
+
+  std::string ostr(5, '0');
+  typedef std::istreambuf_iterator<char> istrb_ite;
+
+  istrb_ite ibfit(ss);
+  auto res = std::copy_n(ibfit, 3, std::copy_n(ibfit, 2, ostr.begin()));
+  VERIFY( res == ostr.begin() + 5 );
+  VERIFY( ostr == "12345" );
+}
+
+void test03()
+{
+  std::string ostr(5, '0');
+  typedef std::istreambuf_iterator<char> istrb_ite;
+
+  auto res = std::copy_n(istrb_ite(), 0, ostr.begin());
+  VERIFY( res == ostr.begin() );
+}
+
+int main()
+{
+  test01();
+  test02();
+  test03();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy_n/istreambuf_iterator/1_neg.cc b/libstdc++-v3/testsuite/25_algorithms/copy_n/istreambuf_iterator/1_neg.cc
new file mode 100644
index 00000000000..42bc5b64306
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/copy_n/istreambuf_iterator/1_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;
+}
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy_n/istreambuf_iterator/2_neg.cc b/libstdc++-v3/testsuite/25_algorithms/copy_n/istreambuf_iterator/2_neg.cc
new file mode 100644
index 00000000000..cede27a248d
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/copy_n/istreambuf_iterator/2_neg.cc
@@ -0,0 +1,38 @@
+// 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::string ostr(10, '0');
+  typedef std::istreambuf_iterator<char> istrb_ite;
+  std::copy_n(istrb_ite(), 10, ostr.begin());
+}
+
+int main()
+{
+  test01();
+  return 0;
+}

Reply via email to