[PATCH v2] libstdc++: Replace use of __mindist in ranges::uninitialized_xxx algos [PR101587]
Tomasz Kaminski
tkaminsk@redhat.com
Fri Apr 4 11:04:53 GMT 2025
On Fri, Apr 4, 2025 at 12:51 PM Jonathan Wakely <jwakely@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.
> *
> testsuite/20_util/specialized_algorithms/uninitialized_copy/constrained.cc:
> Check with ranges that use integer-like class type for
> difference type.
> *
> testsuite/20_util/specialized_algorithms/uninitialized_move/constrained.cc:
> Likewise.
>
> Reviewed-by: Tomasz Kaminski <tkaminsk@redhat.com>
> Reviewed-by: Hewill Kang <hewillk@gmail.com>
>
> ---
>
> Here's a v2 patch with added tests. No change to the code in the
> header, just to the tests.
>
> Tested x86_64-linux.
>
LGTM
>
> 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
> diff --git
> a/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_copy/constrained.cc
> b/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_copy/constrained.cc
> index af3b73364ec..5dff0da4d20 100644
> ---
> a/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_copy/constrained.cc
> +++
> b/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_copy/constrained.cc
> @@ -178,12 +178,40 @@ test03()
> void
> test_pr101587()
> {
> - short in[1];
> + short in[1]{};
> __gnu_test::test_contiguous_range r(in); // difference_type is
> integer-like
> long out[1];
> std::span<long> o(out); // difference_type is ptrdiff_t
> ranges::uninitialized_copy(r, o);
> ranges::uninitialized_copy_n(ranges::begin(r), 0, o.begin(), o.end());
> +
> + // iterator that has an integer-like class type for difference_type
> + struct Iter
> + {
> + using value_type = long;
> + using difference_type = std::ranges::__detail::__max_diff_type;
> +
> + long& operator*() const { return *p; }
> +
> + Iter& operator++() { ++p; return *this; }
> + Iter operator++(int) { return Iter{p++}; }
> +
> + difference_type operator-(Iter i) const { return p - i.p; }
> + bool operator==(const Iter&) const = default;
> +
> + long* p = nullptr;
> + };
> + static_assert(std::sized_sentinel_for<Iter, Iter>);
> +
> + std::ranges::subrange<Iter> rmax(Iter{out+0}, Iter{out+1});
> + // Check with integer-like class type for output range:
> + std::ranges::uninitialized_copy(in, rmax);
> + std::ranges::uninitialized_copy_n(in+0, 1, rmax.begin(), rmax.end());
> +
> + int to[1];
> + // And for input range:
> + std::ranges::uninitialized_copy(rmax, to);
> + std::ranges::uninitialized_copy_n(rmax.begin(), 1, to+0, to+1);
> }
>
> int
> diff --git
> a/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_move/constrained.cc
> b/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_move/constrained.cc
> index fe82d1f156e..3e8124492d4 100644
> ---
> a/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_move/constrained.cc
> +++
> b/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_move/constrained.cc
> @@ -188,12 +188,39 @@ test03()
> void
> test_pr101587()
> {
> - short in[1];
> + short in[1]{};
> __gnu_test::test_contiguous_range r(in); // difference_type is
> integer-like
> long out[1];
> std::span<long> o(out); // difference_type is ptrdiff_t
> ranges::uninitialized_move(r, o);
> ranges::uninitialized_move_n(ranges::begin(r), 0, o.begin(), o.end());
> +
> + struct Iter
> + {
> + using value_type = long;
> + using difference_type = std::ranges::__detail::__max_diff_type;
> +
> + long& operator*() const { return *p; }
> +
> + Iter& operator++() { ++p; return *this; }
> + Iter operator++(int) { return Iter{p++}; }
> +
> + difference_type operator-(Iter i) const { return p - i.p; }
> + bool operator==(const Iter&) const = default;
> +
> + long* p = nullptr;
> + };
> + static_assert(std::sized_sentinel_for<Iter, Iter>);
> +
> + std::ranges::subrange<Iter> rmax(Iter{out+0}, Iter{out+1});
> + // Check with integer-like class type for output range:
> + std::ranges::uninitialized_move(in, rmax);
> + std::ranges::uninitialized_move_n(in+0, 1, rmax.begin(), rmax.end());
> +
> + int to[1];
> + // And for input range:
> + std::ranges::uninitialized_copy(rmax, to);
> + std::ranges::uninitialized_copy_n(rmax.begin(), 1, to+0, to+1);
> }
>
> int
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20250404/637077e6/attachment.htm>
More information about the Libstdc++
mailing list