[PATH 2/3] libstdc++: Simplify std::advance istreambuf_iterator overload

[PATH 2/3] libstdc++: Simplify std::advance istreambuf_iterator overload

[François Dumont]

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

libstdc++: Use only public basic_streambuf methods in std::advance overload

std::advance overload for istreambuf_iterator can be implemented using
basic_streambuf public pubseekoff method so that it doesn't have to be
basic_streambuf friend.

libstdc++-v3/ChangeLog:

         * include/std/streambuf
         (advance(istreambuf_iterator<>&, _Distance)): Remove friend 
declaration.
         * include/bits/streambuf_iterator.h
         (advance(istreambuf_iterator<>&, _Distance)): Re-implement using
         streambuf pubseekoff.
         * 
testsuite/25_algorithms/advance/istreambuf_iterators/char/3.cc: New
         test.

Tested under Linux x86_64.

Ok to commit ?

François


[-- Attachment #2: advance_istreambuf_iterator.patch --]
[-- Type: text/x-patch, Size: 4414 bytes --]

diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h b/libstdc++-v3/include/bits/streambuf_iterator.h
index 8712b90edd6..afe967e5f03 100644
--- a/libstdc++-v3/include/bits/streambuf_iterator.h
+++ b/libstdc++-v3/include/bits/streambuf_iterator.h
@@ -453,37 +453,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       __glibcxx_assert(__n > 0);
       __glibcxx_requires_cond(!__i._M_at_eof(),
-			      _M_message(__gnu_debug::__msg_inc_istreambuf)
-			      ._M_iterator(__i));
-
-      typedef istreambuf_iterator<_CharT>		   __is_iterator_type;
-      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();
-
-      streambuf_type* __sb = __i._M_sbuf;
-      while (__n > 0)
+			      _M_message(__gnu_debug::__msg_advance_oob)
+			      ._M_iterator(__i)
+			      ._M_integer(__n));
+
+      typedef basic_streambuf<_CharT> __streambuf_t;
+      typedef typename __streambuf_t::pos_type __pos_t;
+      __pos_t __cur_pos
+	= __i._M_sbuf->pubseekoff(0, ios_base::cur, ios_base::in);
+      __pos_t __new_pos
+	= __i._M_sbuf->pubseekoff(__n, ios_base::cur, ios_base::in);
+      __i._M_c = char_traits<_CharT>::eof();
+
+      if (__new_pos - __cur_pos != __n)
 	{
-	  streamsize __size = __sb->egptr() - __sb->gptr();
-	  if (__size > __n)
-	    {
-	      __sb->__safe_gbump(__n);
-	      break;
-	    }
-
-	  __sb->__safe_gbump(__size);
-	  __n -= __size;
-	  if (traits_type::eq_int_type(__sb->underflow(), __eof))
-	    {
-	      __glibcxx_requires_cond(__n == 0,
-				_M_message(__gnu_debug::__msg_inc_istreambuf)
-				._M_iterator(__i));
-	      break;
-	    }
+	  __i._M_sbuf = 0;
+	  __glibcxx_requires_cond(!__i._M_at_eof(),
+				  _M_message(__gnu_debug::__msg_advance_oob)
+				  ._M_iterator(__i)
+				  ._M_integer(__n));
 	}
-
-      __i._M_c = __eof;
     }
 
 // @} group iterators
diff --git a/libstdc++-v3/include/std/streambuf b/libstdc++-v3/include/std/streambuf
index 13db284eb58..53892636e47 100644
--- a/libstdc++-v3/include/std/streambuf
+++ b/libstdc++-v3/include/std/streambuf
@@ -155,11 +155,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
         find(istreambuf_iterator<_CharT2>, istreambuf_iterator<_CharT2>,
 	     const _CharT2&);
 
-      template<typename _CharT2, typename _Distance>
-        friend typename __gnu_cxx::__enable_if<__is_char<_CharT2>::__value,
-					       void>::__type
-        advance(istreambuf_iterator<_CharT2>&, _Distance);
-
       friend void __istream_extract(istream&, char*, streamsize);
 
       template<typename _CharT2, typename _Traits2, typename _Alloc>
