libstdc++: Fix ranges::iter_move handling of rvalues [PR106612]

The specification for std::ranges::iter_move apparently requires us to
handle types which do not satisfy std::indirectly_readable, for example
with overloaded operator* which behaves differently for different value
categories.

libstdc++-v3/ChangeLog:

	PR libstdc++/106612
	* include/bits/iterator_concepts.h (_IterMove::__iter_ref_t):
	New alias template.
	(_IterMove::__result): Use __iter_ref_t instead of
	std::iter_reference_t.
	(_IterMove::__type): Remove incorrect __dereferenceable
	constraint.
	(_IterMove::operator()): Likewise. Add correct constraints. Use
	__iter_ref_t instead of std::iter_reference_t. Forward parameter
	as correct value category.
	(iter_swap): Add comments.
	* testsuite/24_iterators/customization_points/iter_move.cc: Test
	that iter_move is found by ADL and that rvalue arguments are
	handled correctly.

Reviewed-by: Patrick Palka <[email protected]>

Author: jwakely

libstdc++-v3/include/bits/iterator_concepts.h

@@ -103,32 +103,42 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   namespace ranges
   {
     /// @cond undocumented
+    // Implementation of std::ranges::iter_move, [iterator.cust.move].
     namespace __imove
     {
       void iter_move() = delete;
 
+      // Satisfied if _Tp is a class or enumeration type and iter_move
+      // can be found by argument-dependent lookup.
       template<typename _Tp>
 	concept __adl_imove
 	  = (std::__detail::__class_or_enum<remove_reference_t<_Tp>>)
-	  && requires(_Tp&& __t) { iter_move(static_cast<_Tp&&>(__t)); };
+	      && requires(_Tp&& __t) { iter_move(static_cast<_Tp&&>(__t)); };
 
       struct _IterMove
       {
       private:
+	// The type returned by dereferencing a value of type _Tp.
+	// Unlike iter_reference_t this preserves the value category of _Tp.
+	template<typename _Tp>
+	  using __iter_ref_t = decltype(*std::declval<_Tp>());
+
 	template<typename _Tp>
 	  struct __result
-	  { using type = iter_reference_t<_Tp>; };
+	  { using type = __iter_ref_t<_Tp>; };
 
+	// Use iter_move(E) if that works.
 	template<typename _Tp>
 	  requires __adl_imove<_Tp>
 	  struct __result<_Tp>
 	  { using type = decltype(iter_move(std::declval<_Tp>())); };
 
+	// Otherwise, if *E if an lvalue, use std::move(*E).
 	template<typename _Tp>
 	  requires (!__adl_imove<_Tp>)
-	  && is_lvalue_reference_v<iter_reference_t<_Tp>>
+	    && is_lvalue_reference_v<__iter_ref_t<_Tp>>
 	  struct __result<_Tp>
-	  { using type = remove_reference_t<iter_reference_t<_Tp>>&&; };
+	  { using type = remove_reference_t<__iter_ref_t<_Tp>>&&; };
 
 	template<typename _Tp>
 	  static constexpr bool
@@ -142,21 +152,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       public:
 	// The result type of iter_move(std::declval<_Tp>())
-	template<std::__detail::__dereferenceable _Tp>
+	template<typename _Tp>
 	  using __type = typename __result<_Tp>::type;
 
-	template<std::__detail::__dereferenceable _Tp>
+	template<typename _Tp>
+	  requires __adl_imove<_Tp> || requires { typename __iter_ref_t<_Tp>; }
 	  [[nodiscard]]
 	  constexpr __type<_Tp>
 	  operator()(_Tp&& __e) const
 	  noexcept(_S_noexcept<_Tp>())
 	  {
 	    if constexpr (__adl_imove<_Tp>)
 	      return iter_move(static_cast<_Tp&&>(__e));
-	    else if constexpr (is_lvalue_reference_v<iter_reference_t<_Tp>>)
-	      return static_cast<__type<_Tp>>(*__e);
+	    else if constexpr (is_lvalue_reference_v<__iter_ref_t<_Tp>>)
+	      return std::move(*static_cast<_Tp&&>(__e));
 	    else
-	      return *__e;
+	      return *static_cast<_Tp&&>(__e);
 	  }
       };
     } // namespace __imove
@@ -167,6 +178,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     }
   } // namespace ranges
 
+  /// The result type of ranges::iter_move(std::declval<_Tp&>())
   template<__detail::__dereferenceable _Tp>
     requires __detail::__can_reference<ranges::__imove::_IterMove::__type<_Tp&>>
     using iter_rvalue_reference_t = ranges::__imove::_IterMove::__type<_Tp&>;
