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 > >> > >