[PATCH] libstdc++: Fix views::zip_transform constraints for empty range pack [PR111138]
Tomasz Kaminski
tkaminsk@redhat.com
Fri Mar 7 17:41:33 GMT 2025
On Fri, Mar 7, 2025 at 5:08 PM Jonathan Wakely <jwakely@redhat.com> wrote:
>
>
> On Fri, 7 Mar 2025 at 15:59, Patrick Palka <ppalka@redhat.com> wrote:
>
>> 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.
>>
>
> Ah yes.
>
> can_zip_xform will check sizeof...(T) > 0 because that's in the
> transform_view constraints, but only after checking move_constructible<F>.
>
We are also required to check `move_constructible<F>` (that I am adding as
part of the patch) for empty range pack case, so this is not observable.
But, I have added (sizeof...(Ts) != 0) check as I have two overloads, and
this will short-circuit.
>
>
>
>>
>> >
>> > 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
>> >
>> >
>> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20250307/9089e7b2/attachment-0001.htm>
More information about the Libstdc++
mailing list