@@ -873,11 +885,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 namespace ranges
 {
   /// @cond undocumented
+  // Implementation of std::ranges::iter_swap, [iterator.cust.swap].
   namespace __iswap
   {
     template<typename _It1, typename _It2>
       void iter_swap(_It1, _It2) = delete;
 
+    // Satisfied if _Tp and _Up are class or enumeration types and iter_swap
+    // can be found by argument-dependent lookup.
     template<typename _Tp, typename _Up>
       concept __adl_iswap
 	= (std::__detail::__class_or_enum<remove_reference_t<_Tp>>

libstdc++-v3/testsuite/24_iterators/customization_points/iter_move.cc

@@ -63,8 +63,103 @@ test01()
   VERIFY( test_X(3, 4) );
 }
 
+template<typename T>
+using rval_ref = std::iter_rvalue_reference_t<T>;
+
+static_assert(std::same_as<rval_ref<int*>, int&&>);
+static_assert(std::same_as<rval_ref<const int*>, const int&&>);
+static_assert(std::same_as<rval_ref<std::move_iterator<int*>>, int&&>);
+
+template<typename T>
+concept iter_movable = requires { std::ranges::iter_move(std::declval<T>()); };
+
+struct Iter
+{
+  friend int& iter_move(Iter&) { static int i = 1; return i; }
+  friend long iter_move(Iter&&) { return 2; }
+  const short& operator*() const & { static short s = 3; return s; }
+  friend float operator*(const Iter&&) { return 4.0f; }
+};
+
+void
+test_adl()
+{
+  Iter it;
+  const Iter& cit = it;
+
+  VERIFY( std::ranges::iter_move(it) == 1 );
+  VERIFY( std::ranges::iter_move(std::move(it)) == 2 );
+  VERIFY( std::ranges::iter_move(cit) == 3 );
+  VERIFY( std::ranges::iter_move(std::move(cit)) == 4.0f );
+
+  // The return type should be unchanged for ADL iter_move:
+  static_assert(std::same_as<decltype(std::ranges::iter_move(it)), int&>);
+  static_assert(std::same_as<decltype(std::ranges::iter_move(std::move(it))),
+			     long>);
+  // When ADL iter_move is not used, return type should be an rvalue:
+  static_assert(std::same_as<decltype(std::ranges::iter_move(cit)),
+			     const short&&>);
+  static_assert(std::same_as<decltype(std::ranges::iter_move(std::move(cit))),
+			     float>);
+
+  // std::iter_rvalue_reference_t always considers the argument as lvalue.
+  static_assert(std::same_as<rval_ref<Iter>, int&>);
+  static_assert(std::same_as<rval_ref<Iter&>, int&>);
+  static_assert(std::same_as<rval_ref<const Iter>, const short&&>);
+  static_assert(std::same_as<rval_ref<const Iter&>, const short&&>);
+}
+
+void
+test_pr106612()
+{
+  // Bug 106612 ranges::iter_move does not consider iterator's value categories
+
+  struct I
+  {
+    int i{};
+    int& operator*() & { return i; }
+    int operator*() const & { return i; }
+    void operator*() && = delete;
+  };
+
+  static_assert( iter_movable<I&> );
+  static_assert( iter_movable<I const&> );
+  static_assert( ! iter_movable<I> );
+  static_assert( std::same_as<std::iter_rvalue_reference_t<I>, int&&> );
+  static_assert( std::same_as<std::iter_rvalue_reference_t<const I>, int> );
+
+  struct I2
+  {
+    int i{};
+    int& operator*() & { return i; }
+    int operator*() const & { return i; }
+    void operator*() &&;
+  };
+
+  static_assert( iter_movable<I2&> );
+  static_assert( iter_movable<I2 const&> );
+  static_assert( iter_movable<I2> );
+  static_assert( std::is_void_v<decltype(std::ranges::iter_move(I2{}))> );
+  static_assert( std::same_as<std::iter_rvalue_reference_t<I2>, int&&> );
+  static_assert( std::same_as<std::iter_rvalue_reference_t<I2 const>, int> );
+
+  enum E { e };
+  enum F { f };
+
+  struct I3
+  {
+    E operator*() const & { return e; }
+    F operator*() && { return f; }
+  };
+
+  static_assert( iter_movable<I3&> );
+  static_assert( iter_movable<I3> );
+  static_assert( std::same_as<decltype(std::ranges::iter_move(I3{})), F> );
+}
+
 int
 main()
 {
   test01();
+  test_adl();
 }