On Fri, 7 Mar 2025, Jonathan Wakely wrote:
>
>
> On Fri, 7 Mar 2025 at 15:05, Tomasz Kamiński wrote:
> Add missing move_constructible && regular_invocable constrains on
> functor type,
> for invocations of `views::zip_transform` without range arguments.
>
> libstdc++-v3/ChangeLog:
>
> * include/std/ranges (_ZipTransform::operator()):
> Add `move_constructible` and `regular_invocable` constraints
> * testsuite/std/ranges/zip_transform/1.cc: New tests
> ---
> Tested on x86_64-linux. OK for trunk?
>
>
> I think the server hook will reject the commit message. Does `git gcc-verify`
> say it's OK?
>
> The summary line has [PR111138] but that PR number is not repeated in the
> ChangeLog part.
>
> There should be "<TAB>PR libstdc++/111138" either before the
> "libstdc++-v3/ChangeLog:" line, or before the "* include/std/ranges" line.
>
> This policy is documented at
> https://gcc.gnu.org/contribute.html#patches
>
> "If your patch relates a bug in the compiler for which there is an existing
> PR number the bug number should be stated. Use the short-form variant
> [PRnnnnn] without the Bugzilla component identifier and with
> no space between 'PR' and the number. The body of the commit message should
> still contain the full form (PR <component>/nnnnn) within the body of the
> commit message so that Bugzilla will correctly notice the
> commit."
>
>
> libstdc++-v3/include/std/ranges | 3 ++-
> .../testsuite/std/ranges/zip_transform/1.cc | 18 ++++++++++++++++++
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/libstdc++-v3/include/std/ranges
> b/libstdc++-v3/include/std/ranges
> index e21f5284b46..33e9926b89f 100644
> --- a/libstdc++-v3/include/std/ranges
> +++ b/libstdc++-v3/include/std/ranges
> @@ -5332,7 +5332,8 @@ namespace views::__adaptor
> struct _ZipTransform
> {
> template<typename _Fp, typename... _Ts>
> - requires (sizeof...(_Ts) == 0) ||
> __detail::__can_zip_transform_view<_Fp, _Ts...>
> + requires (sizeof...(_Ts) == 0) &&
> move_constructible<decay_t<_Fp>> && regular_invocable<decay_t<_Fp>&>
>
>
> I would prefer parentheses here so I don't have to think about the precedence.
>
> I think it's ((sizeof...(T) == 0) && move_cons && invoc) || can_zip_xform,
> right?
Pedantically I guess it should be
requires (sizeof...(T) == 0 && move_cons && invoc)
|| (sizeof...(T) != 0 && can_zip_xform)
We don't want/need to check can_zip_xform if we have no range arguments
and an unsuitable functor type.
>
> Is this still missing a check that decay_t<invoke_result_t<FD&>> is an object
> type?
>
> Maybe we want to create a helper concept which checks decay_t<_Fp>, e.g. add
> this to __detail:
>
> template<typename _Fd>
> concept __can_xform_empty // TODO: better name?
> = move_constructible<_Fd> && regular_invocable<_Fd&>
> && is_object_v<decay_t<invoke_result_t<_Fd&>>>;
>
> And then constrain _ZipTransform::operator() with:
>
> template<typename _Fp, typename... _Ts>
> requires (sizeof...(_Ts) == 0 && __detail::__can_xform_empty<decay_t<_Fp>>)
> || __detail::__can_zip_transform_view<_Fp, _Ts...>
>
> Or ... and maybe this is heresy ... we could overload operator()
>
> template<typename _Fp, typename... _Ts>
> requires __detail::__can_zip_transform_view<_Fp, _Ts...>
> constexpr auto
> operator() [[nodiscard]] (_Fp&& __f, _Ts&&... __ts) const
> {
> return zip_transform_view(std::forward<_Fp>(__f),
> std::forward<_Ts>(__ts)...);
> }
>
> template<typename _Fp>
> requires __detail::__can_xform_empty<decay_t<_Fp>>
> constexpr auto
> operator() [[nodiscard]] (_Fp&& __f) const
> {
> return views::empty<decay_t<invoke_result_t<decay_t<_Fp>&>>>;
> }
>
> It's simpler for me to understand this way, but we would have to pay the cost
> of overload resolution. So probably not a good idea.
>
>
>
> + || __detail::__can_zip_transform_view<_Fp, _Ts...>
> constexpr auto
> operator() [[nodiscard]] (_Fp&& __f, _Ts&&... __ts) const
> {
> diff --git a/libstdc++-v3/testsuite/std/ranges/zip_transform/1.cc
> b/libstdc++-v3/testsuite/std/ranges/zip_transform/1.cc
> index 20abdcba0f8..67839261cc7 100644
> --- a/libstdc++-v3/testsuite/std/ranges/zip_transform/1.cc
> +++ b/libstdc++-v3/testsuite/std/ranges/zip_transform/1.cc
> @@ -9,6 +9,20 @@
> namespace ranges = std::ranges;
> namespace views = std::views;
>
> +template<typename T>
> +concept can_zip_transform = requires (T t) {
> + views::zip_transform(std::forward<T>(t));
> +};
> +
> +static_assert(!can_zip_transform<int>);
> +
> +struct NonMovable {
> + NonMovable(NonMovable&&) = delete;
> +};
> +
> +static_assert(!can_zip_transform<NonMovable>);
> +static_assert(!can_zip_transform<NonMovable&>);
> +
> constexpr bool
> test01()
> {
> @@ -46,6 +60,10 @@ test01()
> VERIFY( ranges::size(z3) == 3 );
> VERIFY( ranges::equal(z3, (int[]){3, 6, 9}) );
>
> + auto z4 = views::zip_transform([] () { return 1; });
> + VERIFY( ranges::size(z4) == 0 );
> + static_assert( std::same_as<ranges::range_value_t<decltype(z4)>,
> int> );
> +
> return true;
> }
>
> --
> 2.48.1
>
>
>
