On Mon, 27 Jan 2025, Patrick Palka wrote:
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk/14?
>
> -- >8 --
>
> This case was incorrectly failing in C++23 mode even after P2492R2
> because the perfect forwarding simplification mechanism assumed bound
> arguments are copy-constructible which is no longer necessarily true
> after that paper. It'd be easy enough to fix the mechanism, but in
> C++23 mode the mechanism is no longer useful since we can just rely on
> deducing 'this' to implement perfect forwarding with a single overload
> (and done in r14-7150-gd2cb4693a0b383). So this patch just disables
> the mechanism in C++23 mode so that the generic implementation is always
> used.
I should add that this doesn't constitute an ABI change because the
disabled "simple" partial specializations of _Partial and _Pipe have the
same layout as the corresponding non-simple versions.
>
> PR libstdc++/118413
>
> libstdc++-v3/ChangeLog:
>
> * include/std/ranges
> (_GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING): New.
> (__closure_has_simple_call_op): Only define in C++20 mode.
> (__closure_has_simple_extra_args): Likewise.
> (_Partial, _Pipe): Likewise, for the "simple" partial
> specializations.
> (*::_S_has_simple_call_op): Likewise.
> (*::_S_has_simple_extra_args): Likewise.
> * testsuite/std/ranges/adaptors/100577.cc: Disable some
> implementation detail checks in C++23 mode.
> * testsuite/std/ranges/adaptors/transform.cc (test09): Also test
> partially applying the move-only function.
> ---
> libstdc++-v3/include/std/ranges | 53 +++++++++++++++++++
> .../testsuite/std/ranges/adaptors/100577.cc | 4 +-
> .../std/ranges/adaptors/transform.cc | 2 +
> 3 files changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
> index ad69a94b21f..a2b8748bea6 100644
> --- a/libstdc++-v3/include/std/ranges
> +++ b/libstdc++-v3/include/std/ranges
> @@ -1028,6 +1028,16 @@ namespace views::__adaptor
> }
> };
>
> +#if __cplusplus <= 202003L
> + // In C++20 mode we simplify perfect forwarding of a range adaptor
> closure's
> + // bound arguments when possible (according to their types), for sake of
> compile
> + // times and diagnostic quality. In C++23 mode we instead rely on
> deducing 'this'
> + // to idiomatically implement perfect forwarding. Note that this means the
> + // simplification logic doesn't consider C++23 <ranges> changes such as
> P2492R2.
> +# define _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING 1
> +#endif
> +
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> // True if the range adaptor closure _Adaptor has a simple operator(), i.e.
> // one that's not overloaded according to constness or value category of
> the
> // _Adaptor object.
> @@ -1039,6 +1049,7 @@ namespace views::__adaptor
> template<typename _Adaptor, typename... _Args>
> concept __adaptor_has_simple_extra_args =
> _Adaptor::_S_has_simple_extra_args
> || _Adaptor::template _S_has_simple_extra_args<_Args...>;
> +#endif
>
> // A range adaptor closure that represents partial application of
> // the range adaptor _Adaptor with arguments _Args.
> @@ -1139,6 +1150,7 @@ namespace views::__adaptor
> #endif
> };
>
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> // Partial specialization of the primary template for the case where the
> extra
> // arguments of the adaptor can always be safely and efficiently forwarded
> by
> // const reference. This lets us get away with a single operator()
> overload,
> @@ -1195,6 +1207,7 @@ namespace views::__adaptor
>
> static constexpr bool _S_has_simple_call_op = true;
> };
> +#endif // _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
>
> template<typename _Lhs, typename _Rhs, typename _Range>
> concept __pipe_invocable
> @@ -1245,6 +1258,7 @@ namespace views::__adaptor
> #endif
> };
>
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> // A partial specialization of the above primary template for the case
> where
> // both adaptor operands have a simple operator(). This in turn lets us
> // implement composition using a single simple operator(), which makes
> @@ -1271,6 +1285,7 @@ namespace views::__adaptor
>
> static constexpr bool _S_has_simple_call_op = true;
> };
> +#endif // _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> } // namespace views::__adaptor
>
> #if __cpp_lib_ranges >= 202202L
> @@ -1454,7 +1469,9 @@ namespace views::__adaptor
> return owning_view{std::forward<_Range>(__r)};
> }
>
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> static constexpr bool _S_has_simple_call_op = true;
> +#endif
> };
>
> inline constexpr _All all;
> @@ -1872,7 +1889,9 @@ namespace views::__adaptor
>
> using _RangeAdaptor<_Filter>::operator();
> static constexpr int _S_arity = 2;
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> static constexpr bool _S_has_simple_extra_args = true;
> +#endif
> };
>
> inline constexpr _Filter filter;
> @@ -2260,7 +2279,9 @@ namespace views::__adaptor
>
> using _RangeAdaptor<_Transform>::operator();
> static constexpr int _S_arity = 2;
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> static constexpr bool _S_has_simple_extra_args = true;
> +#endif
> };
>
> inline constexpr _Transform transform;
> @@ -2494,12 +2515,14 @@ namespace views::__adaptor
>
> using _RangeAdaptor<_Take>::operator();
> static constexpr int _S_arity = 2;
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> // The count argument of views::take is not always simple -- it can be
> // e.g. a move-only class that's implicitly convertible to the
> difference
> // type. But an integer-like count argument is surely simple.
> template<typename _Tp>
> static constexpr bool _S_has_simple_extra_args
> = ranges::__detail::__is_integer_like<_Tp>;
> +#endif
> };
>
> inline constexpr _Take take;
> @@ -2621,7 +2644,9 @@ namespace views::__adaptor
>
> using _RangeAdaptor<_TakeWhile>::operator();
> static constexpr int _S_arity = 2;
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> static constexpr bool _S_has_simple_extra_args = true;
> +#endif
> };
>
> inline constexpr _TakeWhile take_while;
> @@ -2777,9 +2802,11 @@ namespace views::__adaptor
>
> using _RangeAdaptor<_Drop>::operator();
> static constexpr int _S_arity = 2;
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> template<typename _Tp>
> static constexpr bool _S_has_simple_extra_args
> = _Take::_S_has_simple_extra_args<_Tp>;
> +#endif
> };
>
> inline constexpr _Drop drop;
> @@ -2866,7 +2893,9 @@ namespace views::__adaptor
>
> using _RangeAdaptor<_DropWhile>::operator();
> static constexpr int _S_arity = 2;
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> static constexpr bool _S_has_simple_extra_args = true;
> +#endif
> };
>
> inline constexpr _DropWhile drop_while;
> @@ -3273,7 +3302,9 @@ namespace views::__adaptor
> return join_view<all_t<_Range>>{std::forward<_Range>(__r)};
> }
>
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> static constexpr bool _S_has_simple_call_op = true;
> +#endif
> };
>
> inline constexpr _Join join;
> @@ -3730,6 +3761,7 @@ namespace views::__adaptor
>
> using _RangeAdaptor<_LazySplit>::operator();
> static constexpr int _S_arity = 2;
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> // The pattern argument of views::lazy_split is not always simple --
> it can be
> // a non-view range, the value category of which affects whether the
> call
> // is well-formed. But a scalar or a view pattern argument is surely
> @@ -3738,6 +3770,7 @@ namespace views::__adaptor
> static constexpr bool _S_has_simple_extra_args
> = is_scalar_v<_Pattern> || (view<_Pattern>
> && copy_constructible<_Pattern>);
> +#endif
> };
>
> inline constexpr _LazySplit lazy_split;
> @@ -3937,9 +3970,11 @@ namespace views::__adaptor
>
> using _RangeAdaptor<_Split>::operator();
> static constexpr int _S_arity = 2;
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> template<typename _Pattern>
> static constexpr bool _S_has_simple_extra_args
> = _LazySplit::_S_has_simple_extra_args<_Pattern>;
> +#endif
> };
>
> inline constexpr _Split split;
> @@ -4074,7 +4109,9 @@ namespace views::__adaptor
> return common_view{std::forward<_Range>(__r)};
> }
>
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> static constexpr bool _S_has_simple_call_op = true;
> +#endif
> };
>
> inline constexpr _Common common;
> @@ -4207,7 +4244,9 @@ namespace views::__adaptor
> return reverse_view{std::forward<_Range>(__r)};
> }
>
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> static constexpr bool _S_has_simple_call_op = true;
> +#endif
> };
>
> inline constexpr _Reverse reverse;
> @@ -4598,7 +4637,9 @@ namespace views::__adaptor
> return elements_view<all_t<_Range>, _Nm>{std::forward<_Range>(__r)};
> }
>
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> static constexpr bool _S_has_simple_call_op = true;
> +#endif
> };
>
> template<size_t _Nm>
> @@ -6061,7 +6102,9 @@ namespace views::__adaptor
>
> using __adaptor::_RangeAdaptor<_AdjacentTransform>::operator();
> static constexpr int _S_arity = 2;
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> static constexpr bool _S_has_simple_extra_args = true;
> +#endif
> };
>
> template<size_t _Nm>
> @@ -6617,7 +6660,9 @@ namespace views::__adaptor
>
> using __adaptor::_RangeAdaptor<_Chunk>::operator();
> static constexpr int _S_arity = 2;
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> static constexpr bool _S_has_simple_extra_args = true;
> +#endif
> };
>
> inline constexpr _Chunk chunk;
> @@ -6992,7 +7037,9 @@ namespace views::__adaptor
>
> using __adaptor::_RangeAdaptor<_Slide>::operator();
> static constexpr int _S_arity = 2;
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> static constexpr bool _S_has_simple_extra_args = true;
> +#endif
> };
>
> inline constexpr _Slide slide;
> @@ -7187,7 +7234,9 @@ namespace views::__adaptor
>
> using __adaptor::_RangeAdaptor<_ChunkBy>::operator();
> static constexpr int _S_arity = 2;
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> static constexpr bool _S_has_simple_extra_args = true;
> +#endif
> };
>
> inline constexpr _ChunkBy chunk_by;
> @@ -7715,9 +7764,11 @@ namespace views::__adaptor
>
> using _RangeAdaptor<_JoinWith>::operator();
> static constexpr int _S_arity = 2;
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> template<typename _Pattern>
> static constexpr bool _S_has_simple_extra_args
> = _LazySplit::_S_has_simple_extra_args<_Pattern>;
> +#endif
> };
>
> inline constexpr _JoinWith join_with;
> @@ -8333,7 +8384,9 @@ namespace views::__adaptor
>
> using __adaptor::_RangeAdaptor<_Stride>::operator();
> static constexpr int _S_arity = 2;
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> static constexpr bool _S_has_simple_extra_args = true;
> +#endif
> };
>
> inline constexpr _Stride stride;
> diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/100577.cc
> b/libstdc++-v3/testsuite/std/ranges/adaptors/100577.cc
> index 53c81be7f02..eaa8454b318 100644
> --- a/libstdc++-v3/testsuite/std/ranges/adaptors/100577.cc
> +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/100577.cc
> @@ -31,6 +31,7 @@ namespace views = std::ranges::views;
> void
> test01()
> {
> +#ifdef _GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING
> // Verify adaptors are deemed to have simple extra arguments when
> appropriate.
> using views::__adaptor::__adaptor_has_simple_extra_args;
> using std::identity;
> @@ -78,6 +79,7 @@ test01()
> static_assert(!__closure_has_simple_call_op<decltype(a12a | a00)>);
> static_assert(!__closure_has_simple_call_op<decltype(a00 | a12a)>);
> #endif
> +#endif
> }
>
> void
> @@ -135,7 +137,7 @@ test03()
> void
> test04()
> {
> -#if __STDC_HOSTED__
> +#if defined(_GLIBCXX_RANGES_SIMPLIFY_ADAPTOR_FORWARDING) && __STDC_HOSTED__
> // Non-trivially-copyable extra arguments make a closure not simple.
> using F = std::function<bool(bool)>;
> static_assert(!std::is_trivially_copyable_v<F>);
> diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc
> b/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc
> index dfc91fb5e1f..934d2f65dcf 100644
> --- a/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc
> +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc
> @@ -191,8 +191,10 @@ test09()
> #if __cpp_lib_ranges >= 202207L
> // P2494R2 Relaxing range adaptors to allow for move only types
> static_assert( requires { transform(x, move_only{}); } );
> + static_assert( requires { x | transform(move_only{}); } ); // PR
> libstdc++/118413
> #else
> static_assert( ! requires { transform(x, move_only{}); } );
> + static_assert( ! requires { x | transform(move_only{}); } );
> #endif
> }
>
> --
> 2.48.1.91.g5f8f7081f7
>
>
