On Thu, 27 Feb 2025, Jonathan Wakely wrote:

> The ranges::move and ranges::move_backward algorithms are supposed to
> use ranges::iter_move(iter) instead of std::move(*iter), which matters
> for an iterator type with an iter_move overload findable by ADL.
> 
> Currently those algorithms use std::__assign_one which uses std::move,
> so define a new ranges::__detail::__assign_one helper function that uses
> ranges::iter_move.
> 
> libstdc++-v3/ChangeLog:
> 
>       PR libstdc++/105609
>       * include/bits/ranges_algobase.h (__detail::__assign_one): New
>       helper function.
>       (__copy_or_move, __copy_or_move_backward): Use new function
>       instead of std::__assign_one.
>       * testsuite/25_algorithms/move/constrained.cc: Check that
>       ADL iter_move is used in preference to std::move.
>       * testsuite/25_algorithms/move_backward/constrained.cc:
>       Likewise.

LGTM

> ---
> 
> Tested x86_64-linux.
> 
>  libstdc++-v3/include/bits/ranges_algobase.h   | 26 +++++++++++++----
>  .../25_algorithms/move/constrained.cc         | 29 +++++++++++++++++++
>  .../move_backward/constrained.cc              | 29 +++++++++++++++++++
>  3 files changed, 78 insertions(+), 6 deletions(-)
> 
> diff --git a/libstdc++-v3/include/bits/ranges_algobase.h 
> b/libstdc++-v3/include/bits/ranges_algobase.h
> index eceb859e88b..a08f659b3ae 100644
> --- a/libstdc++-v3/include/bits/ranges_algobase.h
> +++ b/libstdc++-v3/include/bits/ranges_algobase.h
> @@ -188,6 +188,20 @@ namespace ranges
>  
>    inline constexpr __equal_fn equal{};
>  
> +namespace __detail
> +{
> +  template<bool _IsMove, typename _OutIter, typename _InIter>
> +    [[__gnu__::__always_inline__]]
> +    constexpr void
> +    __assign_one(_OutIter& __out, _InIter& __in)
> +    {
> +      if constexpr (_IsMove)
> +     *__out = ranges::iter_move(__in);
> +      else
> +     *__out = *__in;
> +    }
> +} // namespace __detail
> +
>    template<typename _Iter, typename _Out>
>      struct in_out_result
>      {
> @@ -291,14 +305,14 @@ namespace ranges
>                   __builtin_memmove(__result, __first,
>                                     sizeof(_ValueTypeI) * __num);
>                 else if (__num == 1)
> -                 std::__assign_one<_IsMove>(__result, __first);
> +                 __detail::__assign_one<_IsMove>(__result, __first);
>                 return {__first + __num, __result + __num};
>               }
>           }
>  
>         for (auto __n = __last - __first; __n > 0; --__n)
>           {
> -           std::__assign_one<_IsMove>(__result, __first);
> +           __detail::__assign_one<_IsMove>(__result, __first);
>             ++__first;
>             ++__result;
>           }
> @@ -308,7 +322,7 @@ namespace ranges
>       {
>         while (__first != __last)
>           {
> -           std::__assign_one<_IsMove>(__result, __first);
> +           __detail::__assign_one<_IsMove>(__result, __first);
>             ++__first;
>             ++__result;
>           }
> @@ -420,7 +434,7 @@ namespace ranges
>                   __builtin_memmove(__result, __first,
>                                     sizeof(_ValueTypeI) * __num);
>                 else if (__num == 1)
> -                 std::__assign_one<_IsMove>(__result, __first);
> +                 __detail::__assign_one<_IsMove>(__result, __first);
>                 return {__first + __num, __result};
>               }
>           }
> @@ -432,7 +446,7 @@ namespace ranges
>           {
>             --__tail;
>             --__result;
> -           std::__assign_one<_IsMove>(__result, __tail);
> +           __detail::__assign_one<_IsMove>(__result, __tail);
>           }
>         return {std::move(__lasti), std::move(__result)};
>       }
> @@ -445,7 +459,7 @@ namespace ranges
>           {
>             --__tail;
>             --__result;
> -           std::__assign_one<_IsMove>(__result, __tail);
> +           __detail::__assign_one<_IsMove>(__result, __tail);
>           }
>         return {std::move(__lasti), std::move(__result)};
>       }
> diff --git a/libstdc++-v3/testsuite/25_algorithms/move/constrained.cc 
> b/libstdc++-v3/testsuite/25_algorithms/move/constrained.cc
> index 587b2f3728b..e2b45b070ef 100644
> --- a/libstdc++-v3/testsuite/25_algorithms/move/constrained.cc
> +++ b/libstdc++-v3/testsuite/25_algorithms/move/constrained.cc
> @@ -204,6 +204,35 @@ test05()
>    VERIFY( ranges::equal(v, (int[]){1,2,3,0}) );
>  }
>  
> +namespace pr105609
> +{
> +  struct I {
> +    using value_type = int;
> +    using difference_type = std::ptrdiff_t;
> +    int operator*() const;
> +    I& operator++();
> +    I operator++(int);
> +    I& operator--();
> +    I operator--(int);
> +    bool operator==(I) const;
> +    friend int& iter_move(const I&);
> +  };
> +}
> +
> +void
> +test06(pr105609::I i)
> +{
> +  // PR libstdc++/105609
> +  // ranges::move should use ranges::iter_move instead of std::move
> +  struct O {
> +    O(int&) { }
> +    O(int&&) = delete;
> +  };
> +
> +  O* o = nullptr;
> +  std::ranges::move(i, i, o);
> +}
> +
>  int
>  main()
>  {
> diff --git 
> a/libstdc++-v3/testsuite/25_algorithms/move_backward/constrained.cc 
> b/libstdc++-v3/testsuite/25_algorithms/move_backward/constrained.cc
> index 8f6fd455b4b..4d94d386dd0 100644
> --- a/libstdc++-v3/testsuite/25_algorithms/move_backward/constrained.cc
> +++ b/libstdc++-v3/testsuite/25_algorithms/move_backward/constrained.cc
> @@ -160,6 +160,35 @@ test03()
>    return ok;
>  }
>  
> +namespace pr105609
> +{
> +  struct I {
> +    using value_type = int;
> +    using difference_type = std::ptrdiff_t;
> +    int operator*() const;
> +    I& operator++();
> +    I operator++(int);
> +    I& operator--();
> +    I operator--(int);
> +    bool operator==(I) const;
> +    friend int& iter_move(const I&);
> +  };
> +}
> +
> +void
> +test04(pr105609::I i)
> +{
> +  // PR libstdc++/105609
> +  // ranges::move should use ranges::iter_move instead of std::move
> +  struct O {
> +    O(int&) { }
> +    O(int&&) = delete;
> +  };
> +
> +  O* o = nullptr;
> +  std::ranges::move_backward(i, i, o);
> +}
> +
>  int
>  main()
>  {
> -- 
> 2.48.1
> 
> 

Reply via email to