Linstdc++’s __max_size_type is only explicitly converted to other integral
if I recall correctly.

Jonathan Wakely <jwak...@redhat.com>於 2025年4月4日 週五,17:56寫道:

> On Fri, 4 Apr 2025 at 10:32, Tomasz Kaminski <tkami...@redhat.com> wrote:
> >
> >
> >
> > On Fri, Apr 4, 2025 at 11:10 AM Jonathan Wakely <jwak...@redhat.com>
> wrote:
> >>
> >> In r15-8980-gf4b6acfc36fb1f I introduced a new function object for
> >> finding the smaller of two distances. In bugzilla Hewill Kang pointed
> >> out that we still need to explicitly convert the result back to the
> >> right difference type, because the result might be an integer-like class
> >> type that doesn't convert to an integral type explicitly.
> >>
> >> Rather than doing that conversion in the __mindist function object, I
> >> think it's simpler to remove it again and just do a comparison and
> >> assignment. We always want the result to have a specific type, so we can
> >> just check if the value of the other type is smaller, and then convert
> >> that to the other type if so.
> >>
> >> libstdc++-v3/ChangeLog:
> >>
> >>         PR libstdc++/101587
> >>         * include/bits/ranges_uninitialized.h (__detail::__mindist):
> >>         Remove.
> >>         (ranges::uninitialized_copy, ranges::uninitialized_copy_n)
> >>         (ranges::uninitialized_move, ranges::uninitialized_move_n): Use
> >>         comparison and assignment instead of __mindist.
> >> ---
> >>
> >> Tested x86_64-linux.
> >
> > The revert makes sense, but this integer-like-classes are causing a lot
> of churm.
> > I would like to see some tests for this scenario in algorithms tests.
> > Using iota<__int128>(0, 5) is good way to produce integer-like
> difference type,
> > that is sized. (iota<__int128>(0) | views::take(5)) creates not-sized
> one.
>
> Yes, but our integer-like class type (__max_size_type) is implicitly
> convertible to integer types, so is more user-friendly than required
> by the standard. I don't think we want to change that, but it means
> it's easy to miss places where portable code should really be using an
> explicit conversion.
>
> Actually I'm not sure this patch is even needed ... because the only
> integer-like class type that is allowed is the one provided by the
> library ... and we don't need the explicit conversion for our type.
> But I like the simplification anyway.
>
> Anyway, I'll add some more tests using iota_view, but they were
> already passing before this patch, that's why I didn't bother.
>
>
> > With test LGTM.
> >
> >>
> >>
> >>  .../include/bits/ranges_uninitialized.h       | 46 ++++++-------------
> >>  1 file changed, 14 insertions(+), 32 deletions(-)
> >>
> >> diff --git a/libstdc++-v3/include/bits/ranges_uninitialized.h
> b/libstdc++-v3/include/bits/ranges_uninitialized.h
> >> index b5580073a6a..12a714b68aa 100644
> >> --- a/libstdc++-v3/include/bits/ranges_uninitialized.h
> >> +++ b/libstdc++-v3/include/bits/ranges_uninitialized.h
> >> @@ -263,26 +263,6 @@ namespace ranges
> >>    inline constexpr __uninitialized_value_construct_n_fn
> >>      uninitialized_value_construct_n;
> >>
> >> -  namespace __detail
> >> -  {
> >> -    // This is only intended for finding smaller iterator differences
> below,
> >> -    // not as a general purpose replacement for std::min.
> >> -    struct __mindist_fn
> >> -    {
> >> -      template<typename _Dp1, typename _Dp2>
> >> -       constexpr common_type_t<_Dp1, _Dp2>
> >> -       operator()(_Dp1 __d1, _Dp2 __d2) const noexcept
> >> -       {
> >> -         // Every C++20 iterator I satisfies weakly_incrementable<I>
> which
> >> -         // requires signed-integer-like<iter_difference_t<I>>.
> >> -         static_assert(std::__detail::__is_signed_integer_like<_Dp1>);
> >> -         static_assert(std::__detail::__is_signed_integer_like<_Dp2>);
> >> -         return std::min<common_type_t<_Dp1, _Dp2>>(__d1, __d2);
> >> -       }
> >> -    };
> >> -    inline constexpr __mindist_fn __mindist{};
> >> -  }
> >> -
> >>    template<typename _Iter, typename _Out>
> >>      using uninitialized_copy_result = in_out_result<_Iter, _Out>;
> >>
> >> @@ -305,10 +285,10 @@ namespace ranges
> >>                       && is_trivially_assignable_v<_OutType&,
> >>
> iter_reference_t<_Iter>>)
> >>           {
> >> -           auto __d1 = __ilast - __ifirst;
> >> -           auto __d2 = __olast - __ofirst;
> >> -           return ranges::copy_n(std::move(__ifirst),
> >> -                                 __detail::__mindist(__d1, __d2),
> __ofirst);
> >> +           auto __d = __ilast - __ifirst;
> >> +           if (auto __d2 = __olast - __ofirst; __d2 < __d)
> >> +             __d = static_cast<iter_difference_t<_Iter>>(__d2);
> >> +           return ranges::copy_n(std::move(__ifirst), __d, __ofirst);
> >>           }
> >>         else
> >>           {
> >> @@ -356,9 +336,9 @@ namespace ranges
> >>                       && is_trivially_assignable_v<_OutType&,
> >>
> iter_reference_t<_Iter>>)
> >>           {
> >> -           auto __d = __olast - __ofirst;
> >> -           return ranges::copy_n(std::move(__ifirst),
> >> -                                 __detail::__mindist(__n, __d),
> __ofirst);
> >> +           if (auto __d = __olast - __ofirst; __d < __n)
> >> +             __n = static_cast<iter_difference_t<_Iter>>(__d);
> >> +           return ranges::copy_n(std::move(__ifirst), __n, __ofirst);
> >>           }
> >>         else
> >>           {
> >> @@ -397,11 +377,12 @@ namespace ranges
> >>                       && is_trivially_assignable_v<_OutType&,
> >>
> iter_rvalue_reference_t<_Iter>>)
> >>           {
> >> -           auto __d1 = __ilast - __ifirst;
> >> -           auto __d2 = __olast - __ofirst;
> >> +           auto __d = __ilast - __ifirst;
> >> +           if (auto __d2 = __olast - __ofirst; __d2 < __d)
> >> +             __d = static_cast<iter_difference_t<_Iter>>(__d2);
> >>             auto [__in, __out]
> >>               =
> ranges::copy_n(std::make_move_iterator(std::move(__ifirst)),
> >> -                              __detail::__mindist(__d1, __d2),
> __ofirst);
> >> +                              __d, __ofirst);
> >>             return {std::move(__in).base(), __out};
> >>           }
> >>         else
> >> @@ -452,10 +433,11 @@ namespace ranges
> >>                       && is_trivially_assignable_v<_OutType&,
> >>
> iter_rvalue_reference_t<_Iter>>)
> >>           {
> >> -           auto __d = __olast - __ofirst;
> >> +           if (auto __d = __olast - __ofirst; __d < __n)
> >> +             __n = static_cast<iter_difference_t<_Iter>>(__d);
> >>             auto [__in, __out]
> >>               =
> ranges::copy_n(std::make_move_iterator(std::move(__ifirst)),
> >> -                              __detail::__mindist(__n, __d), __ofirst);
> >> +                              __n, __ofirst);
> >>             return {std::move(__in).base(), __out};
> >>           }
> >>         else
> >> --
> >> 2.49.0
> >>
>
>

Reply via email to