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 [216.205.24.124]) by sourceware.org (Postfix) with ESMTPS id 535083858436 for ; Thu, 21 Oct 2021 15:23:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 535083858436 Received: from mail-ua1-f71.google.com (mail-ua1-f71.google.com [209.85.222.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-479-G2WEjaVeM7-GCsjAMpHVKg-1; Thu, 21 Oct 2021 11:23:06 -0400 X-MC-Unique: G2WEjaVeM7-GCsjAMpHVKg-1 Received: by mail-ua1-f71.google.com with SMTP id o12-20020ab0150c000000b002cb5393147dso404931uae.9 for ; Thu, 21 Oct 2021 08:23:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=5D5eFmuuSjosi5x9Vuo5MgPLY4dyYdQM+Kz+rnk7gjI=; b=vPySAIXMKeop7B62ijQAgREx86nCnMYTxPZ4X8p3D/LGu4wo0RXnu3aCtwCc0upo2q Qe/X3jnZy2+UYC+mMGkVpZaS4DJmBTlBFOXt16bjEQML+Yg1zYtQXpT4GOkGxgb10B83 T+nnbEcvEXUp4euwHXtW9pZI4Cxq5kcHIFQHyQwAPpaygYMYNofvhEMwhjv77Arm6GGo lN/tMJAXk1hnm4kickFUkxpsjrJ9EXHM/ROI5Jkph618KCUXbqO+7nQkEdQTJZWsXYoS cmF/zuvlK6TaxmMLyI+PdRFziw+SqNntjI/OS/6/gAP3yLsoK51klibI1dhNDnFKtc9s sEkQ== X-Gm-Message-State: AOAM530VuY5suQ2LH+BDtMCjrqxxy0Wye6xmeHHGS05pjCPDS8OE4d0h R+U0hYZWSv3D3ohjTkWeN6bdJcC2dOHX43MtZ/ENsd0XWm4ik+CV7Broc6ALnU0VeSevnaDQPuv rHP4ouN+OmK1X8yrV8AvVOpHL0aH9HjPWfw== X-Received: by 2002:a67:d89b:: with SMTP id f27mr7230865vsj.16.1634829786368; Thu, 21 Oct 2021 08:23:06 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzfe6v/kohVIcm93LG4K6Opx/TXLOsg5Je/OQbLbI4VF3MBpUdVLp2AQyphroXiiWZmT59ppX1SNiwvPtKqCvA= X-Received: by 2002:a67:d89b:: with SMTP id f27mr7230829vsj.16.1634829786092; Thu, 21 Oct 2021 08:23:06 -0700 (PDT) MIME-Version: 1.0 References: <20211019121704.1510908-1-ppalka@redhat.com> <20211019121704.1510908-2-ppalka@redhat.com> In-Reply-To: <20211019121704.1510908-2-ppalka@redhat.com> From: Jonathan Wakely Date: Thu, 21 Oct 2021 16:22:54 +0100 Message-ID: Subject: Re: [PATCH 2/2] libstdc++: Implement P1739R4 changes to views::take/drop/counted To: Patrick Palka Cc: gcc Patches , "libstdc++" X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-14.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, HTML_MESSAGE, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=unavailable autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Oct 2021 15:23:12 -0000 On Tue, 19 Oct 2021 at 13:20, Patrick Palka via Libstdc++ < libstdc++@gcc.gnu.org> wrote: > This implements P1739R4 along with the resolution for LWG 3407 which > corrects the paper's wording. > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? > Yes, thanks. > libstdc++-v3/ChangeLog: > > * include/bits/ranges_util.h (views::_Drop): Forward declare. > (subrange): Befriend views::_Drop. > (subrange::_S_store_size): Declare constexpr instead of just > const, remove obsolete comment. > * include/std/ranges (views::__detail::__is_empty_view): Define. > (views::__detail::__is_basic_string_view): Likewise. > (views::__detail::__is_subrange): Likewise. > (views::__detail::__is_iota_view): Likewise. > (views::__detail::__can_take_view): Rename template parm _Tp to > _Dp. > (views::_Take): Rename template parm _Tp to _Dp, make it > non-deducible > and fix its type to range_difference_t<_Range>. Implement P1739R4 > and > LWG 3407 changes. > (views::__detail::__can_drop_view): Rename template parm _Tp to > _Dp. > (views::_Drop): As with views::_Take. > (views::_Counted): Implement P1739R4 changes. > * include/std/span (__detail::__is_std_span): Rename to ... > (__detail::__is_span): ... this and turn it into a variable > template. > (__detail::__is_std_array): Turn it into a variable template. > (span::span): Adjust uses of __is_std_span and __is_std_array > accordingly. > * testsuite/std/ranges/adaptors/p1739.cc: New test. > --- > libstdc++-v3/include/bits/ranges_util.h | 7 +- > libstdc++-v3/include/std/ranges | 106 +++++++++++++++--- > libstdc++-v3/include/std/span | 12 +- > .../testsuite/std/ranges/adaptors/p1739.cc | 88 +++++++++++++++ > 4 files changed, 192 insertions(+), 21 deletions(-) > create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/p1739.cc > > diff --git a/libstdc++-v3/include/bits/ranges_util.h > b/libstdc++-v3/include/bits/ranges_util.h > index 7e7b958d274..1afa66d298c 100644 > --- a/libstdc++-v3/include/bits/ranges_util.h > +++ b/libstdc++-v3/include/bits/ranges_util.h > @@ -211,6 +211,8 @@ namespace ranges > > } // namespace __detail > > + namespace views { struct _Drop; } > + > enum class subrange_kind : bool { unsized, sized }; > > /// The ranges::subrange class template > @@ -221,8 +223,9 @@ namespace ranges > class subrange : public view_interface> > { > private: > - // XXX: gcc complains when using constexpr here > - static const bool _S_store_size > + friend struct views::_Drop; // Needs to inspect _S_store_size. > + > + static constexpr bool _S_store_size > = _Kind == subrange_kind::sized && !sized_sentinel_for<_Sent, _It>; > > _It _M_begin = _It(); > diff --git a/libstdc++-v3/include/std/ranges > b/libstdc++-v3/include/std/ranges > index 85f232d8fb9..64396027c1b 100644 > --- a/libstdc++-v3/include/std/ranges > +++ b/libstdc++-v3/include/std/ranges > @@ -42,6 +42,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -2147,19 +2148,66 @@ namespace views::__adaptor > { > namespace __detail > { > - template > + template > + constexpr bool __is_empty_view = false; > + > + template > + constexpr bool __is_empty_view> = true; > + > + template > + constexpr bool __is_basic_string_view = false; > + > + template > + constexpr bool __is_basic_string_view _Traits>> = true; > + > + template > + constexpr bool __is_subrange = false; > + > + template > + constexpr bool __is_subrange> = true; > + > + template > + constexpr bool __is_iota_view = false; > + > + template > + constexpr bool __is_iota_view> = true; > + > + template > concept __can_take_view > - = requires { take_view(std::declval<_Range>(), > std::declval<_Tp>()); }; > + = requires { take_view(std::declval<_Range>(), > std::declval<_Dp>()); }; > } // namespace __detail > > struct _Take : __adaptor::_RangeAdaptor<_Take> > { > - template > - requires __detail::__can_take_view<_Range, _Tp> > + template range_difference_t<_Range>> > + requires __detail::__can_take_view<_Range, _Dp> > constexpr auto > - operator() [[nodiscard]] (_Range&& __r, _Tp&& __n) const > + operator() [[nodiscard]] (_Range&& __r, type_identity_t<_Dp> __n) > const > { > - return take_view(std::forward<_Range>(__r), > std::forward<_Tp>(__n)); > + using _Tp = remove_cvref_t<_Range>; > + if constexpr (__detail::__is_empty_view<_Tp>) > + return _Tp(); > + else if constexpr (random_access_range<_Tp> > + && sized_range<_Tp> > + && (std::__detail::__is_span<_Tp> > + || __detail::__is_basic_string_view<_Tp> > + || __detail::__is_subrange<_Tp> > + || __detail::__is_iota_view<_Tp>)) > + { > + __n = std::min<_Dp>(ranges::distance(__r), __n); > + auto __begin = ranges::begin(__r); > + auto __end = __begin + __n; > + if constexpr (std::__detail::__is_span<_Tp>) > + return span(__begin, __end); > + else if constexpr (__detail::__is_basic_string_view<_Tp>) > + return _Tp(__begin, __end); > + else if constexpr (__detail::__is_subrange<_Tp>) > + return subrange>(__begin, __end); > + else > + return iota_view(*__begin, *__end); > + } > + else > + return take_view(std::forward<_Range>(__r), __n); > } > > using _RangeAdaptor<_Take>::operator(); > @@ -2394,19 +2442,49 @@ namespace views::__adaptor > { > namespace __detail > { > - template > + template > concept __can_drop_view > - = requires { drop_view(std::declval<_Range>(), > std::declval<_Tp>()); }; > + = requires { drop_view(std::declval<_Range>(), > std::declval<_Dp>()); }; > } // namespace __detail > > struct _Drop : __adaptor::_RangeAdaptor<_Drop> > { > - template > - requires __detail::__can_drop_view<_Range, _Tp> > + template range_difference_t<_Range>> > + requires __detail::__can_drop_view<_Range, _Dp> > constexpr auto > - operator() [[nodiscard]] (_Range&& __r, _Tp&& __n) const > + operator() [[nodiscard]] (_Range&& __r, type_identity_t<_Dp> __n) > const > { > - return drop_view(std::forward<_Range>(__r), > std::forward<_Tp>(__n)); > + using _Tp = remove_cvref_t<_Range>; > + if constexpr (__detail::__is_empty_view<_Tp>) > + return _Tp(); > + else if constexpr (random_access_range<_Tp> > + && sized_range<_Tp> > + && (std::__detail::__is_span<_Tp> > + || __detail::__is_basic_string_view<_Tp> > + || __detail::__is_iota_view<_Tp> > + || __detail::__is_subrange<_Tp>)) > + { > + __n = std::min<_Dp>(ranges::distance(__r), __n); > + auto __begin = ranges::begin(__r) + __n; > + auto __end = ranges::end(__r); > + if constexpr (std::__detail::__is_span<_Tp>) > + return span(__begin, __end); > + else if constexpr (__detail::__is_subrange<_Tp>) > + { > + if constexpr (_Tp::_S_store_size) > + { > + using ranges::__detail::__to_unsigned_like; > + auto __m = ranges::distance(__r) - __n; > + return _Tp(__begin, __end, __to_unsigned_like(__m)); > + } > + else > + return _Tp(__begin, __end); > + } > + else > + return _Tp(__begin, __end); > + } > + else > + return drop_view(std::forward<_Range>(__r), __n); > } > > using _RangeAdaptor<_Drop>::operator(); > @@ -3536,7 +3614,9 @@ namespace views::__adaptor > constexpr auto > operator() [[nodiscard]] (_Iter __i, iter_difference_t<_Iter> __n) > const > { > - if constexpr (random_access_iterator<_Iter>) > + if constexpr (contiguous_iterator<_Iter>) > + return span(std::__to_address(__i), __n); > + else if constexpr (random_access_iterator<_Iter>) > return subrange(__i, __i + __n); > else > return subrange(counted_iterator(std::move(__i), __n), > diff --git a/libstdc++-v3/include/std/span b/libstdc++-v3/include/std/span > index af0d24b29f2..ba75a497310 100644 > --- a/libstdc++-v3/include/std/span > +++ b/libstdc++-v3/include/std/span > @@ -58,16 +58,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > namespace __detail > { > template > - struct __is_std_span : false_type { }; > + constexpr bool __is_span = false; > > template > - struct __is_std_span> : true_type { }; > + constexpr bool __is_span> = true; > > template > - struct __is_std_array : false_type { }; > + constexpr bool __is_std_array = false; > > template > - struct __is_std_array> : true_type { }; > + constexpr bool __is_std_array> = true; > > template > class __extent_storage > @@ -203,8 +203,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > template > requires ranges::contiguous_range<_Range> && > ranges::sized_range<_Range> > && (ranges::borrowed_range<_Range> || is_const_v) > - && (!__detail::__is_std_span>::value) > - && (!__detail::__is_std_array>::value) > + && (!__detail::__is_span>) > + && (!__detail::__is_std_array>) > && (!is_array_v>) > && __is_compatible_ref>::value > constexpr explicit(extent != dynamic_extent) > diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/p1739.cc > b/libstdc++-v3/testsuite/std/ranges/adaptors/p1739.cc > new file mode 100644 > index 00000000000..243b8a03583 > --- /dev/null > +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/p1739.cc > @@ -0,0 +1,88 @@ > +// { dg-options "-std=gnu++20" } > +// { dg-do run { target c++20 } } > + > +#include > +#include > +#include > + > +// Verify P1739R4 (and LWG 3407) changes views::take/drop/counted. > + > +namespace ranges = std::ranges; > +namespace views = ranges::views; > + > +void > +test01() > +{ > + ranges::empty_view v; > + using ty = decltype(v); > + using ty = decltype(views::take(v, 42)); > + using ty = decltype(views::drop(v, 42)); > +} > + > +void > +test02() > +{ > + int x[] = {1,2,3,4,5}; > + std::span r(x, x+5); > + using ty = decltype(r); > + > + std::same_as auto v1 = views::take(r, 2); > + VERIFY( ranges::equal(v1, (int[]){1,2}) ); > + > + std::same_as auto v2 = views::drop(r, 3); > + VERIFY( ranges::equal(v2, (int[]){4,5}) ); > + > + std::same_as auto v3 = views::counted(r.begin(), 4); > + VERIFY( ranges::equal(v3, (int[]){1,2,3,4}) ); > +} > + > +void > +test03() > +{ > + using namespace std::literals; > + std::string_view r = "hello world"sv; > + using ty = decltype(r); > + > + std::same_as auto v1 = views::take(r, 5); > + VERIFY( ranges::equal(v1, "hello"sv) ); > + > + std::same_as auto v2 = views::drop(r, 6); > + VERIFY( ranges::equal(v2, "world"sv) ); > +} > + > +void > +test04() > +{ > + int x[] = {1,2,3,4,5}; > + ranges::subrange r(x, x+5); > + using ty = decltype(r); > + > + std::same_as auto v1 = views::take(r, 2); > + VERIFY( ranges::equal(v1, (int[]){1,2}) ); > + > + std::same_as auto v2 = views::drop(r, 1); > + VERIFY( ranges::equal(v2, (int[]){2,3,4,5}) ); > +} > + > +void > +test05() > +{ > + auto r1 = views::iota(1, 6); > + using ty1 = decltype(r1); > + > + std::same_as auto v1 = views::take(r1, 2); > + VERIFY( ranges::equal(v1, (int[]){1,2}) ); > + > + std::same_as auto v2 = views::drop(r1, 3); > + VERIFY( ranges::equal(v2, (int[]){4,5}) ); > +} > + > +int > +main() > +{ > + test01(); > + test02(); > + test03(); > + test04(); > + test05(); > +} > -- > 2.33.1.637.gf443b226ca > >