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