From: Jonathan Wakely Date: Fri, 28 Feb 2025 21:44:41 +0000 (+0000) Subject: libstdc++: Fix ranges::iter_move handling of rvalues [PR106612] X-Git-Tag: basepoints/gcc-16~1787 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a8ee522c5923ba17851e4b71316a2dff19d6368f;p=thirdparty%2Fgcc.git 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 --- diff --git a/libstdc++-v3/include/bits/iterator_concepts.h b/libstdc++-v3/include/bits/iterator_concepts.h index 4265c475273b..a201e24d2e23 100644 --- a/libstdc++-v3/include/bits/iterator_concepts.h +++ b/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 concept __adl_imove = (std::__detail::__class_or_enum>) - && 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 + using __iter_ref_t = decltype(*std::declval<_Tp>()); + template struct __result - { using type = iter_reference_t<_Tp>; }; + { using type = __iter_ref_t<_Tp>; }; + // Use iter_move(E) if that works. template 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 requires (!__adl_imove<_Tp>) - && is_lvalue_reference_v> + && is_lvalue_reference_v<__iter_ref_t<_Tp>> struct __result<_Tp> - { using type = remove_reference_t>&&; }; + { using type = remove_reference_t<__iter_ref_t<_Tp>>&&; }; template static constexpr bool @@ -142,10 +152,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION public: // The result type of iter_move(std::declval<_Tp>()) - template + template using __type = typename __result<_Tp>::type; - template + template + requires __adl_imove<_Tp> || requires { typename __iter_ref_t<_Tp>; } [[nodiscard]] constexpr __type<_Tp> operator()(_Tp&& __e) const @@ -153,10 +164,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { if constexpr (__adl_imove<_Tp>) return iter_move(static_cast<_Tp&&>(__e)); - else if constexpr (is_lvalue_reference_v>) - 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> 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 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 concept __adl_iswap = (std::__detail::__class_or_enum> diff --git a/libstdc++-v3/testsuite/24_iterators/customization_points/iter_move.cc b/libstdc++-v3/testsuite/24_iterators/customization_points/iter_move.cc index c5def5ac5776..341bd5b98d7e 100644 --- a/libstdc++-v3/testsuite/24_iterators/customization_points/iter_move.cc +++ b/libstdc++-v3/testsuite/24_iterators/customization_points/iter_move.cc @@ -63,8 +63,103 @@ test01() VERIFY( test_X(3, 4) ); } +template +using rval_ref = std::iter_rvalue_reference_t; + +static_assert(std::same_as, int&&>); +static_assert(std::same_as, const int&&>); +static_assert(std::same_as>, int&&>); + +template +concept iter_movable = requires { std::ranges::iter_move(std::declval()); }; + +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); + static_assert(std::same_as); + // When ADL iter_move is not used, return type should be an rvalue: + static_assert(std::same_as); + static_assert(std::same_as); + + // std::iter_rvalue_reference_t always considers the argument as lvalue. + static_assert(std::same_as, int&>); + static_assert(std::same_as, int&>); + static_assert(std::same_as, const short&&>); + static_assert(std::same_as, 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 ); + static_assert( iter_movable ); + static_assert( ! iter_movable ); + static_assert( std::same_as, int&&> ); + static_assert( std::same_as, int> ); + + struct I2 + { + int i{}; + int& operator*() & { return i; } + int operator*() const & { return i; } + void operator*() &&; + }; + + static_assert( iter_movable ); + static_assert( iter_movable ); + static_assert( iter_movable ); + static_assert( std::is_void_v ); + static_assert( std::same_as, int&&> ); + static_assert( std::same_as, int> ); + + enum E { e }; + enum F { f }; + + struct I3 + { + E operator*() const & { return e; } + F operator*() && { return f; } + }; + + static_assert( iter_movable ); + static_assert( iter_movable ); + static_assert( std::same_as ); +} + int main() { test01(); + test_adl(); }