From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30492 invoked by alias); 9 Oct 2019 20:18:21 -0000 Mailing-List: contact libstdc++-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libstdc++-owner@gcc.gnu.org Received: (qmail 30474 invoked by uid 89); 9 Oct 2019 20:18:21 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-16.4 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,HTML_MESSAGE,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=2_negcc, hadnt, hadn't, UD:2_neg.cc X-HELO: mail-lj1-f175.google.com Received: from mail-lj1-f175.google.com (HELO mail-lj1-f175.google.com) (209.85.208.175) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 09 Oct 2019 20:18:18 +0000 Received: by mail-lj1-f175.google.com with SMTP id d1so3767644ljl.13 for ; Wed, 09 Oct 2019 13:18:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=7N19gNBQJUIlRq9z5e1Okp5f32rAODMFBm0dpbjbeKQ=; b=hq+c5hlklBPHHV89XoRZz0qtuau8b1zKtOquaUPLg7e8UulNAuHeejjFnoba1/uPyH CQXa/ZxJIjFjqk7mMspc8l2La+ZNSYQIoltUhMVro/xNI7rAvlBSNE11rpczUcb2FoX5 qusRqWejbeDdV9pmDaAQ047XTslIohMk/OADg5SEJ+enAXSbGoGVLgIVvFadEneisYLi 9/aJrHWCuprDGpiq5mincNojU7Wz3bb8ekMiEyXKCq3/jLdsDS+71iyFB4zI8y8dZ8Ro lV6oSCIm/7NCZuRjpDXjI9x9Q2ySMFaHjY+LsmsSUb0pI4Q9dX71Gnp6ivQLhPCoHndL 6qZw== MIME-Version: 1.0 References: <20190927110056.GA18624@redhat.com> <4717f824-2fdc-0c12-dd50-82b6d937d485@gmail.com> In-Reply-To: <4717f824-2fdc-0c12-dd50-82b6d937d485@gmail.com> From: Christophe Lyon Date: Wed, 09 Oct 2019 20:18:00 -0000 Message-ID: Subject: Re: Add std::copy_n overload for istreambuf_iterator To: =?UTF-8?Q?Fran=C3=A7ois_Dumont?= Cc: Jonathan Wakely , "libstdc++@gcc.gnu.org" , gcc-patches Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-SW-Source: 2019-10/txt/msg00034.txt.bz2 On Fri, 4 Oct 2019 at 07:01, Fran=C3=A7ois Dumont wr= ote: > On 9/27/19 1:00 PM, Jonathan Wakely wrote: > > On 19/07/19 23:37 +0200, Fran=C3=A7ois Dumont wrote: > >> It sounds reasonable to overload std::copy_n for istreambuf_iterator. > > I wonder whether it's worth doing: > > > > #if __cplusplus >=3D 201703L > > if constexpr (is_same_v<_OutputIterator, _CharT*>) > > return __result + __it._M_sbuf->sgetn(__result, __n); > > else > > { > > #endif > > ... > > #if __cplusplus >=3D 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 __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 >=3D 201103L > >> + template >> _OutputIterator> > >> + friend typename enable_if<__is_char<_CharT2>::__value, > >> + _OutputIterator>::type > >> + copy_n(istreambuf_iterator<_CharT2>, _Size, _OutputIterator); > >> +#endif > >> + > >> template > >> 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 > >> +// . > >> + > >> +// { dg-do run { target c++11 } } > >> + > >> +#include > >> +#include > >> +#include > >> + > >> +#include > >> + > >> +void test01() > >> +{ > >> + std::stringstream ss("12345"); > >> + > >> + std::string ostr(5, '0'); > >> + typedef std::istreambuf_iterator istrb_ite; > >> + std::copy_n(istrb_ite(ss), 0, ostr.begin()); > >> + VERIFY( ostr.front() =3D=3D '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 =3D=3D "12000" ); > > > > And equal to ostr.begin() + 2. > > > >> + > >> + std::copy_n(istrb_ite(ss), 3, ostr.begin() + 2); > >> + VERIFY( ostr =3D=3D "12345" ); > > > > And equal to ostr.begin() + 5 here. > Done. > > > >> +} > >> + > >> +void test02() > >> +{ > >> + std::stringstream ss("12345"); > >> + > >> + std::string ostr(5, '0'); > >> + typedef std::istreambuf_iterator istrb_ite; > >> + > >> + istrb_ite ibfit(ss); > >> + std::copy_n(ibfit, 3, std::copy_n(ibfit, 2, ostr.begin())); > >> + VERIFY( ostr =3D=3D "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 test= s. > > * 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. > Hi, Maybe this has already been reported, but I've noticed that this new test fails on arm & aarch64. My libstdc++.log says: /libstdc++-v3/testsuite/25_algorithms/copy_n/istreambuf_iterator/1.cc:42: void test01(): Assertion 'ostr =3D=3D "12345"' failed. Christophe * 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=C3=A7ois > >