diff --git a/libstdc++-v3/testsuite/25_algorithms/advance/istreambuf_iterators/char/3.cc b/libstdc++-v3/testsuite/25_algorithms/advance/istreambuf_iterators/char/3.cc
new file mode 100644
index 00000000000..06b38fea91e
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/advance/istreambuf_iterators/char/3.cc
@@ -0,0 +1,49 @@
+// Copyright (C) 2020 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/>.
+
+// Debug mode would detect the invalid std::advance call.
+// { dg-require-normal-mode "" }
+
+#include <iterator>
+#include <sstream>
+#include <algorithm>
+
+#include <testsuite_hooks.h>
+
+void test01()
+{
+  using namespace std;
+
+  typedef istreambuf_iterator<char> in_iterator_type;
+
+  const char data1[] = "Drei Phantasien nach Friedrich Holderlin";
+  istringstream iss1(data1);
+  in_iterator_type beg1(iss1);
+  in_iterator_type end1;
+
+  VERIFY( beg1 != end1 );
+
+  advance(beg1, sizeof(data1));
+
+  VERIFY( beg1 == end1 );
+}
+
+int main()
+{
+  test01();
+  return 0;
+}

Re: [PATH 2/3] libstdc++: Simplify std::advance istreambuf_iterator overload

[Jonathan Wakely]

On 09/09/20 22:12 +0200, François Dumont via Libstdc++ wrote:
>libstdc++: Use only public basic_streambuf methods in std::advance overload
>
>std::advance overload for istreambuf_iterator can be implemented using
>basic_streambuf public pubseekoff method so that it doesn't have to be
>basic_streambuf friend.
>
>libstdc++-v3/ChangeLog:
>
>        * include/std/streambuf
>        (advance(istreambuf_iterator<>&, _Distance)): Remove friend 
>declaration.
>        * include/bits/streambuf_iterator.h
>        (advance(istreambuf_iterator<>&, _Distance)): Re-implement using
>        streambuf pubseekoff.
>        * 
>testsuite/25_algorithms/advance/istreambuf_iterators/char/3.cc: New
>        test.
>
>Tested under Linux x86_64.
>
>Ok to commit ?
>
>François
>

>diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h b/libstdc++-v3/include/bits/streambuf_iterator.h
>index 8712b90edd6..afe967e5f03 100644
>--- a/libstdc++-v3/include/bits/streambuf_iterator.h
>+++ b/libstdc++-v3/include/bits/streambuf_iterator.h
>@@ -453,37 +453,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 
>       __glibcxx_assert(__n > 0);
>       __glibcxx_requires_cond(!__i._M_at_eof(),
>-			      _M_message(__gnu_debug::__msg_inc_istreambuf)
>-			      ._M_iterator(__i));
>-
>-      typedef istreambuf_iterator<_CharT>		   __is_iterator_type;
>-      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();
>-
>-      streambuf_type* __sb = __i._M_sbuf;
>-      while (__n > 0)
>+			      _M_message(__gnu_debug::__msg_advance_oob)
>+			      ._M_iterator(__i)
>+			      ._M_integer(__n));
>+
>+      typedef basic_streambuf<_CharT> __streambuf_t;
>+      typedef typename __streambuf_t::pos_type __pos_t;
>+      __pos_t __cur_pos
>+	= __i._M_sbuf->pubseekoff(0, ios_base::cur, ios_base::in);
>+      __pos_t __new_pos
>+	= __i._M_sbuf->pubseekoff(__n, ios_base::cur, ios_base::in);

Two seeks might be considerably more expensive than the existing code.
For example, advancing by __n=1 is very cheap if the get area already
contains data, we just bump a pointer. Two seeks does virtual calls
to the derived streambuf which does ... something? We don't know, it
depends on the streambuf. It might even fail and return
pos_type(off_type(-1)). 

I don't think this is an improvement.