From patchwork Thu Nov 30 15:49:59 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 81041 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9AB30384DEFE for ; Thu, 30 Nov 2023 15:51:09 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org 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 ADED1384DEF9 for ; Thu, 30 Nov 2023 15:50:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org ADED1384DEF9 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 ADED1384DEF9 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=1701359448; cv=none; b=rFcuZMrL/YoIERQ35DbDkBUvzuFHRK5ECARkU34MeFaZ75nEz4yZ/jh726C3SJY4SU+6QxZokx676rVviPioUKtUCjhGJT7eqsHf1V+QrgFIPK7//+uHVgtNDwHivSdbaJNSo1TvEimhQhERqciM9T/Gp0cruZEnuLOyRipG3Kw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701359448; c=relaxed/simple; bh=oHehxU/NBeJEi/OFKowUIsxEGmhwwtud6erO8n2g6DE=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=s34IGbv8Yefc+aDJigoyH2Dx8H8eI78SGcpGhrr+S7DSp/23npZnO/O3rYCP4ZZ9a6+gzyjx+d97/6P1xhKC60hw3SK5trJMUTcBOdf5jspLSvUYe2cz3UPYEuV0VNIyV92oTuqbZqD3EjGxbMzJayujmerHAmNRXpmDPqBLkQY= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1701359444; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=o+28L6bTemhbWi90PLCl/zkF/iA/eaM4x1MTzu7jCIQ=; b=f5L2/a6B+aWVgB9pos3o95lm73hB0ICKQYBs9aV98Vdsu9JxJo1hbd60vG1tPvq3iXqZGS zRxzpcxnjCeOT3Ir7kq9FaId7BKtQClr6FuaX6q/fil/KGSlrm/q8LVcv3UgZaMWjZt/Zc i+hFNLv3VXQGSpUcgdEMlQUmZGnc6qU= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-279-kHCJcHH2O3ufHn0z0DR7rA-1; Thu, 30 Nov 2023 10:50:42 -0500 X-MC-Unique: kHCJcHH2O3ufHn0z0DR7rA-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 64B3B821BC9; Thu, 30 Nov 2023 15:50:42 +0000 (UTC) Received: from localhost (unknown [10.42.28.7]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2D7D12166B26; Thu, 30 Nov 2023 15:50:42 +0000 (UTC) From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [PATCH] libstdc++: Implement LGW 4016 for std::ranges::to Date: Thu, 30 Nov 2023 15:49:59 +0000 Message-ID: <20231130155041.74365-1-jwakely@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.6 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.8 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=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org 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