* [PATCH 1/2] libstdc++: Convert _RangeAdaptorClosure into a CRTP class [PR108827]
@ 2023-04-14 19:45 Patrick Palka
2023-04-14 19:45 ` [PATCH 2/2] libstdc++: Implement range_adaptor_closure from P2387R3 [PR108827] Patrick Palka
2023-04-17 3:24 ` [PATCH 1/2] libstdc++: Convert _RangeAdaptorClosure into a CRTP class [PR108827] Patrick Palka
0 siblings, 2 replies; 9+ messages in thread
From: Patrick Palka @ 2023-04-14 19:45 UTC (permalink / raw)
To: gcc-patches; +Cc: libstdc++, Patrick Palka
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 each instantiation of _RangeAdaptorClosure would then
introduce its own logically different hidden friends. So for example
during overload resolution for the outer pipe operator in
:x | (views::reverse | views::join)
we'd have to consider 6 different hidden operator| friends:
2 from _RangeAdaptorClosure<_Reverse>
2 from _RangeAdaptorClosure<_Join>
2 from _RangeAdaptorClosure<_Pipe<_Reverse, _Join>>
which is wasteful and can even cause hard errors in some cases. So we
instead define the operator| overloads at namespace scope in an isolated
namespace.
PR libstdc++/108827
libstdc++-v3/ChangeLog:
* include/std/ranges (__adaptor::_RangeAdaptorClosure): Move ...
(__adaptor::__closure::_RangeAdaptorClosure): ... here and turn
it into a CRTP class template. Move hidden operator| friends
into namespace scope and adjust their constraints. Add a
using-declaration for this at __adaptor::_RangeAdaptorClosure.
(__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: Reintroduce
static_assert expecting that adding empty range adaptor
closure objects to a pipeline doesn't increase the size of a
pipeline.
---
libstdc++-v3/include/std/ranges | 69 +++++++++++--------
.../testsuite/std/ranges/adaptors/all.cc | 7 --
2 files changed, 42 insertions(+), 34 deletions(-)
diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 283d757faa4..531ec6f68b3 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -872,30 +872,45 @@ namespace views::__adaptor
template<typename _Lhs, typename _Rhs>
struct _Pipe;
- // The base class of every range adaptor closure.
- //
- // 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
+ namespace __closure
{
+ // The base class of every range adaptor closure.
+ //
+ // 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.
+ template<typename _Derived>
+ struct _RangeAdaptorClosure
+ { };
+
+ template<typename _Tp, typename _Up>
+ requires (!same_as<_Tp, _RangeAdaptorClosure<_Up>>)
+ void __is_range_adaptor_closure_fn
+ (const _Tp&, const _RangeAdaptorClosure<_Up>&); // not defined
+
+ template<typename _Tp>
+ concept __is_range_adaptor_closure
+ = requires (_Tp __t) { __closure::__is_range_adaptor_closure_fn(__t, __t); };
+
// range | adaptor is equivalent to adaptor(range).
template<typename _Self, typename _Range>
- requires derived_from<remove_cvref_t<_Self>, _RangeAdaptorClosure>
+ requires __is_range_adaptor_closure<_Self>
&& __adaptor_invocable<_Self, _Range>
- friend constexpr auto
+ 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<typename _Lhs, typename _Rhs>
- requires derived_from<_Lhs, _RangeAdaptorClosure>
- && derived_from<_Rhs, _RangeAdaptorClosure>
- friend constexpr auto
+ 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)}; }
- };
+ } // namespace __closure
+
+ using __closure::_RangeAdaptorClosure;
// The base class of every range adaptor non-closure.
//
@@ -937,7 +952,7 @@ namespace views::__adaptor
// A range adaptor closure that represents partial application of
// the range adaptor _Adaptor with arguments _Args.
template<typename _Adaptor, typename... _Args>
- struct _Partial : _RangeAdaptorClosure
+ struct _Partial : _RangeAdaptorClosure<_Partial<_Adaptor, _Args...>>
{
tuple<_Args...> _M_args;
@@ -978,7 +993,7 @@ namespace views::__adaptor
// A lightweight specialization of the above primary template for
// the common case where _Adaptor accepts a single extra argument.
template<typename _Adaptor, typename _Arg>
- struct _Partial<_Adaptor, _Arg> : _RangeAdaptorClosure
+ struct _Partial<_Adaptor, _Arg> : _RangeAdaptorClosure<_Partial<_Adaptor, _Arg>>
{
_Arg _M_arg;
@@ -1011,7 +1026,7 @@ namespace views::__adaptor
template<typename _Adaptor, typename... _Args>
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;
@@ -1041,7 +1056,7 @@ namespace views::__adaptor
template<typename _Adaptor, typename _Arg>
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;
@@ -1066,7 +1081,7 @@ namespace views::__adaptor
// A range adaptor closure that represents composition of the range
// adaptor closures _Lhs and _Rhs.
template<typename _Lhs, typename _Rhs>
- struct _Pipe : _RangeAdaptorClosure
+ struct _Pipe : _RangeAdaptorClosure<_Pipe<_Lhs, _Rhs>>
{
[[no_unique_address]] _Lhs _M_lhs;
[[no_unique_address]] _Rhs _M_rhs;
@@ -1102,7 +1117,7 @@ namespace views::__adaptor
template<typename _Lhs, typename _Rhs>
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;
@@ -1264,7 +1279,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<typename _Range>
static constexpr bool
@@ -3048,7 +3063,7 @@ namespace views::__adaptor
= requires { join_view<all_t<_Range>>{std::declval<_Range>()}; };
} // namespace __detail
- struct _Join : __adaptor::_RangeAdaptorClosure
+ struct _Join : __adaptor::_RangeAdaptorClosure<_Join>
{
template<viewable_range _Range>
requires __detail::__can_join_view<_Range>
@@ -3842,7 +3857,7 @@ namespace views::__adaptor
= requires { common_view{std::declval<_Range>()}; };
} // namespace __detail
- struct _Common : __adaptor::_RangeAdaptorClosure
+ struct _Common : __adaptor::_RangeAdaptorClosure<_Common>
{
template<viewable_range _Range>
requires __detail::__already_common<_Range>
@@ -3963,7 +3978,7 @@ namespace views::__adaptor
= requires { reverse_view{std::declval<_Range>()}; };
} // namespace __detail
- struct _Reverse : __adaptor::_RangeAdaptorClosure
+ struct _Reverse : __adaptor::_RangeAdaptorClosure<_Reverse>
{
template<viewable_range _Range>
requires __detail::__is_reverse_view<remove_cvref_t<_Range>>
@@ -4363,7 +4378,7 @@ namespace views::__adaptor
} // namespace __detail
template<size_t _Nm>
- struct _Elements : __adaptor::_RangeAdaptorClosure
+ struct _Elements : __adaptor::_RangeAdaptorClosure<_Elements<_Nm>>
{
template<viewable_range _Range>
requires __detail::__can_elements_view<_Nm, _Range>
@@ -5484,7 +5499,7 @@ namespace views::__adaptor
}
template<size_t _Nm>
- struct _Adjacent : __adaptor::_RangeAdaptorClosure
+ struct _Adjacent : __adaptor::_RangeAdaptorClosure<_Adjacent<_Nm>>
{
template<viewable_range _Range>
requires (_Nm == 0) || __detail::__can_adjacent_view<_Nm, _Range>
@@ -8609,7 +8624,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<viewable_range _Range>
requires __detail::__can_as_rvalue_view<_Range>
@@ -8918,7 +8933,7 @@ namespace views::__adaptor
= requires { enumerate_view<all_t<_Tp>>(std::declval<_Tp>()); };
}
- struct _Enumerate : __adaptor::_RangeAdaptorClosure
+ struct _Enumerate : __adaptor::_RangeAdaptorClosure<_Enumerate>
{
template<viewable_range _Range>
requires __detail::__can_enumerate_view<_Range>
@@ -9004,7 +9019,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<viewable_range _Range>
constexpr auto
diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc
index 57cb542174b..6bbaf73a20a 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<decltype(views::common
| views::common
| views::keys
| views::reverse)>);
-#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<auto all = views::all>
void
--
2.40.0.335.g9857273be0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] libstdc++: Implement range_adaptor_closure from P2387R3 [PR108827]
2023-04-14 19:45 [PATCH 1/2] libstdc++: Convert _RangeAdaptorClosure into a CRTP class [PR108827] Patrick Palka
@ 2023-04-14 19:45 ` Patrick Palka
2023-04-17 3:27 ` Patrick Palka
2023-04-17 3:24 ` [PATCH 1/2] libstdc++: Convert _RangeAdaptorClosure into a CRTP class [PR108827] Patrick Palka
1 sibling, 1 reply; 9+ messages in thread
From: Patrick Palka @ 2023-04-14 19:45 UTC (permalink / raw)
To: gcc-patches; +Cc: libstdc++, Patrick Palka
Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
PR libstdc++/108827
libstdc++-v3/ChangeLog:
* include/bits/ranges_cmp.h (__cpp_lib_ranges): Bump value
for C++23.
* include/std/ranges (range_adaptor_closure): Define for C++23.
* include/std/version (__cpp_lib_ranges): Bump value for
C++23.
* testsuite/std/ranges/version_c++23.cc: Bump expected value
of __cpp_lib_ranges.
* testsuite/std/ranges/range_adaptor_closure.cc: New test.
---
libstdc++-v3/include/bits/ranges_cmp.h | 4 ++
libstdc++-v3/include/std/ranges | 8 ++++
libstdc++-v3/include/std/version | 3 ++
.../std/ranges/range_adaptor_closure.cc | 42 +++++++++++++++++++
.../testsuite/std/ranges/version_c++23.cc | 2 +-
5 files changed, 58 insertions(+), 1 deletion(-)
create mode 100644 libstdc++-v3/testsuite/std/ranges/range_adaptor_closure.cc
diff --git a/libstdc++-v3/include/bits/ranges_cmp.h b/libstdc++-v3/include/bits/ranges_cmp.h
index 85c1a77ccf7..6710d829a37 100644
--- a/libstdc++-v3/include/bits/ranges_cmp.h
+++ b/libstdc++-v3/include/bits/ranges_cmp.h
@@ -57,7 +57,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#ifdef __cpp_lib_concepts
// Define this here, included by all the headers that need to define it.
+#if __cplusplus > 202002L
+#define __cpp_lib_ranges 202202L
+#else
#define __cpp_lib_ranges 202110L
+#endif
namespace ranges
{
diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 531ec6f68b3..bf3c246f95d 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -1137,6 +1137,14 @@ namespace views::__adaptor
};
} // namespace views::__adaptor
+#if __cplusplus > 20202L
+ template<typename _Derived>
+ requires is_class_v<_Derived> && same_as<_Derived, remove_cv_t<_Derived>>
+ class range_adaptor_closure
+ : public views::__adaptor::_RangeAdaptorClosure<_Derived>
+ { };
+#endif
+
template<range _Range> requires is_object_v<_Range>
class ref_view : public view_interface<ref_view<_Range>>
{
diff --git a/libstdc++-v3/include/std/version b/libstdc++-v3/include/std/version
index 9f31f25f1e9..ad928a225ef 100644
--- a/libstdc++-v3/include/std/version
+++ b/libstdc++-v3/include/std/version
@@ -265,8 +265,10 @@
#define __cpp_lib_interpolate 201902L
#if __cpp_lib_concepts
# define __cpp_lib_move_iterator_concept 202207L
+#if __cplusplus <= 202002L // N.B. updated value in C++23
# define __cpp_lib_ranges 202110L
#endif
+#endif
#define __cpp_lib_shift 201806L
#if _GLIBCXX_HOSTED
@@ -330,6 +332,7 @@
#define __cpp_lib_reference_from_temporary 202202L
#define __cpp_lib_to_underlying 202102L
#define __cpp_lib_unreachable 202202L
+#define __cpp_lib_ranges 202202L
#define __cpp_lib_ranges_zip 202110L
#define __cpp_lib_ranges_chunk 202202L
#define __cpp_lib_ranges_slide 202202L
diff --git a/libstdc++-v3/testsuite/std/ranges/range_adaptor_closure.cc b/libstdc++-v3/testsuite/std/ranges/range_adaptor_closure.cc
new file mode 100644
index 00000000000..c642712ed9e
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/ranges/range_adaptor_closure.cc
@@ -0,0 +1,42 @@
+// { dg-options "-std=gnu++23" }
+// { dg-do run { target c++23 } }
+
+#include <ranges>
+#include <algorithm>
+#include <testsuite_hooks.h>
+
+namespace ranges = std::ranges;
+namespace views = std::views;
+
+struct _Negate : ranges::range_adaptor_closure<_Negate>
+{
+ template<ranges::viewable_range _Range>
+ constexpr auto
+ operator()(_Range&& __r) const
+ requires requires { views::transform(std::declval<_Range>(), std::negate{}); }
+ { return views::transform(std::forward<_Range>(__r), std::negate{}); }
+};
+
+constexpr _Negate negate;
+
+constexpr bool
+test01()
+{
+ int x[] = {1, 2, 3};
+ VERIFY( ranges::equal(x | negate, (int[]){-1, -2, -3}) );
+ VERIFY( ranges::equal(x | negate | negate, x) );
+ VERIFY( ranges::equal(x | (negate | negate), x) );
+ VERIFY( ranges::equal(x | views::reverse | negate, x | negate | views::reverse) );
+ VERIFY( ranges::equal(x | (views::reverse | negate), x | (negate | views::reverse)) );
+ static_assert( sizeof(negate | views::reverse | views::join) == 1 );
+ static_assert( sizeof(views::reverse | negate | views::join) == 1 );
+ static_assert( sizeof(views::reverse | views::join | negate) == 1 );
+
+ return true;
+}
+
+int
+main()
+{
+ static_assert(test01());
+}
diff --git a/libstdc++-v3/testsuite/std/ranges/version_c++23.cc b/libstdc++-v3/testsuite/std/ranges/version_c++23.cc
index e2c14edc8ef..90cfceb626c 100644
--- a/libstdc++-v3/testsuite/std/ranges/version_c++23.cc
+++ b/libstdc++-v3/testsuite/std/ranges/version_c++23.cc
@@ -4,7 +4,7 @@
#include <version>
#if __STDC_HOSTED__
-# if __cpp_lib_ranges != 202110L
+# if __cpp_lib_ranges != 202202L
# error "Feature-test macro __cpp_lib_ranges has wrong value in <version>"
# endif
#endif
--
2.40.0.335.g9857273be0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] libstdc++: Convert _RangeAdaptorClosure into a CRTP class [PR108827]
2023-04-14 19:45 [PATCH 1/2] libstdc++: Convert _RangeAdaptorClosure into a CRTP class [PR108827] Patrick Palka
2023-04-14 19:45 ` [PATCH 2/2] libstdc++: Implement range_adaptor_closure from P2387R3 [PR108827] Patrick Palka
@ 2023-04-17 3:24 ` Patrick Palka
2023-08-16 16:05 ` Patrick Palka
1 sibling, 1 reply; 9+ messages in thread
From: Patrick Palka @ 2023-04-17 3:24 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches, libstdc++
On Fri, 14 Apr 2023, Patrick Palka wrote:
> 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 each instantiation of _RangeAdaptorClosure would then
> introduce its own logically different hidden friends. So for example
> during overload resolution for the outer pipe operator in
>
> :x | (views::reverse | views::join)
>
> we'd have to consider 6 different hidden operator| friends:
>
> 2 from _RangeAdaptorClosure<_Reverse>
> 2 from _RangeAdaptorClosure<_Join>
> 2 from _RangeAdaptorClosure<_Pipe<_Reverse, _Join>>
>
> which is wasteful and can even cause hard errors in some cases. So we
> instead define the operator| overloads at namespace scope in an isolated
> namespace.
On second thought, since this doesn't fix a bug or add new functionality
it seems more like GCC 14 material. The size reduction is nice but it's
probably not a big deal in practice since adaptor pipelines are usually
very transient objects that don't get passed around as function
arguments etc.
But perhaps the second patch implementing range_adaptor_closure would be
desirable for GCC 13? I'll post an updated standalone version of that
patch for separate consideration.
>
> PR libstdc++/108827
>
> libstdc++-v3/ChangeLog:
>
> * include/std/ranges (__adaptor::_RangeAdaptorClosure): Move ...
> (__adaptor::__closure::_RangeAdaptorClosure): ... here and turn
> it into a CRTP class template. Move hidden operator| friends
> into namespace scope and adjust their constraints. Add a
> using-declaration for this at __adaptor::_RangeAdaptorClosure.
> (__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: Reintroduce
> static_assert expecting that adding empty range adaptor
> closure objects to a pipeline doesn't increase the size of a
> pipeline.
> ---
> libstdc++-v3/include/std/ranges | 69 +++++++++++--------
> .../testsuite/std/ranges/adaptors/all.cc | 7 --
> 2 files changed, 42 insertions(+), 34 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
> index 283d757faa4..531ec6f68b3 100644
> --- a/libstdc++-v3/include/std/ranges
> +++ b/libstdc++-v3/include/std/ranges
> @@ -872,30 +872,45 @@ namespace views::__adaptor
> template<typename _Lhs, typename _Rhs>
> struct _Pipe;
>
> - // The base class of every range adaptor closure.
> - //
> - // 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
> + namespace __closure
> {
> + // The base class of every range adaptor closure.
> + //
> + // 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.
> + template<typename _Derived>
> + struct _RangeAdaptorClosure
> + { };
> +
> + template<typename _Tp, typename _Up>
> + requires (!same_as<_Tp, _RangeAdaptorClosure<_Up>>)
> + void __is_range_adaptor_closure_fn
> + (const _Tp&, const _RangeAdaptorClosure<_Up>&); // not defined
> +
> + template<typename _Tp>
> + concept __is_range_adaptor_closure
> + = requires (_Tp __t) { __closure::__is_range_adaptor_closure_fn(__t, __t); };
> +
> // range | adaptor is equivalent to adaptor(range).
> template<typename _Self, typename _Range>
> - requires derived_from<remove_cvref_t<_Self>, _RangeAdaptorClosure>
> + requires __is_range_adaptor_closure<_Self>
> && __adaptor_invocable<_Self, _Range>
> - friend constexpr auto
> + 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<typename _Lhs, typename _Rhs>
> - requires derived_from<_Lhs, _RangeAdaptorClosure>
> - && derived_from<_Rhs, _RangeAdaptorClosure>
> - friend constexpr auto
> + 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)}; }
> - };
> + } // namespace __closure
> +
> + using __closure::_RangeAdaptorClosure;
>
> // The base class of every range adaptor non-closure.
> //
> @@ -937,7 +952,7 @@ namespace views::__adaptor
> // A range adaptor closure that represents partial application of
> // the range adaptor _Adaptor with arguments _Args.
> template<typename _Adaptor, typename... _Args>
> - struct _Partial : _RangeAdaptorClosure
> + struct _Partial : _RangeAdaptorClosure<_Partial<_Adaptor, _Args...>>
> {
> tuple<_Args...> _M_args;
>
> @@ -978,7 +993,7 @@ namespace views::__adaptor
> // A lightweight specialization of the above primary template for
> // the common case where _Adaptor accepts a single extra argument.
> template<typename _Adaptor, typename _Arg>
> - struct _Partial<_Adaptor, _Arg> : _RangeAdaptorClosure
> + struct _Partial<_Adaptor, _Arg> : _RangeAdaptorClosure<_Partial<_Adaptor, _Arg>>
> {
> _Arg _M_arg;
>
> @@ -1011,7 +1026,7 @@ namespace views::__adaptor
> template<typename _Adaptor, typename... _Args>
> 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;
>
> @@ -1041,7 +1056,7 @@ namespace views::__adaptor
> template<typename _Adaptor, typename _Arg>
> 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;
>
> @@ -1066,7 +1081,7 @@ namespace views::__adaptor
> // A range adaptor closure that represents composition of the range
> // adaptor closures _Lhs and _Rhs.
> template<typename _Lhs, typename _Rhs>
> - struct _Pipe : _RangeAdaptorClosure
> + struct _Pipe : _RangeAdaptorClosure<_Pipe<_Lhs, _Rhs>>
> {
> [[no_unique_address]] _Lhs _M_lhs;
> [[no_unique_address]] _Rhs _M_rhs;
> @@ -1102,7 +1117,7 @@ namespace views::__adaptor
> template<typename _Lhs, typename _Rhs>
> 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;
> @@ -1264,7 +1279,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<typename _Range>
> static constexpr bool
> @@ -3048,7 +3063,7 @@ namespace views::__adaptor
> = requires { join_view<all_t<_Range>>{std::declval<_Range>()}; };
> } // namespace __detail
>
> - struct _Join : __adaptor::_RangeAdaptorClosure
> + struct _Join : __adaptor::_RangeAdaptorClosure<_Join>
> {
> template<viewable_range _Range>
> requires __detail::__can_join_view<_Range>
> @@ -3842,7 +3857,7 @@ namespace views::__adaptor
> = requires { common_view{std::declval<_Range>()}; };
> } // namespace __detail
>
> - struct _Common : __adaptor::_RangeAdaptorClosure
> + struct _Common : __adaptor::_RangeAdaptorClosure<_Common>
> {
> template<viewable_range _Range>
> requires __detail::__already_common<_Range>
> @@ -3963,7 +3978,7 @@ namespace views::__adaptor
> = requires { reverse_view{std::declval<_Range>()}; };
> } // namespace __detail
>
> - struct _Reverse : __adaptor::_RangeAdaptorClosure
> + struct _Reverse : __adaptor::_RangeAdaptorClosure<_Reverse>
> {
> template<viewable_range _Range>
> requires __detail::__is_reverse_view<remove_cvref_t<_Range>>
> @@ -4363,7 +4378,7 @@ namespace views::__adaptor
> } // namespace __detail
>
> template<size_t _Nm>
> - struct _Elements : __adaptor::_RangeAdaptorClosure
> + struct _Elements : __adaptor::_RangeAdaptorClosure<_Elements<_Nm>>
> {
> template<viewable_range _Range>
> requires __detail::__can_elements_view<_Nm, _Range>
> @@ -5484,7 +5499,7 @@ namespace views::__adaptor
> }
>
> template<size_t _Nm>
> - struct _Adjacent : __adaptor::_RangeAdaptorClosure
> + struct _Adjacent : __adaptor::_RangeAdaptorClosure<_Adjacent<_Nm>>
> {
> template<viewable_range _Range>
> requires (_Nm == 0) || __detail::__can_adjacent_view<_Nm, _Range>
> @@ -8609,7 +8624,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<viewable_range _Range>
> requires __detail::__can_as_rvalue_view<_Range>
> @@ -8918,7 +8933,7 @@ namespace views::__adaptor
> = requires { enumerate_view<all_t<_Tp>>(std::declval<_Tp>()); };
> }
>
> - struct _Enumerate : __adaptor::_RangeAdaptorClosure
> + struct _Enumerate : __adaptor::_RangeAdaptorClosure<_Enumerate>
> {
> template<viewable_range _Range>
> requires __detail::__can_enumerate_view<_Range>
> @@ -9004,7 +9019,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<viewable_range _Range>
> constexpr auto
> diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc
> index 57cb542174b..6bbaf73a20a 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<decltype(views::common
> | views::common
> | views::keys
> | views::reverse)>);
> -#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<auto all = views::all>
> void
> --
> 2.40.0.335.g9857273be0
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] libstdc++: Implement range_adaptor_closure from P2387R3 [PR108827]
2023-04-14 19:45 ` [PATCH 2/2] libstdc++: Implement range_adaptor_closure from P2387R3 [PR108827] Patrick Palka
@ 2023-04-17 3:27 ` Patrick Palka
2023-04-17 7:26 ` Jonathan Wakely
0 siblings, 1 reply; 9+ messages in thread
From: Patrick Palka @ 2023-04-17 3:27 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches, libstdc++
On Fri, 14 Apr 2023, Patrick Palka wrote:
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
>
> PR libstdc++/108827
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/ranges_cmp.h (__cpp_lib_ranges): Bump value
> for C++23.
> * include/std/ranges (range_adaptor_closure): Define for C++23.
> * include/std/version (__cpp_lib_ranges): Bump value for
> C++23.
> * testsuite/std/ranges/version_c++23.cc: Bump expected value
> of __cpp_lib_ranges.
> * testsuite/std/ranges/range_adaptor_closure.cc: New test.
Here's a standalone version of this patch (not dependent on the first
patch in the series) it terms of the current non-CRTP representation
of _RangeAdaptorClosure:
-- >8 --
Subject: [PATCH] libstdc++: Implement range_adaptor_closure from P2387R3
[PR108827]
Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
PR libstdc++/108827
libstdc++-v3/ChangeLog:
* include/bits/ranges_cmp.h (__cpp_lib_ranges): Bump value
for C++23.
* include/std/ranges (range_adaptor_closure): Define for C++23.
* include/std/version (__cpp_lib_ranges): Bump value for
C++23.
* testsuite/std/ranges/version_c++23.cc: Bump expected value
of __cpp_lib_ranges.
* testsuite/std/ranges/range_adaptor_closure.cc: New test.
---
libstdc++-v3/include/bits/ranges_cmp.h | 4 ++
libstdc++-v3/include/std/ranges | 8 ++++
libstdc++-v3/include/std/version | 3 ++
.../std/ranges/range_adaptor_closure.cc | 46 +++++++++++++++++++
.../testsuite/std/ranges/version_c++23.cc | 2 +-
5 files changed, 62 insertions(+), 1 deletion(-)
create mode 100644 libstdc++-v3/testsuite/std/ranges/range_adaptor_closure.cc
diff --git a/libstdc++-v3/include/bits/ranges_cmp.h b/libstdc++-v3/include/bits/ranges_cmp.h
index 85c1a77ccf7..6710d829a37 100644
--- a/libstdc++-v3/include/bits/ranges_cmp.h
+++ b/libstdc++-v3/include/bits/ranges_cmp.h
@@ -57,7 +57,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#ifdef __cpp_lib_concepts
// Define this here, included by all the headers that need to define it.
+#if __cplusplus > 202002L
+#define __cpp_lib_ranges 202202L
+#else
#define __cpp_lib_ranges 202110L
+#endif
namespace ranges
{
diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 1714f3fb16c..c4d4d85bf90 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -1122,6 +1122,14 @@ namespace views::__adaptor
};
} // namespace views::__adaptor
+#if __cplusplus > 202002L
+ template<typename _Derived>
+ requires is_class_v<_Derived> && same_as<_Derived, remove_cv_t<_Derived>>
+ class range_adaptor_closure
+ : public views::__adaptor::_RangeAdaptorClosure
+ { };
+#endif
+
template<range _Range> requires is_object_v<_Range>
class ref_view : public view_interface<ref_view<_Range>>
{
diff --git a/libstdc++-v3/include/std/version b/libstdc++-v3/include/std/version
index 027e5711ec5..02ead8f1443 100644
--- a/libstdc++-v3/include/std/version
+++ b/libstdc++-v3/include/std/version
@@ -265,8 +265,10 @@
#define __cpp_lib_interpolate 201902L
#if __cpp_lib_concepts
# define __cpp_lib_move_iterator_concept 202207L
+#if __cplusplus <= 202002L // N.B. updated value in C++23
# define __cpp_lib_ranges 202110L
#endif
+#endif
#define __cpp_lib_shift 201806L
#if _GLIBCXX_HOSTED
@@ -330,6 +332,7 @@
#define __cpp_lib_reference_from_temporary 202202L
#define __cpp_lib_to_underlying 202102L
#define __cpp_lib_unreachable 202202L
+#define __cpp_lib_ranges 202202L
#define __cpp_lib_ranges_zip 202110L
#define __cpp_lib_ranges_chunk 202202L
#define __cpp_lib_ranges_slide 202202L
diff --git a/libstdc++-v3/testsuite/std/ranges/range_adaptor_closure.cc b/libstdc++-v3/testsuite/std/ranges/range_adaptor_closure.cc
new file mode 100644
index 00000000000..6221f071331
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/ranges/range_adaptor_closure.cc
@@ -0,0 +1,46 @@
+// { dg-options "-std=gnu++23" }
+// { dg-do run { target c++23 } }
+
+#include <ranges>
+#include <algorithm>
+#include <testsuite_hooks.h>
+
+namespace ranges = std::ranges;
+namespace views = std::views;
+
+struct _Negate : ranges::range_adaptor_closure<_Negate>
+{
+ template<ranges::viewable_range _Range>
+ constexpr auto
+ operator()(_Range&& __r) const
+ requires requires { views::transform(std::declval<_Range>(), std::negate{}); }
+ { return views::transform(std::forward<_Range>(__r), std::negate{}); }
+};
+
+constexpr _Negate negate;
+
+constexpr bool
+test01()
+{
+ int x[] = {1, 2, 3};
+ VERIFY( ranges::equal(x | negate, (int[]){-1, -2, -3}) );
+ VERIFY( ranges::equal(x | negate | negate, x) );
+ VERIFY( ranges::equal(x | (negate | negate), x) );
+ VERIFY( ranges::equal(x | views::reverse | negate, x | negate | views::reverse) );
+ VERIFY( ranges::equal(x | (views::reverse | negate), x | (negate | views::reverse)) );
+#if 0
+ // These asserts currently fail for the same reason as the disabled asserts
+ // in std/ranges/adaptors/all.cc.
+ static_assert( sizeof(negate | views::reverse | views::join) == 1 );
+ static_assert( sizeof(views::reverse | negate | views::join) == 1 );
+ static_assert( sizeof(views::reverse | views::join | negate) == 1 );
+#endif
+
+ return true;
+}
+
+int
+main()
+{
+ static_assert(test01());
+}
diff --git a/libstdc++-v3/testsuite/std/ranges/version_c++23.cc b/libstdc++-v3/testsuite/std/ranges/version_c++23.cc
index 04609bb602c..e8342fa986a 100644
--- a/libstdc++-v3/testsuite/std/ranges/version_c++23.cc
+++ b/libstdc++-v3/testsuite/std/ranges/version_c++23.cc
@@ -4,7 +4,7 @@
#include <version>
#if __STDC_HOSTED__
-# if __cpp_lib_ranges != 202110L
+# if __cpp_lib_ranges != 202202L
# error "Feature-test macro __cpp_lib_ranges has wrong value in <version>"
# endif
#endif
--
2.40.0.335.g9857273be0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] libstdc++: Implement range_adaptor_closure from P2387R3 [PR108827]
2023-04-17 3:27 ` Patrick Palka
@ 2023-04-17 7:26 ` Jonathan Wakely
2023-04-18 8:57 ` Jonathan Wakely
0 siblings, 1 reply; 9+ messages in thread
From: Jonathan Wakely @ 2023-04-17 7:26 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches, libstdc++
[-- Attachment #1: Type: text/plain, Size: 6409 bytes --]
On Mon, 17 Apr 2023, 05:28 Patrick Palka via Libstdc++, <
libstdc++@gcc.gnu.org> wrote:
> On Fri, 14 Apr 2023, Patrick Palka wrote:
>
> > Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
> >
> > PR libstdc++/108827
> >
> > libstdc++-v3/ChangeLog:
> >
> > * include/bits/ranges_cmp.h (__cpp_lib_ranges): Bump value
> > for C++23.
> > * include/std/ranges (range_adaptor_closure): Define for C++23.
> > * include/std/version (__cpp_lib_ranges): Bump value for
> > C++23.
> > * testsuite/std/ranges/version_c++23.cc: Bump expected value
> > of __cpp_lib_ranges.
> > * testsuite/std/ranges/range_adaptor_closure.cc: New test.
>
> Here's a standalone version of this patch (not dependent on the first
> patch in the series) it terms of the current non-CRTP representation
> of _RangeAdaptorClosure:
>
OK, thanks for separating this from the other change.
> -- >8 --
>
> Subject: [PATCH] libstdc++: Implement range_adaptor_closure from P2387R3
> [PR108827]
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
>
> PR libstdc++/108827
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/ranges_cmp.h (__cpp_lib_ranges): Bump value
> for C++23.
> * include/std/ranges (range_adaptor_closure): Define for C++23.
> * include/std/version (__cpp_lib_ranges): Bump value for
> C++23.
> * testsuite/std/ranges/version_c++23.cc: Bump expected value
> of __cpp_lib_ranges.
> * testsuite/std/ranges/range_adaptor_closure.cc: New test.
> ---
> libstdc++-v3/include/bits/ranges_cmp.h | 4 ++
> libstdc++-v3/include/std/ranges | 8 ++++
> libstdc++-v3/include/std/version | 3 ++
> .../std/ranges/range_adaptor_closure.cc | 46 +++++++++++++++++++
> .../testsuite/std/ranges/version_c++23.cc | 2 +-
> 5 files changed, 62 insertions(+), 1 deletion(-)
> create mode 100644
> libstdc++-v3/testsuite/std/ranges/range_adaptor_closure.cc
>
> diff --git a/libstdc++-v3/include/bits/ranges_cmp.h
> b/libstdc++-v3/include/bits/ranges_cmp.h
> index 85c1a77ccf7..6710d829a37 100644
> --- a/libstdc++-v3/include/bits/ranges_cmp.h
> +++ b/libstdc++-v3/include/bits/ranges_cmp.h
> @@ -57,7 +57,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
> #ifdef __cpp_lib_concepts
> // Define this here, included by all the headers that need to define it.
> +#if __cplusplus > 202002L
> +#define __cpp_lib_ranges 202202L
> +#else
> #define __cpp_lib_ranges 202110L
> +#endif
>
> namespace ranges
> {
> diff --git a/libstdc++-v3/include/std/ranges
> b/libstdc++-v3/include/std/ranges
> index 1714f3fb16c..c4d4d85bf90 100644
> --- a/libstdc++-v3/include/std/ranges
> +++ b/libstdc++-v3/include/std/ranges
> @@ -1122,6 +1122,14 @@ namespace views::__adaptor
> };
> } // namespace views::__adaptor
>
> +#if __cplusplus > 202002L
> + template<typename _Derived>
> + requires is_class_v<_Derived> && same_as<_Derived,
> remove_cv_t<_Derived>>
> + class range_adaptor_closure
> + : public views::__adaptor::_RangeAdaptorClosure
> + { };
> +#endif
> +
> template<range _Range> requires is_object_v<_Range>
> class ref_view : public view_interface<ref_view<_Range>>
> {
> diff --git a/libstdc++-v3/include/std/version
> b/libstdc++-v3/include/std/version
> index 027e5711ec5..02ead8f1443 100644
> --- a/libstdc++-v3/include/std/version
> +++ b/libstdc++-v3/include/std/version
> @@ -265,8 +265,10 @@
> #define __cpp_lib_interpolate 201902L
> #if __cpp_lib_concepts
> # define __cpp_lib_move_iterator_concept 202207L
> +#if __cplusplus <= 202002L // N.B. updated value in C++23
> # define __cpp_lib_ranges 202110L
> #endif
> +#endif
> #define __cpp_lib_shift 201806L
>
> #if _GLIBCXX_HOSTED
> @@ -330,6 +332,7 @@
> #define __cpp_lib_reference_from_temporary 202202L
> #define __cpp_lib_to_underlying 202102L
> #define __cpp_lib_unreachable 202202L
> +#define __cpp_lib_ranges 202202L
> #define __cpp_lib_ranges_zip 202110L
> #define __cpp_lib_ranges_chunk 202202L
> #define __cpp_lib_ranges_slide 202202L
> diff --git a/libstdc++-v3/testsuite/std/ranges/range_adaptor_closure.cc
> b/libstdc++-v3/testsuite/std/ranges/range_adaptor_closure.cc
> new file mode 100644
> index 00000000000..6221f071331
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/std/ranges/range_adaptor_closure.cc
> @@ -0,0 +1,46 @@
> +// { dg-options "-std=gnu++23" }
> +// { dg-do run { target c++23 } }
> +
> +#include <ranges>
> +#include <algorithm>
> +#include <testsuite_hooks.h>
> +
> +namespace ranges = std::ranges;
> +namespace views = std::views;
> +
> +struct _Negate : ranges::range_adaptor_closure<_Negate>
> +{
> + template<ranges::viewable_range _Range>
> + constexpr auto
> + operator()(_Range&& __r) const
> + requires requires { views::transform(std::declval<_Range>(),
> std::negate{}); }
> + { return views::transform(std::forward<_Range>(__r), std::negate{}); }
> +};
> +
> +constexpr _Negate negate;
> +
> +constexpr bool
> +test01()
> +{
> + int x[] = {1, 2, 3};
> + VERIFY( ranges::equal(x | negate, (int[]){-1, -2, -3}) );
> + VERIFY( ranges::equal(x | negate | negate, x) );
> + VERIFY( ranges::equal(x | (negate | negate), x) );
> + VERIFY( ranges::equal(x | views::reverse | negate, x | negate |
> views::reverse) );
> + VERIFY( ranges::equal(x | (views::reverse | negate), x | (negate |
> views::reverse)) );
> +#if 0
> + // These asserts currently fail for the same reason as the disabled
> asserts
> + // in std/ranges/adaptors/all.cc.
> + static_assert( sizeof(negate | views::reverse | views::join) == 1 );
> + static_assert( sizeof(views::reverse | negate | views::join) == 1 );
> + static_assert( sizeof(views::reverse | views::join | negate) == 1 );
> +#endif
> +
> + return true;
> +}
> +
> +int
> +main()
> +{
> + static_assert(test01());
> +}
> diff --git a/libstdc++-v3/testsuite/std/ranges/version_c++23.cc
> b/libstdc++-v3/testsuite/std/ranges/version_c++23.cc
> index 04609bb602c..e8342fa986a 100644
> --- a/libstdc++-v3/testsuite/std/ranges/version_c++23.cc
> +++ b/libstdc++-v3/testsuite/std/ranges/version_c++23.cc
> @@ -4,7 +4,7 @@
> #include <version>
>
> #if __STDC_HOSTED__
> -# if __cpp_lib_ranges != 202110L
> +# if __cpp_lib_ranges != 202202L
> # error "Feature-test macro __cpp_lib_ranges has wrong value in
> <version>"
> # endif
> #endif
> --
> 2.40.0.335.g9857273be0
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] libstdc++: Implement range_adaptor_closure from P2387R3 [PR108827]
2023-04-17 7:26 ` Jonathan Wakely
@ 2023-04-18 8:57 ` Jonathan Wakely
2023-04-18 8:59 ` Jakub Jelinek
0 siblings, 1 reply; 9+ messages in thread
From: Jonathan Wakely @ 2023-04-18 8:57 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: Patrick Palka, gcc-patches, libstdc++, Jakub Jelinek
On Mon, 17 Apr 2023 at 08:26, Jonathan Wakely via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> On Mon, 17 Apr 2023, 05:28 Patrick Palka via Libstdc++, <
> libstdc++@gcc.gnu.org> wrote:
>
> > On Fri, 14 Apr 2023, Patrick Palka wrote:
> >
> > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
> > >
> > > PR libstdc++/108827
> > >
> > > libstdc++-v3/ChangeLog:
> > >
> > > * include/bits/ranges_cmp.h (__cpp_lib_ranges): Bump value
> > > for C++23.
> > > * include/std/ranges (range_adaptor_closure): Define for C++23.
> > > * include/std/version (__cpp_lib_ranges): Bump value for
> > > C++23.
> > > * testsuite/std/ranges/version_c++23.cc: Bump expected value
> > > of __cpp_lib_ranges.
> > > * testsuite/std/ranges/range_adaptor_closure.cc: New test.
> >
> > Here's a standalone version of this patch (not dependent on the first
> > patch in the series) it terms of the current non-CRTP representation
> > of _RangeAdaptorClosure:
> >
>
> OK, thanks for separating this from the other change.
I'd like RM approval to push this C++23-only change to gcc-13 as well as trunk.
>
>
>
> > -- >8 --
> >
> > Subject: [PATCH] libstdc++: Implement range_adaptor_closure from P2387R3
> > [PR108827]
> >
> > Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
> >
> > PR libstdc++/108827
> >
> > libstdc++-v3/ChangeLog:
> >
> > * include/bits/ranges_cmp.h (__cpp_lib_ranges): Bump value
> > for C++23.
> > * include/std/ranges (range_adaptor_closure): Define for C++23.
> > * include/std/version (__cpp_lib_ranges): Bump value for
> > C++23.
> > * testsuite/std/ranges/version_c++23.cc: Bump expected value
> > of __cpp_lib_ranges.
> > * testsuite/std/ranges/range_adaptor_closure.cc: New test.
> > ---
> > libstdc++-v3/include/bits/ranges_cmp.h | 4 ++
> > libstdc++-v3/include/std/ranges | 8 ++++
> > libstdc++-v3/include/std/version | 3 ++
> > .../std/ranges/range_adaptor_closure.cc | 46 +++++++++++++++++++
> > .../testsuite/std/ranges/version_c++23.cc | 2 +-
> > 5 files changed, 62 insertions(+), 1 deletion(-)
> > create mode 100644
> > libstdc++-v3/testsuite/std/ranges/range_adaptor_closure.cc
> >
> > diff --git a/libstdc++-v3/include/bits/ranges_cmp.h
> > b/libstdc++-v3/include/bits/ranges_cmp.h
> > index 85c1a77ccf7..6710d829a37 100644
> > --- a/libstdc++-v3/include/bits/ranges_cmp.h
> > +++ b/libstdc++-v3/include/bits/ranges_cmp.h
> > @@ -57,7 +57,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >
> > #ifdef __cpp_lib_concepts
> > // Define this here, included by all the headers that need to define it.
> > +#if __cplusplus > 202002L
> > +#define __cpp_lib_ranges 202202L
> > +#else
> > #define __cpp_lib_ranges 202110L
> > +#endif
> >
> > namespace ranges
> > {
> > diff --git a/libstdc++-v3/include/std/ranges
> > b/libstdc++-v3/include/std/ranges
> > index 1714f3fb16c..c4d4d85bf90 100644
> > --- a/libstdc++-v3/include/std/ranges
> > +++ b/libstdc++-v3/include/std/ranges
> > @@ -1122,6 +1122,14 @@ namespace views::__adaptor
> > };
> > } // namespace views::__adaptor
> >
> > +#if __cplusplus > 202002L
> > + template<typename _Derived>
> > + requires is_class_v<_Derived> && same_as<_Derived,
> > remove_cv_t<_Derived>>
> > + class range_adaptor_closure
> > + : public views::__adaptor::_RangeAdaptorClosure
> > + { };
> > +#endif
> > +
> > template<range _Range> requires is_object_v<_Range>
> > class ref_view : public view_interface<ref_view<_Range>>
> > {
> > diff --git a/libstdc++-v3/include/std/version
> > b/libstdc++-v3/include/std/version
> > index 027e5711ec5..02ead8f1443 100644
> > --- a/libstdc++-v3/include/std/version
> > +++ b/libstdc++-v3/include/std/version
> > @@ -265,8 +265,10 @@
> > #define __cpp_lib_interpolate 201902L
> > #if __cpp_lib_concepts
> > # define __cpp_lib_move_iterator_concept 202207L
> > +#if __cplusplus <= 202002L // N.B. updated value in C++23
> > # define __cpp_lib_ranges 202110L
> > #endif
> > +#endif
> > #define __cpp_lib_shift 201806L
> >
> > #if _GLIBCXX_HOSTED
> > @@ -330,6 +332,7 @@
> > #define __cpp_lib_reference_from_temporary 202202L
> > #define __cpp_lib_to_underlying 202102L
> > #define __cpp_lib_unreachable 202202L
> > +#define __cpp_lib_ranges 202202L
> > #define __cpp_lib_ranges_zip 202110L
> > #define __cpp_lib_ranges_chunk 202202L
> > #define __cpp_lib_ranges_slide 202202L
> > diff --git a/libstdc++-v3/testsuite/std/ranges/range_adaptor_closure.cc
> > b/libstdc++-v3/testsuite/std/ranges/range_adaptor_closure.cc
> > new file mode 100644
> > index 00000000000..6221f071331
> > --- /dev/null
> > +++ b/libstdc++-v3/testsuite/std/ranges/range_adaptor_closure.cc
> > @@ -0,0 +1,46 @@
> > +// { dg-options "-std=gnu++23" }
> > +// { dg-do run { target c++23 } }
> > +
> > +#include <ranges>
> > +#include <algorithm>
> > +#include <testsuite_hooks.h>
> > +
> > +namespace ranges = std::ranges;
> > +namespace views = std::views;
> > +
> > +struct _Negate : ranges::range_adaptor_closure<_Negate>
> > +{
> > + template<ranges::viewable_range _Range>
> > + constexpr auto
> > + operator()(_Range&& __r) const
> > + requires requires { views::transform(std::declval<_Range>(),
> > std::negate{}); }
> > + { return views::transform(std::forward<_Range>(__r), std::negate{}); }
> > +};
> > +
> > +constexpr _Negate negate;
> > +
> > +constexpr bool
> > +test01()
> > +{
> > + int x[] = {1, 2, 3};
> > + VERIFY( ranges::equal(x | negate, (int[]){-1, -2, -3}) );
> > + VERIFY( ranges::equal(x | negate | negate, x) );
> > + VERIFY( ranges::equal(x | (negate | negate), x) );
> > + VERIFY( ranges::equal(x | views::reverse | negate, x | negate |
> > views::reverse) );
> > + VERIFY( ranges::equal(x | (views::reverse | negate), x | (negate |
> > views::reverse)) );
> > +#if 0
> > + // These asserts currently fail for the same reason as the disabled
> > asserts
> > + // in std/ranges/adaptors/all.cc.
> > + static_assert( sizeof(negate | views::reverse | views::join) == 1 );
> > + static_assert( sizeof(views::reverse | negate | views::join) == 1 );
> > + static_assert( sizeof(views::reverse | views::join | negate) == 1 );
> > +#endif
> > +
> > + return true;
> > +}
> > +
> > +int
> > +main()
> > +{
> > + static_assert(test01());
> > +}
> > diff --git a/libstdc++-v3/testsuite/std/ranges/version_c++23.cc
> > b/libstdc++-v3/testsuite/std/ranges/version_c++23.cc
> > index 04609bb602c..e8342fa986a 100644
> > --- a/libstdc++-v3/testsuite/std/ranges/version_c++23.cc
> > +++ b/libstdc++-v3/testsuite/std/ranges/version_c++23.cc
> > @@ -4,7 +4,7 @@
> > #include <version>
> >
> > #if __STDC_HOSTED__
> > -# if __cpp_lib_ranges != 202110L
> > +# if __cpp_lib_ranges != 202202L
> > # error "Feature-test macro __cpp_lib_ranges has wrong value in
> > <version>"
> > # endif
> > #endif
> > --
> > 2.40.0.335.g9857273be0
> >
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] libstdc++: Implement range_adaptor_closure from P2387R3 [PR108827]
2023-04-18 8:57 ` Jonathan Wakely
@ 2023-04-18 8:59 ` Jakub Jelinek
0 siblings, 0 replies; 9+ messages in thread
From: Jakub Jelinek @ 2023-04-18 8:59 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: Jonathan Wakely, Patrick Palka, gcc-patches, libstdc++
On Tue, Apr 18, 2023 at 09:57:19AM +0100, Jonathan Wakely wrote:
> On Mon, 17 Apr 2023 at 08:26, Jonathan Wakely via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
> >
> > On Mon, 17 Apr 2023, 05:28 Patrick Palka via Libstdc++, <
> > libstdc++@gcc.gnu.org> wrote:
> >
> > > On Fri, 14 Apr 2023, Patrick Palka wrote:
> > >
> > > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
> > > >
> > > > PR libstdc++/108827
> > > >
> > > > libstdc++-v3/ChangeLog:
> > > >
> > > > * include/bits/ranges_cmp.h (__cpp_lib_ranges): Bump value
> > > > for C++23.
> > > > * include/std/ranges (range_adaptor_closure): Define for C++23.
> > > > * include/std/version (__cpp_lib_ranges): Bump value for
> > > > C++23.
> > > > * testsuite/std/ranges/version_c++23.cc: Bump expected value
> > > > of __cpp_lib_ranges.
> > > > * testsuite/std/ranges/range_adaptor_closure.cc: New test.
> > >
> > > Here's a standalone version of this patch (not dependent on the first
> > > patch in the series) it terms of the current non-CRTP representation
> > > of _RangeAdaptorClosure:
> > >
> >
> > OK, thanks for separating this from the other change.
>
> I'd like RM approval to push this C++23-only change to gcc-13 as well as trunk.
Ok.
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] libstdc++: Convert _RangeAdaptorClosure into a CRTP class [PR108827]
2023-04-17 3:24 ` [PATCH 1/2] libstdc++: Convert _RangeAdaptorClosure into a CRTP class [PR108827] Patrick Palka
@ 2023-08-16 16:05 ` Patrick Palka
2023-08-16 17:32 ` Jonathan Wakely
0 siblings, 1 reply; 9+ messages in thread
From: Patrick Palka @ 2023-08-16 16:05 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches, libstdc++
On Sun, Apr 16, 2023 at 11:24 PM Patrick Palka <ppalka@redhat.com> wrote:
>
> On Fri, 14 Apr 2023, Patrick Palka wrote:
>
> > 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 each instantiation of _RangeAdaptorClosure would then
> > introduce its own logically different hidden friends. So for example
> > during overload resolution for the outer pipe operator in
> >
> > :x | (views::reverse | views::join)
> >
> > we'd have to consider 6 different hidden operator| friends:
> >
> > 2 from _RangeAdaptorClosure<_Reverse>
> > 2 from _RangeAdaptorClosure<_Join>
> > 2 from _RangeAdaptorClosure<_Pipe<_Reverse, _Join>>
> >
> > which is wasteful and can even cause hard errors in some cases. So we
> > instead define the operator| overloads at namespace scope in an isolated
> > namespace.
>
> On second thought, since this doesn't fix a bug or add new functionality
> it seems more like GCC 14 material. The size reduction is nice but it's
> probably not a big deal in practice since adaptor pipelines are usually
> very transient objects that don't get passed around as function
> arguments etc.
Ping, does this look OK for trunk?
>
> But perhaps the second patch implementing range_adaptor_closure would be
> desirable for GCC 13? I'll post an updated standalone version of that
> patch for separate consideration.
>
> >
> > PR libstdc++/108827
> >
> > libstdc++-v3/ChangeLog:
> >
> > * include/std/ranges (__adaptor::_RangeAdaptorClosure): Move ...
> > (__adaptor::__closure::_RangeAdaptorClosure): ... here and turn
> > it into a CRTP class template. Move hidden operator| friends
> > into namespace scope and adjust their constraints. Add a
> > using-declaration for this at __adaptor::_RangeAdaptorClosure.
> > (__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: Reintroduce
> > static_assert expecting that adding empty range adaptor
> > closure objects to a pipeline doesn't increase the size of a
> > pipeline.
> > ---
> > libstdc++-v3/include/std/ranges | 69 +++++++++++--------
> > .../testsuite/std/ranges/adaptors/all.cc | 7 --
> > 2 files changed, 42 insertions(+), 34 deletions(-)
> >
> > diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
> > index 283d757faa4..531ec6f68b3 100644
> > --- a/libstdc++-v3/include/std/ranges
> > +++ b/libstdc++-v3/include/std/ranges
> > @@ -872,30 +872,45 @@ namespace views::__adaptor
> > template<typename _Lhs, typename _Rhs>
> > struct _Pipe;
> >
> > - // The base class of every range adaptor closure.
> > - //
> > - // 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
> > + namespace __closure
> > {
> > + // The base class of every range adaptor closure.
> > + //
> > + // 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.
> > + template<typename _Derived>
> > + struct _RangeAdaptorClosure
> > + { };
> > +
> > + template<typename _Tp, typename _Up>
> > + requires (!same_as<_Tp, _RangeAdaptorClosure<_Up>>)
> > + void __is_range_adaptor_closure_fn
> > + (const _Tp&, const _RangeAdaptorClosure<_Up>&); // not defined
> > +
> > + template<typename _Tp>
> > + concept __is_range_adaptor_closure
> > + = requires (_Tp __t) { __closure::__is_range_adaptor_closure_fn(__t, __t); };
> > +
> > // range | adaptor is equivalent to adaptor(range).
> > template<typename _Self, typename _Range>
> > - requires derived_from<remove_cvref_t<_Self>, _RangeAdaptorClosure>
> > + requires __is_range_adaptor_closure<_Self>
> > && __adaptor_invocable<_Self, _Range>
> > - friend constexpr auto
> > + 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<typename _Lhs, typename _Rhs>
> > - requires derived_from<_Lhs, _RangeAdaptorClosure>
> > - && derived_from<_Rhs, _RangeAdaptorClosure>
> > - friend constexpr auto
> > + 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)}; }
> > - };
> > + } // namespace __closure
> > +
> > + using __closure::_RangeAdaptorClosure;
> >
> > // The base class of every range adaptor non-closure.
> > //
> > @@ -937,7 +952,7 @@ namespace views::__adaptor
> > // A range adaptor closure that represents partial application of
> > // the range adaptor _Adaptor with arguments _Args.
> > template<typename _Adaptor, typename... _Args>
> > - struct _Partial : _RangeAdaptorClosure
> > + struct _Partial : _RangeAdaptorClosure<_Partial<_Adaptor, _Args...>>
> > {
> > tuple<_Args...> _M_args;
> >
> > @@ -978,7 +993,7 @@ namespace views::__adaptor
> > // A lightweight specialization of the above primary template for
> > // the common case where _Adaptor accepts a single extra argument.
> > template<typename _Adaptor, typename _Arg>
> > - struct _Partial<_Adaptor, _Arg> : _RangeAdaptorClosure
> > + struct _Partial<_Adaptor, _Arg> : _RangeAdaptorClosure<_Partial<_Adaptor, _Arg>>
> > {
> > _Arg _M_arg;
> >
> > @@ -1011,7 +1026,7 @@ namespace views::__adaptor
> > template<typename _Adaptor, typename... _Args>
> > 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;
> >
> > @@ -1041,7 +1056,7 @@ namespace views::__adaptor
> > template<typename _Adaptor, typename _Arg>
> > 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;
> >
> > @@ -1066,7 +1081,7 @@ namespace views::__adaptor
> > // A range adaptor closure that represents composition of the range
> > // adaptor closures _Lhs and _Rhs.
> > template<typename _Lhs, typename _Rhs>
> > - struct _Pipe : _RangeAdaptorClosure
> > + struct _Pipe : _RangeAdaptorClosure<_Pipe<_Lhs, _Rhs>>
> > {
> > [[no_unique_address]] _Lhs _M_lhs;
> > [[no_unique_address]] _Rhs _M_rhs;
> > @@ -1102,7 +1117,7 @@ namespace views::__adaptor
> > template<typename _Lhs, typename _Rhs>
> > 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;
> > @@ -1264,7 +1279,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<typename _Range>
> > static constexpr bool
> > @@ -3048,7 +3063,7 @@ namespace views::__adaptor
> > = requires { join_view<all_t<_Range>>{std::declval<_Range>()}; };
> > } // namespace __detail
> >
> > - struct _Join : __adaptor::_RangeAdaptorClosure
> > + struct _Join : __adaptor::_RangeAdaptorClosure<_Join>
> > {
> > template<viewable_range _Range>
> > requires __detail::__can_join_view<_Range>
> > @@ -3842,7 +3857,7 @@ namespace views::__adaptor
> > = requires { common_view{std::declval<_Range>()}; };
> > } // namespace __detail
> >
> > - struct _Common : __adaptor::_RangeAdaptorClosure
> > + struct _Common : __adaptor::_RangeAdaptorClosure<_Common>
> > {
> > template<viewable_range _Range>
> > requires __detail::__already_common<_Range>
> > @@ -3963,7 +3978,7 @@ namespace views::__adaptor
> > = requires { reverse_view{std::declval<_Range>()}; };
> > } // namespace __detail
> >
> > - struct _Reverse : __adaptor::_RangeAdaptorClosure
> > + struct _Reverse : __adaptor::_RangeAdaptorClosure<_Reverse>
> > {
> > template<viewable_range _Range>
> > requires __detail::__is_reverse_view<remove_cvref_t<_Range>>
> > @@ -4363,7 +4378,7 @@ namespace views::__adaptor
> > } // namespace __detail
> >
> > template<size_t _Nm>
> > - struct _Elements : __adaptor::_RangeAdaptorClosure
> > + struct _Elements : __adaptor::_RangeAdaptorClosure<_Elements<_Nm>>
> > {
> > template<viewable_range _Range>
> > requires __detail::__can_elements_view<_Nm, _Range>
> > @@ -5484,7 +5499,7 @@ namespace views::__adaptor
> > }
> >
> > template<size_t _Nm>
> > - struct _Adjacent : __adaptor::_RangeAdaptorClosure
> > + struct _Adjacent : __adaptor::_RangeAdaptorClosure<_Adjacent<_Nm>>
> > {
> > template<viewable_range _Range>
> > requires (_Nm == 0) || __detail::__can_adjacent_view<_Nm, _Range>
> > @@ -8609,7 +8624,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<viewable_range _Range>
> > requires __detail::__can_as_rvalue_view<_Range>
> > @@ -8918,7 +8933,7 @@ namespace views::__adaptor
> > = requires { enumerate_view<all_t<_Tp>>(std::declval<_Tp>()); };
> > }
> >
> > - struct _Enumerate : __adaptor::_RangeAdaptorClosure
> > + struct _Enumerate : __adaptor::_RangeAdaptorClosure<_Enumerate>
> > {
> > template<viewable_range _Range>
> > requires __detail::__can_enumerate_view<_Range>
> > @@ -9004,7 +9019,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<viewable_range _Range>
> > constexpr auto
> > diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc
> > index 57cb542174b..6bbaf73a20a 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<decltype(views::common
> > | views::common
> > | views::keys
> > | views::reverse)>);
> > -#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<auto all = views::all>
> > void
> > --
> > 2.40.0.335.g9857273be0
> >
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] libstdc++: Convert _RangeAdaptorClosure into a CRTP class [PR108827]
2023-08-16 16:05 ` Patrick Palka
@ 2023-08-16 17:32 ` Jonathan Wakely
0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Wakely @ 2023-08-16 17:32 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches, libstdc++
On Wed, 16 Aug 2023 at 17:06, Patrick Palka via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> On Sun, Apr 16, 2023 at 11:24 PM Patrick Palka <ppalka@redhat.com> wrote:
> >
> > On Fri, 14 Apr 2023, Patrick Palka wrote:
> >
> > > 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 each instantiation of _RangeAdaptorClosure would then
> > > introduce its own logically different hidden friends. So for example
> > > during overload resolution for the outer pipe operator in
> > >
> > > :x | (views::reverse | views::join)
> > >
> > > we'd have to consider 6 different hidden operator| friends:
> > >
> > > 2 from _RangeAdaptorClosure<_Reverse>
> > > 2 from _RangeAdaptorClosure<_Join>
> > > 2 from _RangeAdaptorClosure<_Pipe<_Reverse, _Join>>
> > >
> > > which is wasteful and can even cause hard errors in some cases. So we
> > > instead define the operator| overloads at namespace scope in an isolated
> > > namespace.
> >
> > On second thought, since this doesn't fix a bug or add new functionality
> > it seems more like GCC 14 material. The size reduction is nice but it's
> > probably not a big deal in practice since adaptor pipelines are usually
> > very transient objects that don't get passed around as function
> > arguments etc.
>
> Ping, does this look OK for trunk?
OK for trunk, thanks.
>
> >
> > But perhaps the second patch implementing range_adaptor_closure would be
> > desirable for GCC 13? I'll post an updated standalone version of that
> > patch for separate consideration.
> >
> > >
> > > PR libstdc++/108827
> > >
> > > libstdc++-v3/ChangeLog:
> > >
> > > * include/std/ranges (__adaptor::_RangeAdaptorClosure): Move ...
> > > (__adaptor::__closure::_RangeAdaptorClosure): ... here and turn
> > > it into a CRTP class template. Move hidden operator| friends
> > > into namespace scope and adjust their constraints. Add a
> > > using-declaration for this at __adaptor::_RangeAdaptorClosure.
> > > (__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: Reintroduce
> > > static_assert expecting that adding empty range adaptor
> > > closure objects to a pipeline doesn't increase the size of a
> > > pipeline.
> > > ---
> > > libstdc++-v3/include/std/ranges | 69 +++++++++++--------
> > > .../testsuite/std/ranges/adaptors/all.cc | 7 --
> > > 2 files changed, 42 insertions(+), 34 deletions(-)
> > >
> > > diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
> > > index 283d757faa4..531ec6f68b3 100644
> > > --- a/libstdc++-v3/include/std/ranges
> > > +++ b/libstdc++-v3/include/std/ranges
> > > @@ -872,30 +872,45 @@ namespace views::__adaptor
> > > template<typename _Lhs, typename _Rhs>
> > > struct _Pipe;
> > >
> > > - // The base class of every range adaptor closure.
> > > - //
> > > - // 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
> > > + namespace __closure
> > > {
> > > + // The base class of every range adaptor closure.
> > > + //
> > > + // 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.
> > > + template<typename _Derived>
> > > + struct _RangeAdaptorClosure
> > > + { };
> > > +
> > > + template<typename _Tp, typename _Up>
> > > + requires (!same_as<_Tp, _RangeAdaptorClosure<_Up>>)
> > > + void __is_range_adaptor_closure_fn
> > > + (const _Tp&, const _RangeAdaptorClosure<_Up>&); // not defined
> > > +
> > > + template<typename _Tp>
> > > + concept __is_range_adaptor_closure
> > > + = requires (_Tp __t) { __closure::__is_range_adaptor_closure_fn(__t, __t); };
> > > +
> > > // range | adaptor is equivalent to adaptor(range).
> > > template<typename _Self, typename _Range>
> > > - requires derived_from<remove_cvref_t<_Self>, _RangeAdaptorClosure>
> > > + requires __is_range_adaptor_closure<_Self>
> > > && __adaptor_invocable<_Self, _Range>
> > > - friend constexpr auto
> > > + 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<typename _Lhs, typename _Rhs>
> > > - requires derived_from<_Lhs, _RangeAdaptorClosure>
> > > - && derived_from<_Rhs, _RangeAdaptorClosure>
> > > - friend constexpr auto
> > > + 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)}; }
> > > - };
> > > + } // namespace __closure
> > > +
> > > + using __closure::_RangeAdaptorClosure;
> > >
> > > // The base class of every range adaptor non-closure.
> > > //
> > > @@ -937,7 +952,7 @@ namespace views::__adaptor
> > > // A range adaptor closure that represents partial application of
> > > // the range adaptor _Adaptor with arguments _Args.
> > > template<typename _Adaptor, typename... _Args>
> > > - struct _Partial : _RangeAdaptorClosure
> > > + struct _Partial : _RangeAdaptorClosure<_Partial<_Adaptor, _Args...>>
> > > {
> > > tuple<_Args...> _M_args;
> > >
> > > @@ -978,7 +993,7 @@ namespace views::__adaptor
> > > // A lightweight specialization of the above primary template for
> > > // the common case where _Adaptor accepts a single extra argument.
> > > template<typename _Adaptor, typename _Arg>
> > > - struct _Partial<_Adaptor, _Arg> : _RangeAdaptorClosure
> > > + struct _Partial<_Adaptor, _Arg> : _RangeAdaptorClosure<_Partial<_Adaptor, _Arg>>
> > > {
> > > _Arg _M_arg;
> > >
> > > @@ -1011,7 +1026,7 @@ namespace views::__adaptor
> > > template<typename _Adaptor, typename... _Args>
> > > 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;
> > >
> > > @@ -1041,7 +1056,7 @@ namespace views::__adaptor
> > > template<typename _Adaptor, typename _Arg>
> > > 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;
> > >
> > > @@ -1066,7 +1081,7 @@ namespace views::__adaptor
> > > // A range adaptor closure that represents composition of the range
> > > // adaptor closures _Lhs and _Rhs.
> > > template<typename _Lhs, typename _Rhs>
> > > - struct _Pipe : _RangeAdaptorClosure
> > > + struct _Pipe : _RangeAdaptorClosure<_Pipe<_Lhs, _Rhs>>
> > > {
> > > [[no_unique_address]] _Lhs _M_lhs;
> > > [[no_unique_address]] _Rhs _M_rhs;
> > > @@ -1102,7 +1117,7 @@ namespace views::__adaptor
> > > template<typename _Lhs, typename _Rhs>
> > > 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;
> > > @@ -1264,7 +1279,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<typename _Range>
> > > static constexpr bool
> > > @@ -3048,7 +3063,7 @@ namespace views::__adaptor
> > > = requires { join_view<all_t<_Range>>{std::declval<_Range>()}; };
> > > } // namespace __detail
> > >
> > > - struct _Join : __adaptor::_RangeAdaptorClosure
> > > + struct _Join : __adaptor::_RangeAdaptorClosure<_Join>
> > > {
> > > template<viewable_range _Range>
> > > requires __detail::__can_join_view<_Range>
> > > @@ -3842,7 +3857,7 @@ namespace views::__adaptor
> > > = requires { common_view{std::declval<_Range>()}; };
> > > } // namespace __detail
> > >
> > > - struct _Common : __adaptor::_RangeAdaptorClosure
> > > + struct _Common : __adaptor::_RangeAdaptorClosure<_Common>
> > > {
> > > template<viewable_range _Range>
> > > requires __detail::__already_common<_Range>
> > > @@ -3963,7 +3978,7 @@ namespace views::__adaptor
> > > = requires { reverse_view{std::declval<_Range>()}; };
> > > } // namespace __detail
> > >
> > > - struct _Reverse : __adaptor::_RangeAdaptorClosure
> > > + struct _Reverse : __adaptor::_RangeAdaptorClosure<_Reverse>
> > > {
> > > template<viewable_range _Range>
> > > requires __detail::__is_reverse_view<remove_cvref_t<_Range>>
> > > @@ -4363,7 +4378,7 @@ namespace views::__adaptor
> > > } // namespace __detail
> > >
> > > template<size_t _Nm>
> > > - struct _Elements : __adaptor::_RangeAdaptorClosure
> > > + struct _Elements : __adaptor::_RangeAdaptorClosure<_Elements<_Nm>>
> > > {
> > > template<viewable_range _Range>
> > > requires __detail::__can_elements_view<_Nm, _Range>
> > > @@ -5484,7 +5499,7 @@ namespace views::__adaptor
> > > }
> > >
> > > template<size_t _Nm>
> > > - struct _Adjacent : __adaptor::_RangeAdaptorClosure
> > > + struct _Adjacent : __adaptor::_RangeAdaptorClosure<_Adjacent<_Nm>>
> > > {
> > > template<viewable_range _Range>
> > > requires (_Nm == 0) || __detail::__can_adjacent_view<_Nm, _Range>
> > > @@ -8609,7 +8624,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<viewable_range _Range>
> > > requires __detail::__can_as_rvalue_view<_Range>
> > > @@ -8918,7 +8933,7 @@ namespace views::__adaptor
> > > = requires { enumerate_view<all_t<_Tp>>(std::declval<_Tp>()); };
> > > }
> > >
> > > - struct _Enumerate : __adaptor::_RangeAdaptorClosure
> > > + struct _Enumerate : __adaptor::_RangeAdaptorClosure<_Enumerate>
> > > {
> > > template<viewable_range _Range>
> > > requires __detail::__can_enumerate_view<_Range>
> > > @@ -9004,7 +9019,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<viewable_range _Range>
> > > constexpr auto
> > > diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc
> > > index 57cb542174b..6bbaf73a20a 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<decltype(views::common
> > > | views::common
> > > | views::keys
> > > | views::reverse)>);
> > > -#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<auto all = views::all>
> > > void
> > > --
> > > 2.40.0.335.g9857273be0
> > >
> > >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-08-16 17:32 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-14 19:45 [PATCH 1/2] libstdc++: Convert _RangeAdaptorClosure into a CRTP class [PR108827] Patrick Palka
2023-04-14 19:45 ` [PATCH 2/2] libstdc++: Implement range_adaptor_closure from P2387R3 [PR108827] Patrick Palka
2023-04-17 3:27 ` Patrick Palka
2023-04-17 7:26 ` Jonathan Wakely
2023-04-18 8:57 ` Jonathan Wakely
2023-04-18 8:59 ` Jakub Jelinek
2023-04-17 3:24 ` [PATCH 1/2] libstdc++: Convert _RangeAdaptorClosure into a CRTP class [PR108827] Patrick Palka
2023-08-16 16:05 ` Patrick Palka
2023-08-16 17:32 ` Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).