From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1888) id 3910B3858D35; Thu, 17 Aug 2023 16:40:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 3910B3858D35 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1692290413; bh=4hpCiFJZs0/VazqwStl78zG2X5E124jEz8d/n8hFx8E=; h=From:To:Subject:Date:From; b=xNggr9HfZrios3YkwbZtvdvlQRs1Br95a0OX3h7ZlnXRqCRvDeaPDpie3XJE/6sto ULKYTPbBFeFBz8lOSQMyAECQH7K5eAI7OERKkIbofHAVmUmLuNK7nwPKnuxY29JpY1 6/WlzyBPrX7yRbGjhx9n4/poXYSrvLAx2UzAlsKc= MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Patrick Palka To: gcc-cvs@gcc.gnu.org, libstdc++-cvs@gcc.gnu.org Subject: [gcc r14-3293] libstdc++: Convert _RangeAdaptorClosure into a CRTP base [PR108827] X-Act-Checkin: gcc X-Git-Author: Patrick Palka X-Git-Refname: refs/heads/master X-Git-Oldrev: bd7257f08c9b18bdd66a59bff2e02801a8aaf340 X-Git-Newrev: 4a6f3676e7dd9e73a822f4da634b037299f0d482 Message-Id: <20230817164013.3910B3858D35@sourceware.org> Date: Thu, 17 Aug 2023 16:40:13 +0000 (GMT) List-Id: https://gcc.gnu.org/g:4a6f3676e7dd9e73a822f4da634b037299f0d482 commit r14-3293-g4a6f3676e7dd9e73a822f4da634b037299f0d482 Author: Patrick Palka Date: Thu Aug 17 12:40:04 2023 -0400 libstdc++: Convert _RangeAdaptorClosure into a CRTP base [PR108827] Using the CRTP idiom for this base class avoids bloating the size of a pipeline when adding distinct empty range adaptor closure objects to it, as detailed in section 4.1 of P2387R3. But it means we can no longer define its operator| overloads as hidden friends, since it'd mean each instantiation of _RangeAdaptorClosure introduces its own distinct set of hidden friends. So e.g. for the outer | in x | (views::reverse | views::join) ADL would find 6 distinct hidden operator| friends: two from _RangeAdaptorClosure<_Reverse> two from _RangeAdaptorClosure<_Join> two from _RangeAdaptorClosure<_Pipe<_Reverse, _Join>> but we really only want to consider the last two. We avoid this issue by instead defining the operator| overloads at namespace scope alongside _RangeAdaptorClosure. This should be fine because the only types defined in this namespace are _RangeAdaptorClosure, _RangeAdaptor, _Pipe and _Partial, so we don't have to worry about unintentional ADL. Reviewed-by: Jonathan Wakely PR libstdc++/108827 libstdc++-v3/ChangeLog: * include/std/ranges (__adaptor::_RangeAdaptorClosure): Convert into a CRTP class template. Move hidden operator| friends into namespace scope and adjust their constraints. (__closure::__is_range_adaptor_closure_fn): Define. (__closure::__is_range_adaptor_closure): Define. (__adaptor::_Partial): Adjust use of _RangeAdaptorClosure. (__adaptor::_Pipe): Likewise. (views::_All): Likewise. (views::_Join): Likewise. (views::_Common): Likewise. (views::_Reverse): Likewise. (views::_Elements): Likewise. (views::_Adjacent): Likewise. (views::_AsRvalue): Likewise. (views::_Enumerate): Likewise. (views::_AsConst): Likewise. * testsuite/std/ranges/adaptors/all.cc: Reinstate assertion expecting that adding empty range adaptor closure objects to a pipeline doesn't increase the size of a pipeline. Diff: --- libstdc++-v3/include/std/ranges | 80 +++++++++++++---------- libstdc++-v3/testsuite/std/ranges/adaptors/all.cc | 7 -- 2 files changed, 45 insertions(+), 42 deletions(-) diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges index fb4a6a8b9d7d..ffa3e97ef944 100644 --- a/libstdc++-v3/include/std/ranges +++ b/libstdc++-v3/include/std/ranges @@ -892,25 +892,35 @@ namespace views::__adaptor // The derived class should define the optional static data member // _S_has_simple_call_op to true if the behavior of this adaptor is // independent of the constness/value category of the adaptor object. - struct _RangeAdaptorClosure - { - // range | adaptor is equivalent to adaptor(range). - template - requires derived_from, _RangeAdaptorClosure> - && __adaptor_invocable<_Self, _Range> - friend constexpr auto - operator|(_Range&& __r, _Self&& __self) - { return std::forward<_Self>(__self)(std::forward<_Range>(__r)); } - - // Compose the adaptors __lhs and __rhs into a pipeline, returning - // another range adaptor closure object. - template - requires derived_from<_Lhs, _RangeAdaptorClosure> - && derived_from<_Rhs, _RangeAdaptorClosure> - friend constexpr auto - operator|(_Lhs __lhs, _Rhs __rhs) - { return _Pipe<_Lhs, _Rhs>{std::move(__lhs), std::move(__rhs)}; } - }; + template + struct _RangeAdaptorClosure + { }; + + template + requires (!same_as<_Tp, _RangeAdaptorClosure<_Up>>) + void __is_range_adaptor_closure_fn + (const _Tp&, const _RangeAdaptorClosure<_Up>&); // not defined + + template + concept __is_range_adaptor_closure + = requires (_Tp __t) { __adaptor::__is_range_adaptor_closure_fn(__t, __t); }; + + // range | adaptor is equivalent to adaptor(range). + template + requires __is_range_adaptor_closure<_Self> + && __adaptor_invocable<_Self, _Range> + constexpr auto + operator|(_Range&& __r, _Self&& __self) + { return std::forward<_Self>(__self)(std::forward<_Range>(__r)); } + + // Compose the adaptors __lhs and __rhs into a pipeline, returning + // another range adaptor closure object. + template + requires __is_range_adaptor_closure<_Lhs> + && __is_range_adaptor_closure<_Rhs> + constexpr auto + operator|(_Lhs __lhs, _Rhs __rhs) + { return _Pipe<_Lhs, _Rhs>{std::move(__lhs), std::move(__rhs)}; } // The base class of every range adaptor non-closure. // @@ -952,7 +962,7 @@ namespace views::__adaptor // A range adaptor closure that represents partial application of // the range adaptor _Adaptor with arguments _Args. template - struct _Partial : _RangeAdaptorClosure + struct _Partial : _RangeAdaptorClosure<_Partial<_Adaptor, _Args...>> { tuple<_Args...> _M_args; @@ -993,7 +1003,7 @@ namespace views::__adaptor // A lightweight specialization of the above primary template for // the common case where _Adaptor accepts a single extra argument. template - struct _Partial<_Adaptor, _Arg> : _RangeAdaptorClosure + struct _Partial<_Adaptor, _Arg> : _RangeAdaptorClosure<_Partial<_Adaptor, _Arg>> { _Arg _M_arg; @@ -1026,7 +1036,7 @@ namespace views::__adaptor template requires __adaptor_has_simple_extra_args<_Adaptor, _Args...> && (is_trivially_copyable_v<_Args> && ...) - struct _Partial<_Adaptor, _Args...> : _RangeAdaptorClosure + struct _Partial<_Adaptor, _Args...> : _RangeAdaptorClosure<_Partial<_Adaptor, _Args...>> { tuple<_Args...> _M_args; @@ -1056,7 +1066,7 @@ namespace views::__adaptor template requires __adaptor_has_simple_extra_args<_Adaptor, _Arg> && is_trivially_copyable_v<_Arg> - struct _Partial<_Adaptor, _Arg> : _RangeAdaptorClosure + struct _Partial<_Adaptor, _Arg> : _RangeAdaptorClosure<_Partial<_Adaptor, _Arg>> { _Arg _M_arg; @@ -1081,7 +1091,7 @@ namespace views::__adaptor // A range adaptor closure that represents composition of the range // adaptor closures _Lhs and _Rhs. template - struct _Pipe : _RangeAdaptorClosure + struct _Pipe : _RangeAdaptorClosure<_Pipe<_Lhs, _Rhs>> { [[no_unique_address]] _Lhs _M_lhs; [[no_unique_address]] _Rhs _M_rhs; @@ -1117,7 +1127,7 @@ namespace views::__adaptor template requires __closure_has_simple_call_op<_Lhs> && __closure_has_simple_call_op<_Rhs> - struct _Pipe<_Lhs, _Rhs> : _RangeAdaptorClosure + struct _Pipe<_Lhs, _Rhs> : _RangeAdaptorClosure<_Pipe<_Lhs, _Rhs>> { [[no_unique_address]] _Lhs _M_lhs; [[no_unique_address]] _Rhs _M_rhs; @@ -1141,7 +1151,7 @@ namespace views::__adaptor template requires is_class_v<_Derived> && same_as<_Derived, remove_cv_t<_Derived>> class range_adaptor_closure - : public views::__adaptor::_RangeAdaptorClosure + : public views::__adaptor::_RangeAdaptorClosure<_Derived> { }; #endif @@ -1287,7 +1297,7 @@ namespace views::__adaptor concept __can_owning_view = requires { owning_view{std::declval<_Range>()}; }; } // namespace __detail - struct _All : __adaptor::_RangeAdaptorClosure + struct _All : __adaptor::_RangeAdaptorClosure<_All> { template static constexpr bool @@ -3071,7 +3081,7 @@ namespace views::__adaptor = requires { join_view>{std::declval<_Range>()}; }; } // namespace __detail - struct _Join : __adaptor::_RangeAdaptorClosure + struct _Join : __adaptor::_RangeAdaptorClosure<_Join> { template requires __detail::__can_join_view<_Range> @@ -3865,7 +3875,7 @@ namespace views::__adaptor = requires { common_view{std::declval<_Range>()}; }; } // namespace __detail - struct _Common : __adaptor::_RangeAdaptorClosure + struct _Common : __adaptor::_RangeAdaptorClosure<_Common> { template requires __detail::__already_common<_Range> @@ -3986,7 +3996,7 @@ namespace views::__adaptor = requires { reverse_view{std::declval<_Range>()}; }; } // namespace __detail - struct _Reverse : __adaptor::_RangeAdaptorClosure + struct _Reverse : __adaptor::_RangeAdaptorClosure<_Reverse> { template requires __detail::__is_reverse_view> @@ -4386,7 +4396,7 @@ namespace views::__adaptor } // namespace __detail template - struct _Elements : __adaptor::_RangeAdaptorClosure + struct _Elements : __adaptor::_RangeAdaptorClosure<_Elements<_Nm>> { template requires __detail::__can_elements_view<_Nm, _Range> @@ -5504,7 +5514,7 @@ namespace views::__adaptor } template - struct _Adjacent : __adaptor::_RangeAdaptorClosure + struct _Adjacent : __adaptor::_RangeAdaptorClosure<_Adjacent<_Nm>> { template requires (_Nm == 0) || __detail::__can_adjacent_view<_Nm, _Range> @@ -8629,7 +8639,7 @@ namespace views::__adaptor concept __can_as_rvalue_view = requires { as_rvalue_view(std::declval<_Tp>()); }; } - struct _AsRvalue : __adaptor::_RangeAdaptorClosure + struct _AsRvalue : __adaptor::_RangeAdaptorClosure<_AsRvalue> { template requires __detail::__can_as_rvalue_view<_Range> @@ -8938,7 +8948,7 @@ namespace views::__adaptor = requires { enumerate_view>(std::declval<_Tp>()); }; } - struct _Enumerate : __adaptor::_RangeAdaptorClosure + struct _Enumerate : __adaptor::_RangeAdaptorClosure<_Enumerate> { template requires __detail::__can_enumerate_view<_Range> @@ -9024,7 +9034,7 @@ namespace views::__adaptor concept __can_as_const_view = requires { as_const_view(std::declval<_Range>()); }; } - struct _AsConst : __adaptor::_RangeAdaptorClosure + struct _AsConst : __adaptor::_RangeAdaptorClosure<_AsConst> { template constexpr auto diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc index 57cb542174b1..6bbaf73a20a9 100644 --- a/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc @@ -105,12 +105,6 @@ static_assert(std::is_empty_v); -#if 0 -// Adding empty range adaptor closure objects to a pipeline used to not -// increase the size of the pipeline, but now that our range adaptor closure -// objects derive from a common empty base class, [[no_unique_address]] can no -// longer make two empty adjacent range adaptor closure objects occupy the same -// data member address. static_assert(sizeof(decltype(views::take(5) | views::drop(5))) == sizeof(decltype(views::take(5) | views::join @@ -119,7 +113,6 @@ static_assert(sizeof(decltype(views::take(5) | views::drop(5))) | views::keys | views::drop(5) | views::reverse))); -#endif template void