libstdc++: ranges::rotate should use ranges::iter_move [PR121913]
Commit Message
Using std::move(*it) is incorrect for iterators that use proxy refs, we
should use ranges::iter_move(it) instead.
libstdc++-v3/ChangeLog:
PR libstdc++/121913
* include/bits/ranges_algo.h (__rotate_fn::operator()): Use
ranges::iter_move(it) instead of std::move(*it).
* testsuite/25_algorithms/rotate/121913.cc: New test.
---
The testcase is highly contrived, but the fix seems worth doing anyway.
Tested powerpc64le-linux.
libstdc++-v3/include/bits/ranges_algo.h | 4 +-
.../testsuite/25_algorithms/rotate/121913.cc | 45 +++++++++++++++++++
2 files changed, 47 insertions(+), 2 deletions(-)
create mode 100644 libstdc++-v3/testsuite/25_algorithms/rotate/121913.cc
Comments
On Thu, 11 Sep 2025, Jonathan Wakely wrote:
> Using std::move(*it) is incorrect for iterators that use proxy refs, we
> should use ranges::iter_move(it) instead.
>
> libstdc++-v3/ChangeLog:
>
> PR libstdc++/121913
> * include/bits/ranges_algo.h (__rotate_fn::operator()): Use
> ranges::iter_move(it) instead of std::move(*it).
> * testsuite/25_algorithms/rotate/121913.cc: New test.
> ---
>
> The testcase is highly contrived, but the fix seems worth doing anyway.
>
> Tested powerpc64le-linux.
>
> libstdc++-v3/include/bits/ranges_algo.h | 4 +-
> .../testsuite/25_algorithms/rotate/121913.cc | 45 +++++++++++++++++++
> 2 files changed, 47 insertions(+), 2 deletions(-)
> create mode 100644 libstdc++-v3/testsuite/25_algorithms/rotate/121913.cc
>
> diff --git a/libstdc++-v3/include/bits/ranges_algo.h b/libstdc++-v3/include/bits/ranges_algo.h
> index 609b82e26d8a..9b348d82ce55 100644
> --- a/libstdc++-v3/include/bits/ranges_algo.h
> +++ b/libstdc++-v3/include/bits/ranges_algo.h
> @@ -1685,7 +1685,7 @@ namespace ranges
> {
> auto __mid = ranges::next(__p, __n - 1);
> auto __end = ranges::next(__mid);
> - auto __t = std::move(*__p);
> + auto __t = ranges::iter_move(__p);
> ranges::move(ranges::next(__p), __end, __p);
> *__mid = std::move(__t);
> return {std::move(__ret), std::move(__lasti)};
LGTM, this is a clear improvement.
The code paths in question are an optimization to perform moves instead
of swaps; I assume this optimization is valid/unobservable for iterators
that return proxy refs (but whose value type is POD)?
> @@ -1713,7 +1713,7 @@ namespace ranges
> {
> auto __mid = ranges::next(__p, __n - 1);
> auto __end = ranges::next(__mid);
> - auto __t = std::move(*__mid);
> + auto __t = ranges::iter_move(__mid);
> ranges::move_backward(__p, __mid, __end);
> *__p = std::move(__t);
> return {std::move(__ret), std::move(__lasti)};
> diff --git a/libstdc++-v3/testsuite/25_algorithms/rotate/121913.cc b/libstdc++-v3/testsuite/25_algorithms/rotate/121913.cc
> new file mode 100644
> index 000000000000..d648699adbda
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/25_algorithms/rotate/121913.cc
> @@ -0,0 +1,45 @@
> +// { dg-do compile { target c++20 } }
> +
> +// Bug libstdc++/121913 ranges::rotate should use ranges::iter_move
> +
> +#include <algorithm>
> +
> +struct A { };
> +
> +struct B
> +{
> + B& operator=(const B&) = delete;
> + B& operator=(const A&) const;
> +
> + operator A() const;
> +};
> +
> +struct I
> +{
> + using value_type = A;
> + using difference_type = int;
> + B operator*() const;
> + B operator[](int) const;
> + I& operator++();
> + I operator++(int);
> + I& operator--();
> + I operator--(int);
> + I& operator+=(int);
> + I& operator-=(int);
> +
> + auto operator<=>(const I&) const = default;
> +
> + friend A iter_move(const I&);
> + friend I operator+(I, int);
> + friend I operator-(I, int);
> + friend I operator+(int, I);
> + friend int operator-(I, I);
> +};
> +
> +static_assert( std::random_access_iterator<I> );
> +
> +void
> +test_pr121913()
> +{
> + std::ranges::rotate(I{}, I{}, I{});
> +}
> --
> 2.51.0
>
>
@@ -1685,7 +1685,7 @@ namespace ranges
{
auto __mid = ranges::next(__p, __n - 1);
auto __end = ranges::next(__mid);
- auto __t = std::move(*__p);
+ auto __t = ranges::iter_move(__p);
ranges::move(ranges::next(__p), __end, __p);
*__mid = std::move(__t);
return {std::move(__ret), std::move(__lasti)};
@@ -1713,7 +1713,7 @@ namespace ranges
{
auto __mid = ranges::next(__p, __n - 1);
auto __end = ranges::next(__mid);
- auto __t = std::move(*__mid);
+ auto __t = ranges::iter_move(__mid);
ranges::move_backward(__p, __mid, __end);
*__p = std::move(__t);
return {std::move(__ret), std::move(__lasti)};
new file mode 100644
@@ -0,0 +1,45 @@
+// { dg-do compile { target c++20 } }
+
+// Bug libstdc++/121913 ranges::rotate should use ranges::iter_move
+
+#include <algorithm>
+
+struct A { };
+
+struct B
+{
+ B& operator=(const B&) = delete;
+ B& operator=(const A&) const;
+
+ operator A() const;
+};
+
+struct I
+{
+ using value_type = A;
+ using difference_type = int;
+ B operator*() const;
+ B operator[](int) const;
+ I& operator++();
+ I operator++(int);
+ I& operator--();
+ I operator--(int);
+ I& operator+=(int);
+ I& operator-=(int);
+
+ auto operator<=>(const I&) const = default;
+
+ friend A iter_move(const I&);
+ friend I operator+(I, int);
+ friend I operator-(I, int);
+ friend I operator+(int, I);
+ friend int operator-(I, I);
+};
+
+static_assert( std::random_access_iterator<I> );
+
+void
+test_pr121913()
+{
+ std::ranges::rotate(I{}, I{}, I{});
+}