From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id E41913865C26 for ; Tue, 5 Dec 2023 23:35:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E41913865C26 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E41913865C26 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701819352; cv=none; b=ezCbXGLkZmifRgVLzOC9ugWQ3Jli90AUwkdl9LK5ytx9xuzcpjIrAm75FA5ar+6r+yHqwkFDFcI42C2qXwR0tvy0f1UIblGJt3ozgMKhFCTKzdm2ozR+dVFyAqZYpNfqKtjMaMdKtxDwh1G35oMvl7AyKMws2ObSXbkVg1Ijbjk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701819352; c=relaxed/simple; bh=6enEkY0dI7wmTHkxnSnFOZXFj1pv6/meRKXoEtBl//8=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=vwrD+NkiiTqkz/UHfPK1KjghcFfmoWcNjYfeYI39gYWh06ZAoe3TcL86l4cHRm1FBfYlIsFWV0lHJ1R2GmJZrtJml2BanGc7b/ojYbqvmZEqfy+7d3ZsqoV/mWie9+dR8SrZTzlRDAWXkV3O8lcovhX3QMiSFWfQEb60G93zGpU= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1701819349; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=0Swj1mu/X362qHPBvxxbwJdbPFYMSLAuT+zveQ4IB9g=; b=H7Lp5rcilUFUoHTKgDwv9dFozUIReJ5PvzcVWFgfRcfewq2YZdxPYsbbqMCyI9DKgLsBSX Vwhcli8aqmAS+dGtdrrKKN60Ep23EYl8dsIHrjCy/z+6DcKLzc5LEUY/C8WC72DjXiEE/P 3O0XceQFCxJu+z2EFbfKnDyw6tODOOM= Received: from mail-yw1-f199.google.com (mail-yw1-f199.google.com [209.85.128.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-589-kLBNjByNPQuphbqdpPBKCQ-1; Tue, 05 Dec 2023 18:35:47 -0500 X-MC-Unique: kLBNjByNPQuphbqdpPBKCQ-1 Received: by mail-yw1-f199.google.com with SMTP id 00721157ae682-5d7e7e10231so3088057b3.1 for ; Tue, 05 Dec 2023 15:35:47 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701819347; x=1702424147; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=0Swj1mu/X362qHPBvxxbwJdbPFYMSLAuT+zveQ4IB9g=; b=DzES1GUTAAzE/FGtjhDReWZap3WHilpYRxzyVddxhJGv4OVP3/8iR146YHCiSwihGC B68X4ztw3RgBugWRsWeekd0wDjExAg2gFtnlIKCnNZAwYYDcTGEB5EOkdUh4rlkjUTE3 0a8a/SLoyXFWl4VEjPqytJ1FPnpo8Tv7nmBMLf37S8BDJ6UHyM91zYQxcFrmDX8LRcAK 98V7cYTByVpglh9YiyLnAZQP/MagKSTvEo0OqKUd6wrK/ddpx25tA0yygG5xxY8am+wj u3J5mrk2n5s1s+Jw7ztqGJS/JhlfheXLJMima7gscIAiH+BikFA4r20kSKDeCcWCoVKV 309A== X-Gm-Message-State: AOJu0Yzf+eOZPt2IALU5nTfsEB0l/sxYw50WwAFbeUpIn3PrUsof0e9+ aTISexgssp6Ke/bHHEvHFuQZ/uXGmxBP2zVNWzuhfrlT2glAuZTpJOpxIDgjr00C5h5iPNB/GPW Uou4xoY5XksR+0mp1p5zIZqjH8QhyK4SDnQ== X-Received: by 2002:a81:84c3:0:b0:5d6:bc5c:9770 with SMTP id u186-20020a8184c3000000b005d6bc5c9770mr1799548ywf.4.1701819347184; Tue, 05 Dec 2023 15:35:47 -0800 (PST) X-Google-Smtp-Source: AGHT+IH3I71phCrIRztWX+acCzZUmc7L/ftoZGILZWhwu8P+EbkGC2iiZWcpO/D9OXQKFjvV0art+qq6E1MNsLfnMVs= X-Received: by 2002:a81:84c3:0:b0:5d6:bc5c:9770 with SMTP id u186-20020a8184c3000000b005d6bc5c9770mr1799543ywf.4.1701819346803; Tue, 05 Dec 2023 15:35:46 -0800 (PST) MIME-Version: 1.0 References: <20231130155041.74365-1-jwakely@redhat.com> In-Reply-To: From: Jonathan Wakely Date: Tue, 5 Dec 2023 23:35:35 +0000 Message-ID: Subject: Re: [PATCH] libstdc++: Implement LGW 4016 for std::ranges::to To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-13.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, 30 Nov 2023 at 15:53, Jonathan Wakely wrote: > > Before pushing I'll fix the summary to say "LWG" instead of "LGW" (the > airport code for London Gatwick!) Pushed to trunk now. > > On Thu, 30 Nov 2023 at 15:51, Jonathan Wakely wrote: > > > > This hasn't been finally approved by LWG yet, but everybody seems to be > > in favour of it. I think I'll push this soon. > > > > Tested x86_64-linux. > > > > -- >8 -- > > > > This implements the proposed resolution of LWG 4016, so that > > std::ranges::to does not use std::back_inserter and std::inserter. > > Instead it inserts at the back of the container directly, using > > the first supported one of emplace_back, push_back, emplace, and insert. > > > > Using emplace avoids creating a temporary that has to be moved into the > > container, for cases where the source range and the destination > > container do not have the same value type. > > > > libstdc++-v3/ChangeLog: > > > > * include/std/ranges (__detail::__container_insertable): Remove. > > (__detail::__container_inserter): Remove. > > (ranges::to): Use emplace_back or emplace, as per LWG 4016. > > * testsuite/std/ranges/conv/1.cc (Cont4, test_2_1_4): Check for > > use of emplace_back and emplace. > > --- > > libstdc++-v3/include/std/ranges | 50 +++---- > > libstdc++-v3/testsuite/std/ranges/conv/1.cc | 149 ++++++++++++++++---- > > 2 files changed, 144 insertions(+), 55 deletions(-) > > > > diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges > > index 9d4c2e01c4d..afd0a38e0cf 100644 > > --- a/libstdc++-v3/include/std/ranges > > +++ b/libstdc++-v3/include/std/ranges > > @@ -9229,26 +9229,6 @@ namespace __detail > > { __c.max_size() } -> same_as; > > }; > > > > - template > > - constexpr bool __container_insertable > > - = requires(_Container& __c, _Ref&& __ref) { > > - typename _Container::value_type; > > - requires ( > > - requires { __c.push_back(std::forward<_Ref>(__ref)); } > > - || requires { __c.insert(__c.end(), std::forward<_Ref>(__ref)); } > > - ); > > - }; > > - > > - template > > - constexpr auto > > - __container_inserter(_Container& __c) > > - { > > - if constexpr (requires { __c.push_back(std::declval<_Ref>()); }) > > - return std::back_inserter(__c); > > - else > > - return std::inserter(__c, __c.end()); > > - } > > - > > template > > constexpr bool __toable = requires { > > requires (!input_range<_Cont> > > @@ -9301,17 +9281,33 @@ namespace __detail > > std::forward<_Args>(__args)...); > > else > > { > > - using __detail::__container_insertable; > > - using __detail::__reservable_container; > > using _RefT = range_reference_t<_Rg>; > > static_assert(constructible_from<_Cont, _Args...>); > > - static_assert(__container_insertable<_Cont, _RefT>); > > _Cont __c(std::forward<_Args>(__args)...); > > - if constexpr (sized_range<_Rg> && __reservable_container<_Cont>) > > + if constexpr (sized_range<_Rg> > > + && __detail::__reservable_container<_Cont>) > > __c.reserve(static_cast>(ranges::size(__r))); > > - auto __ins = __detail::__container_inserter<_RefT>(__c); > > - for (auto&& __e : __r) > > - *__ins++ = std::forward(__e); > > + // _GLIBCXX_RESOLVE_LIB_DEFECTS > > + // 4016. container-insertable checks do not match what > > + // container-inserter does > > + auto __it = ranges::begin(__r); > > + const auto __sent = ranges::end(__r); > > + while (__it != __sent) > > + { > > + if constexpr (requires { __c.emplace_back(*__it); }) > > + __c.emplace_back(*__it); > > + else if constexpr (requires { __c.push_back(*__it); }) > > + __c.push_back(*__it); > > + else > > + { > > + auto __end = __c.end(); > > + if constexpr (requires { __c.emplace(__end, *__it); }) > > + __end = __c.emplace(__end, *__it); > > + else > > + __end = __c.insert(__end, *__it); > > + } > > + ++__it; > > + } > > return __c; > > } > > } > > diff --git a/libstdc++-v3/testsuite/std/ranges/conv/1.cc b/libstdc++-v3/testsuite/std/ranges/conv/1.cc > > index 4b6814b1add..b5f861dedb3 100644 > > --- a/libstdc++-v3/testsuite/std/ranges/conv/1.cc > > +++ b/libstdc++-v3/testsuite/std/ranges/conv/1.cc > > @@ -203,33 +203,51 @@ test_2_1_3() > > VERIFY( c2.c.get_allocator() == Alloc(78) ); > > } > > > > -template > > +enum AppendKind { None, EmplaceBack, PushBack, Emplace, Insert }; > > + > > +template > > struct Cont4 > > { > > - using value_type = typename C::value_type; > > - > > // Only support construction with no args or an allocator. > > - // This forces the use of either push_back or insert to fill the container. > > + // This forces the use of either emplace_back, push_back, emplace or insert. > > Cont4() { } > > Cont4(typename C::allocator_type a) : c(a) { } > > > > - // Satisfying container-insertable requires either this ... > > template > > - requires UsePushBack > > + requires (Kind <= EmplaceBack) > > + && requires(C& c, T&& t) { c.emplace_back(std::forward(t)); } > > + void > > + emplace_back(T&& t) > > + { > > + kind = EmplaceBack; > > + c.emplace_back(std::forward(t)); > > + } > > + > > + template > > + requires (Kind <= PushBack) > > && requires(C& c, T&& t) { c.push_back(std::forward(t)); } > > void > > push_back(T&& t) > > { > > + kind = PushBack; > > c.push_back(std::forward(t)); > > - used_push_back = true; > > } > > > > - // ... or this: > > + template > > + requires (Kind <= Emplace) > > + && requires(C& c, T&& t) { c.emplace(c.end(), std::forward(t)); } > > + typename C::iterator > > + emplace(typename C::iterator, T&& t) > > + { > > + kind = Emplace; > > + return c.emplace(c.end(), std::forward(t)); > > + } > > + > > template > > typename C::iterator > > insert(typename C::iterator, T&& t) > > { > > - used_push_back = false; > > + kind = Insert; > > return c.insert(c.end(), std::forward(t)); > > } > > > > @@ -254,7 +272,7 @@ struct Cont4 > > auto max_size() const { return c.max_size(); } > > > > C c; > > - bool used_push_back = false; > > + AppendKind kind{}; > > bool used_reserve = false; > > }; > > > > @@ -265,38 +283,113 @@ test_2_1_4() > > // container-insertable> > > > > using Alloc = __gnu_test::uneq_allocator; > > + using Alloc2 = __gnu_test::uneq_allocator; > > using V = std::vector; > > + using List = std::list; > > > > std::list l{1u, 2u, 3u}; > > - auto c = std::ranges::to>(l); > > - static_assert(std::is_same_v>); > > + std::list l2{4l, 5l, 6l}; > > + > > + // use vector::emplace_back and vector::reserve > > + auto c = std::ranges::to>(l); > > + static_assert(std::is_same_v>); > > VERIFY( c.c == V(l.begin(), l.end()) ); > > - VERIFY( c.used_push_back ); > > + VERIFY( c.kind == EmplaceBack ); > > VERIFY( c.used_reserve ); > > > > - std::list l2{4l, 5l, 6l}; > > - auto c2 = std::ranges::to>(l2, Alloc(78)); > > - static_assert(std::is_same_v>); > > + // use vector::emplace_back and vector::reserve > > + auto c2 = std::ranges::to>(l2, Alloc(78)); > > + static_assert(std::is_same_v>); > > VERIFY( c2.c == V(l2.begin(), l2.end()) ); > > VERIFY( c2.c.get_allocator() == Alloc(78) ); > > - VERIFY( c2.used_push_back ); > > + VERIFY( c2.kind == EmplaceBack ); > > VERIFY( c2.used_reserve ); > > > > - using Alloc2 = __gnu_test::uneq_allocator; > > - using List = std::list; > > - auto c3 = std::ranges::to>(c.c, Alloc2(99)); > > - static_assert(std::is_same_v>); > > + // use list::emplace_back > > + auto c3 = std::ranges::to>(c.c, Alloc2(99)); > > + static_assert(std::is_same_v>); > > VERIFY( c3.c == List(l.begin(), l.end()) ); > > VERIFY( c3.c.get_allocator() == Alloc(99) ); > > - VERIFY( c3.used_push_back ); > > + VERIFY( c3.kind == EmplaceBack ); > > VERIFY( ! c3.used_reserve ); > > > > - auto c4 = std::ranges::to>(c.c, Alloc2(111)); > > - static_assert(std::is_same_v>); > > - VERIFY( c4.c == List(l.begin(), l.end()) ); > > - VERIFY( c4.c.get_allocator() == Alloc(111) ); > > - VERIFY( ! c4.used_push_back ); > > - VERIFY( ! c4.used_reserve ); > > + // use vector::push_back and vector::reserve > > + auto c4 = std::ranges::to>(l); > > + static_assert(std::is_same_v>); > > + VERIFY( c4.c == V(l.begin(), l.end()) ); > > + VERIFY( c4.kind == PushBack ); > > + VERIFY( c4.used_reserve ); > > + > > + // use vector::push_back and vector::reserve > > + auto c5 = std::ranges::to>(l2, Alloc(78)); > > + static_assert(std::is_same_v>); > > + VERIFY( c5.c == V(l2.begin(), l2.end()) ); > > + VERIFY( c5.c.get_allocator() == Alloc(78) ); > > + VERIFY( c5.kind == PushBack ); > > + VERIFY( c5.used_reserve ); > > + > > + // use list::push_back > > + auto c6 = std::ranges::to>(c.c, Alloc2(99)); > > + static_assert(std::is_same_v>); > > + VERIFY( c6.c == List(l.begin(), l.end()) ); > > + VERIFY( c6.c.get_allocator() == Alloc(99) ); > > + VERIFY( c6.kind == PushBack ); > > + VERIFY( ! c6.used_reserve ); > > + > > + // use vector::emplace and vector::reserve > > + auto c7 = std::ranges::to>(l); > > + static_assert(std::is_same_v>); > > + VERIFY( c7.c == V(l.begin(), l.end()) ); > > + VERIFY( c7.kind == Emplace ); > > + VERIFY( c7.used_reserve ); > > + > > + // use vector::emplace and vector::reserve > > + auto c8 = std::ranges::to>(l2, Alloc(78)); > > + static_assert(std::is_same_v>); > > + VERIFY( c8.c == V(l2.begin(), l2.end()) ); > > + VERIFY( c8.c.get_allocator() == Alloc(78) ); > > + VERIFY( c8.kind == Emplace ); > > + VERIFY( c8.used_reserve ); > > + > > + // use list::emplace > > + auto c9 = std::ranges::to>(c.c, Alloc2(99)); > > + static_assert(std::is_same_v>); > > + VERIFY( c9.c == List(l.begin(), l.end()) ); > > + VERIFY( c9.c.get_allocator() == Alloc(99) ); > > + VERIFY( c9.kind == Emplace ); > > + VERIFY( ! c9.used_reserve ); > > + > > + // use vector::insert and vector::reserve > > + auto c10 = std::ranges::to>(l); > > + static_assert(std::is_same_v>); > > + VERIFY( c10.c == V(l.begin(), l.end()) ); > > + VERIFY( c10.kind == Insert ); > > + VERIFY( c10.used_reserve ); > > + > > + // use vector::insert and vector::reserve > > + auto c11 = std::ranges::to>(l2, Alloc(78)); > > + static_assert(std::is_same_v>); > > + VERIFY( c11.c == V(l2.begin(), l2.end()) ); > > + VERIFY( c11.c.get_allocator() == Alloc(78) ); > > + VERIFY( c11.kind == Insert ); > > + VERIFY( c11.used_reserve ); > > + > > + // use list::insert > > + auto c12 = std::ranges::to>(c.c, Alloc2(99)); > > + static_assert(std::is_same_v>); > > + VERIFY( c12.c == List(l.begin(), l.end()) ); > > + VERIFY( c12.c.get_allocator() == Alloc(99) ); > > + VERIFY( c12.kind == Insert ); > > + VERIFY( ! c12.used_reserve ); > > + > > + struct NoCopyPls > > + { > > + NoCopyPls(int) { } > > + NoCopyPls(const NoCopyPls&) { throw; } > > + }; > > + > > + // Uses list::emplace_back(const int&) not list::push_back(NoCopyPls&&). > > + (void) std::ranges::to>(l); > > } > > > > void > > -- > > 2.43.0 > >