> > Aha, it's because the only view that uses __max_diff_type is > iota_view, and that's an input range not an output range
Currently, for libstdc++, only iota_view and repeat_view use integer-class types. libc++ doesn't have integer-class types, not sure why they intended not to implement one. MSVC-STL uses this type in other places, such as the difference type of cartesian_product in some cases. It is reasonable to infer that this integer-class type will be used in libstdc++'s cartesian_product_view in the future because there is a *TODO* there, or it may be used to join_view or concat_view to resolve LWG 3666 <https://cplusplus.github.io/LWG/issue3666> or LWG 4073 <https://cplusplus.github.io/LWG/issue4073>. Jonathan Wakely <jwak...@redhat.com> 於 2025年4月4日 週五 下午6:27寫道： > On Fri, 4 Apr 2025 at 11:05, Hewill Kang <hewi...@gmail.com> wrote: > > > > Linstdc++’s __max_size_type is only explicitly converted to other > integral if I recall correctly. > > Hmm, yes, you're right. So why don't we get errors for __mindist not > converting back to an integer? > > Aha, it's because the only view that uses __max_diff_type is > iota_view, and that's an input range not an output range, so you can > only copy *from* iota_view, not to it. And that means for: > ranges::uninitialized_copy(from, to) > only the from range uses __max_diff_type, and so we convert > distance(to) to __max_diff_type, and it works fine. > > We would only need an explicit conversion if distance(to) was an > integer-like class type, and distance(from) was not. > > So we need a test with a custom output range that uses > iter_different_t<decltype(iota(__int128(0), __int128(0)))>. > > > > > > 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 > >> >> > >> > >
