LWG-3520 iter_move and iter_swap are inconsistent for transform_view::iterator (#2200)

Co-authored-by: Michael Schellenberger Costa <mschellenbergercosta@googlemail.com>
Co-authored-by: Adam Bucior <35536269+AdamBucior@users.noreply.github.com>
Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>

Author: 4 people

stl/inc/ranges

@@ -2038,15 +2038,6 @@ namespace ranges {
                     return *_It;
                 }
             }
-
-            friend constexpr void iter_swap(const _Iterator& _Left, const _Iterator& _Right) _NOEXCEPT_IDL0(noexcept(
-                _RANGES iter_swap(_Left._Current, _Right._Current))) requires indirectly_swappable<iterator_t<_Base>> {
-#if _ITERATOR_DEBUG_LEVEL != 0
-                _Left._Check_dereference();
-                _Right._Check_dereference();
-#endif // _ITERATOR_DEBUG_LEVEL != 0
-                _RANGES iter_swap(_Left._Current, _Right._Current);
-            }
         };
 
         template <bool _Const>

tests/std/include/range_algorithm_support.hpp

@@ -1449,3 +1449,8 @@ template <class R>
 concept CanBool = requires(R&& r) {
     std::forward<R>(r) ? true : false;
 };
+
+template <class I>
+concept CanIterSwap = requires(I&& i1, I&& i2) {
+    ranges::iter_swap(std::forward<I>(i1), std::forward<I>(i2));
+};

tests/std/tests/P0896R4_views_transform/test.cpp

@@ -509,16 +509,7 @@ struct iterator_instantiator {
             assert(ranges::iter_move(i0) == add8(mutable_ints[0])); // NB: moving from int leaves it unchanged
             STATIC_ASSERT(NOEXCEPT_IDL0(ranges::iter_move(i0)));
 
-            if constexpr (forward_iterator<Iter>) {
-                auto i1 = ranges::next(i0);
-                ranges::iter_swap(i0, i1);
-                assert(mutable_ints[0] == 1);
-                assert(mutable_ints[1] == 0);
-                ranges::iter_swap(i1, i0);
-                assert(mutable_ints[0] == 0);
-                assert(mutable_ints[1] == 1);
-                STATIC_ASSERT(NOEXCEPT_IDL0(ranges::iter_swap(i0, i1)));
-            }
+            STATIC_ASSERT(!CanIterSwap<decltype(i0)>);
         }
 
         { // Validate increments
@@ -784,4 +775,19 @@ int main() {
 
         (void) views::transform(Fn{})(span<int>{});
     }
+
+    { // Validate that iter_swap works when result of transformation is an lvalue reference
+        char base[] = "hello";
+        auto v      = ranges::transform_view{base, [](char& c) -> char& { return c; }};
+        auto i1     = v.begin();
+        auto i2     = v.begin() + 1;
+
+        assert(*i1 == 'h');
+        assert(*i2 == 'e');
+
+        ranges::iter_swap(i1, i2);
+
+        assert(*i1 == 'e');
+        assert(*i2 == 'h');
+    }
 }

tests/std/tests/P0896R4_views_transform_death/test.cpp

@@ -287,26 +287,6 @@ void test_iter_move_value_initialized_iterator() {
     (void) ranges::iter_move(i); // cannot dereference value-initialized transform_view iterator
 }
 
-void test_iter_swap_value_initialized_iterators() {
-    ranges::iterator_t<TV> i0{};
-    ranges::iterator_t<TV> i1{};
-    (void) ranges::iter_swap(i0, i1); // cannot dereference value-initialized transform_view iterator
-}
-
-void test_iter_swap_value_initialized_iterator_left() {
-    ranges::iterator_t<TV> i0{};
-    TV r{some_ints, lambda};
-    ranges::iterator_t<TV> i1 = r.begin();
-    (void) ranges::iter_swap(i0, i1); // cannot dereference value-initialized transform_view iterator
-}
-
-void test_iter_swap_value_initialized_iterator_right() {
-    TV r{some_ints, lambda};
-    ranges::iterator_t<TV> i0 = r.begin();
-    ranges::iterator_t<TV> i1{};
-    (void) ranges::iter_swap(i0, i1); // cannot dereference value-initialized transform_view iterator
-}
-
 void test_sentinel_compare_value_initialized() {
     auto r = ranges::subrange{counted_iterator{some_ints, ranges::distance(some_ints)}, default_sentinel}
            | views::transform(lambda);
@@ -382,9 +362,6 @@ int main(int argc, char* argv[]) {
         test_operator_minus_incompatible_different,
         test_operator_minus_incompatible_value_initialized,
         test_iter_move_value_initialized_iterator,
-        test_iter_swap_value_initialized_iterators,
-        test_iter_swap_value_initialized_iterator_left,
-        test_iter_swap_value_initialized_iterator_right,
         test_sentinel_compare_value_initialized,
         test_sentinel_difference_value_initialized,
         test_flipped_sentinel_difference_value_initialized,