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.133.124]) by sourceware.org (Postfix) with ESMTPS id B537A385DC0F for ; Thu, 30 Nov 2023 15:53:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B537A385DC0F 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 B537A385DC0F Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701359607; cv=none; b=QfYgWG/vahI0dV10YKQWDu/SFpSi490k6XbS1VAZfJMNd/6yuGAgQ55OFxR0DTKHRlSqGof8x+SUDjIEFNi2bWoD6avIXRMvYAz7LDQrTDFZbswovbkUFcjpH8bnBn6BwJTNNRQnBxN5R5xBbOBMLVRxgdDpy9+nwjLPooBYyTU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701359607; c=relaxed/simple; bh=H5CZTNeOW9hXTrN3ujxhCPGQwwtRM4tBjfgVo7Tbac0=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=d1uXqvYrOzkpC5dqpF60mi+6U3fVqMxeB20jTvfVzTZnLuf2NOLVgX8t+d52bNNDEtSBcsbTRUN4oJf2rlbaMI8i3rcVx/hPyW2H7IpqfPf/xwAdqcAH8muKtlMZjJBguTbeYuWa/oHZHtKwkmgAGnWHt+dQL84RguQeBIVu/os= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1701359605; 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=mpavrfUcWlZYyUQK4Z/A8hCOkvI+cQs1rM4jiGy1HS8=; b=S6k4WmVDMiln1Nl0obayOAM90Uc7GuF+6RwFuLSAwYiTv9eGCF/KphSgeXeBSqCizEHoIS BNffrBu+PmI3caEtGqEFTpFNnW5xOfmQRQkIbFyaOU9LFQQMktDzQi79vhuJr8wIu2xvNj jDbN6BGTpprfwWHBc+MWh6J6OToVrp8= Received: from mail-yb1-f197.google.com (mail-yb1-f197.google.com [209.85.219.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-292-Wpdmr3KyPJuQoGzqpvtMhg-1; Thu, 30 Nov 2023 10:53:23 -0500 X-MC-Unique: Wpdmr3KyPJuQoGzqpvtMhg-1 Received: by mail-yb1-f197.google.com with SMTP id 3f1490d57ef6-db548f8dae4so11101276.3 for ; Thu, 30 Nov 2023 07:53:23 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701359603; x=1701964403; 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=mpavrfUcWlZYyUQK4Z/A8hCOkvI+cQs1rM4jiGy1HS8=; b=UJo2hKjpDDonaJ3IdmZQae54oB3PDgkRyz1Z1dvcsR1iOPuYmAvpFSzD0vg0UlB7d2 4NlyBuJzRpocEDv5Ug9rkWu+jaMhFrbdlZPqn6gSvJUGuNaDeAcf1Zuc72uK6JLWwfLa BZC0o6nW2oZ8dLNwhwqtxF+apDpZLgV9vb5ZIXNmiiHG9CPYipUTVpjw/Eugv5QoLM33 MVRilSNjM9q2FKhOM4HHHeOeWEWSmbMXJaNw0dm+tD4AsQ6/JNS0uf1+EifSbCo/2xVe kRFJ1eVi97RRgarPxhvH7RBqfJQDUs5SGwAqb8xz93govxGoMVdUiDLzFacw2caG7Hqk 8EAg== X-Gm-Message-State: AOJu0YwxIFygq/BRFhSRlm/+mw/ib4GjoBLV1ZidLg7wQrhyZxSBiNch EXoSViS9/J9+lgLNFO6bzY/ghxdbTAFt50enYgNC1mVDeevSCV5+LuHw/V8HAunQKOFMc1HyWS6 W4SvKlSrOfXEM2cmqsGkcmssHasv0nzkCpLYexwRUMA== X-Received: by 2002:a25:f81a:0:b0:d9c:2c30:ebb1 with SMTP id u26-20020a25f81a000000b00d9c2c30ebb1mr23454601ybd.12.1701359603274; Thu, 30 Nov 2023 07:53:23 -0800 (PST) X-Google-Smtp-Source: AGHT+IFnb/aFdhE55gGqNfSsJIoKMaVGAPiAjLBm2W5ozq57F1aBAxwUtfoF54BdKvOHQLERSypYcafUjoF74cqOQTk= X-Received: by 2002:a25:f81a:0:b0:d9c:2c30:ebb1 with SMTP id u26-20020a25f81a000000b00d9c2c30ebb1mr23454579ybd.12.1701359602838; Thu, 30 Nov 2023 07:53:22 -0800 (PST) MIME-Version: 1.0 References: <20231130155041.74365-1-jwakely@redhat.com> In-Reply-To: <20231130155041.74365-1-jwakely@redhat.com> From: Jonathan Wakely Date: Thu, 30 Nov 2023 15:53:12 +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=-12.2 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_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Before pushing I'll fix the summary to say "LWG" instead of "LGW" (the airport code for London Gatwick!) 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 >