[PATH 1/3] libstdc++: Simplify std::copy istreambuf_iterator overload

[François Dumont]

On 10/09/20 5:05 pm, Jonathan Wakely wrote:
> On 09/09/20 22:11 +0200, François Dumont via Libstdc++ wrote:
>> libstdc++: Use only public basic_streambuf methods in __copy_move_a2 
>> overload
>>
>> __copy_move_a2 for istreambuf_iterator can be implemented using public
>> basic_streambuf in_avail and sgetn so that __copy_move_a2 do not need 
>> to be
>> basic_streambuf friend.
>>
>> libstdc++-v3/ChangeLog:
>>
>> Â Â Â Â Â Â Â  * include/std/streambuf (__copy_move_a2): Remove 
>> friend declaration.
>> Â Â Â Â Â Â Â  * include/bits/streambuf_iterator.h (__copy_move_a2): 
>> Re-implement using
>> Â Â Â Â Â Â Â  streambuf in_avail and sgetn.
>
> Does this work?
tests are passing yes, and there are a good number of it for this code.
>
> The original code doesn't use any virtual functions except underflow,
> which is called to refill the get area.

I see, I really didn't consider the virtual function calls. So I just 
thought that less friend declaration and simpler code was better, my fault.

>
> The new code calls in_avail() which is equivalent to the old
> __sb->egptr() - __sb->gptr() code. If that returns a non-zero value
> we'll use sgetn() to read that many characters (which makes a virtual
> call to xsgetn). Then we call in_avail() again, which will now make a
> virtual call to showmanyc(). There is no guarantee that showmanyc()
> will return the correct value. It might return a conservative
> underestimate, possibly even zero. If it returns zero then you break
> out of the loop and return, but you haven't reached EOF.

I was concern about the 'If' in the showmanyc doc: If it returns a 
positive value... But I think I forgot when I saw that tests were passing.

So, let's forget this patch.

Thanks for the feedback.

>
>
> Even if showmanyc() returns an accurate value (e.g. for filebuf it can
> use a system call to find the size of the file on disk) you'll now
> make another xsgetn() virtual call and showmanyc() virtual call for
> each buffer's worth of characters. The original code only makes one
> virtual call per buffer's worth of characters.
>
>
>> 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 184c82cd5bf..8712b90edd6 100644
>> --- a/libstdc++-v3/include/bits/streambuf_iterator.h
>> +++ b/libstdc++-v3/include/bits/streambuf_iterator.h
>> @@ -367,31 +367,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>            istreambuf_iterator<_CharT> __last, _CharT* __result)
>>     {
>>       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;
>>
>>       if (__first._M_sbuf && !__last._M_sbuf)
>>     {
>>       streambuf_type* __sb = __first._M_sbuf;
>> -      int_type __c = __sb->sgetc();
>> -      while (!traits_type::eq_int_type(__c, traits_type::eof()))
>> +      std::streamsize __avail = __sb->in_avail();
>> +      while (__avail > 0)
>>         {
>> -          const streamsize __n = __sb->egptr() - __sb->gptr();
>> -          if (__n > 1)
>> -        {
>> -          traits_type::copy(__result, __sb->gptr(), __n);
>> -          __sb->__safe_gbump(__n);
>> -          __result += __n;
>> -          __c = __sb->underflow();
>> -        }
>> -          else
>> -        {
>> -          *__result++ = traits_type::to_char_type(__c);
>> -          __c = __sb->snextc();
>> -        }
>> +          __result += __sb->sgetn(__result, __avail);
>> +          __avail = __sb->in_avail();
>>         }
>>     }
>> +
>>       return __result;
>>     }
>>
>> diff --git a/libstdc++-v3/include/std/streambuf 
>> b/libstdc++-v3/include/std/streambuf
>> index cae35e75bda..13db284eb58 100644
>> --- a/libstdc++-v3/include/std/streambuf
>> +++ b/libstdc++-v3/include/std/streambuf
>> @@ -149,12 +149,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>       friend streamsize
>>       __copy_streambufs_eof<>(basic_streambuf*, basic_streambuf*, 
>> bool&);
>>
>> -      template<bool _IsMove, typename _CharT2>
>> -        friend typename 
>> __gnu_cxx::__enable_if<__is_char<_CharT2>::__value,
>> -                           _CharT2*>::__type
>> -        __copy_move_a2(istreambuf_iterator<_CharT2>,
>> -               istreambuf_iterator<_CharT2>, _CharT2*);
>> -
>>       template<typename _CharT2>
>>         friend typename 
>> __gnu_cxx::__enable_if<__is_char<_CharT2>::__value,
>>                   istreambuf_iterator<_CharT2> >::__type
>