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 [PR *nnnnn*] 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? 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 > >
