[libcxx][ranges] Add `ranges::iter_swap`.

[• Quuxplusone]

libcxx/include/__iterator/iter_swap.h

10–11

s/SWAP/ITER_SWAP/g

14–15

alphabetize plz

45–53

Do we have any sense of whether the Standard intends this unqualified iter_move to refer to std::ranges::iter_move (which is what it currently refers to in this PR, IIUC) or to an ADL iter_move (which is what would make more sense IMHO IIUC)?
If it is intended to refer to std::ranges::iter_move, then please (for the love of god) replace iter_move with ranges::iter_move throughout.
If it is intended to be an ADL iter_move, then... yuck, but let's figure something out.

61

Here and line 80, I suggest removing the cast to (void). IIUC, the reason it's there in https://eel.is/c++draft/iterator.cust.swap#4.1 is to make the "expression-equivalent-to" part work out to the correct type.
However, it does occur to me that a cast to (void) is technically not a no-op, because it affects the well-formedness of the expression when iter_swap has been marked [[nodiscard]] (and a user's ADL iter_swap might be marked [[nodiscard]] for all we know).
I'd be interested in @tcanens' take on whether the (void) cast is indispensable.

76–77

This should say indirectly_movable_storable<_T1&&, _T2&&>, right? because the type of E1 is not _T1 but rather _T1&&?
Can you find a test case that demonstrates the difference, and add it as a regression test?
This comment probably applies equally to all your uses of exposition-only helpers such as __readable_swappable, but I'm even less confident that that difference will be detectable by a test.

81

Here, __iter_exchange_move needs to be ADL-proofed — but actually, I strongly suggest that you just inline its body right here (and inline its noexcept-specification on line 78). Yes I know the Standard factors it out; but it's used only once and the factoring-out doesn't seem to do anything but make the code longer (both in the Standard and in libc++).

libcxx/test/std/iterators/iterator.requirements/iterator.cust/iterator.cust.swap.pass.cpp

25

Has this been done in other tests already? If I'd seen it, I'd say why are you adding the &.
I would make IterSwapT an alias for decltype(std::ranges::iter_swap), and then I would check both std::invocable_v<IterSwapT, blah> and a few std::invocable_v<IterSwapT&, blah> and std::invocable_v<IterSwapT&&, blah>. (Remember reference-collapsing means that with your current definition, all three of IterSwapT{,&,&&} are equivalent to IterSwapT&, and that strikes me as less-useful-than-it-could-be.)

31

nit: explicit plz

33

https://quuxplusone.github.io/blog/2021/04/03/static-constexpr-whittling-knife/#friendness-to-befriend-an-entity :)

38–42

Here and line 54: I'm worried that your implementation of this function is unnecessary and/or indistinguishable from the default ranges::swap, which means you're not actually testing that it's called. How about replacing these implementations with

friend constexpr void iter_swap(HasIterSwap& a, HasIterSwap& b) {
    called1 = true;
}
friend constexpr void iter_swap(HasIterSwap& a, bool& b) {
    called2 = true;
}

and then writing tests that assert(called1 && !called2) (or whatever you expect to happen).
For constexpr-friendliness, you'll probably need to make your constructor explicit HasIterSwap(bool& c1, bool& c2) : called1(c1), called2(c2) {}. Slack me if this doesn't make sense.

85

Yes — ranges::iter_swap(++i, ++j) is supposed to increment i only once, not twice.
But this is physically obvious. The Standard needs to mention it because the Standard is pretending that these are expressions-not-function-calls. We don't have to test it; you can remove this TODO.

156

It seems like this is testing the order in which std::swap performs its moves — is that right? If so, I don't think it belongs in libcxx/test/std/ at all. Ditto throughout.

libcxx/test/std/iterators/iterator.requirements/iterator.cust/unqualified_lookup_wrapper.h

75

int moves() const { return moves_; } :)

[zoecarver]

libcxx/include/__iterator/iter_swap.h

61

I'm slightly against removing the void cast, basically to make sure a case like ensureVoidCast works below.

76–77

This is my bad for naming these _T1 and _T2 so when I read the standard wording, my brain thought it meant to just use the template params (which don't exist in the wording). I think you're right.

libcxx/test/std/iterators/iterator.requirements/iterator.cust/iterator.cust.swap.pass.cpp

156

Hmm, I see what you're saying with L155 specifically. And we probably don't really need this test (or maybe a better test would be arr[0].moves() + arr[1].moves() == 3.

Ditto throughout.

Where else do you see this?

[tcanens]

libcxx/include/__iterator/iter_swap.h

45–53

This is definitely ranges::iter_move.

61

nodiscard has zero normative effect, so it can't affect the well-formedness of anything.

76–77

The type of an expression is never a reference type.

79

This is missing the assignment,

[• Quuxplusone]

libcxx/include/__iterator/iter_swap.h

76–77

The type of an expression is never a reference type.

So all of these trait checks should be using e.g. indirectly_movable_storable<remove_reference_t<_T1>, remove_reference_t<_T2>>? Blech. OK.

libcxx/test/std/iterators/iterator.requirements/iterator.cust/iterator.cust.swap.pass.cpp

156

At least lines 159, 162, 165, 168, 171, 174, 177.

[cjdb]

Some initial feedback, will give the tests a more in-depth review in a day or so.

libcxx/include/__iterator/iter_swap.h

40–43

56–57

61

I would prefer it if ranges::iter_swap didn't randomly cause people to want to enable -Wno-unused-result. Let's keep the cast to void.

64–66

I'll leave the last overload as an exercise.

[zoecarver]

libcxx/include/__iterator/iter_swap.h

61

I would prefer it if ranges::iter_swap didn't randomly cause people to want to enable -Wno-unused-result. Let's keep the cast to void.

+1 I'm going to leave it. (I don't see any way it hurts to have it, but I can see ways it helps.)

64–66

When I've done this in other places, I've been asked to change it to use requires, and in this case (and especially below) I think it might be clearer than trying to fit it into the template.

76–77

So all of these trait checks should be using e.g. indirectly_movable_storable<remove_reference_t<_T1>, remove_reference_t<_T2>>? Blech. OK.

I think they can actually just be _T1 and _T2 (no remove_reference_t), right?

79

You're right. I thought __iter_exchange_move would take care of this, but it only checks one of the assignment operators, not the other.

libcxx/test/std/iterators/iterator.requirements/iterator.cust/iterator.cust.swap.pass.cpp

156

At least lines 159, 162, 165, 168, 171, 174, 177.

These are just checking that the values of the array buff are swapped. I think this is exactly what we want to be checking.

[• Quuxplusone]

libcxx/include/__iterator/iter_swap.h

76–77

@tcanens said "the type of an expression is never a reference type," so no, you'd have to remove the reference first (e.g. if _T1 is int&, then we should apply the trait to int, not to int&).

[zoecarver]

libcxx/include/__iterator/iter_swap.h

76–77

I'm not sure I have the correct words to describe what I'm thinking here but won't the && in _T1&& "eat" the reference part of the reference type?

[zoecarver]

libcxx/include/__iterator/iter_swap.h

40–43

In other PRs I've been asked to put the !__unqualified_iter_swap part in the requires clause. I also like this better because then the concept is more composable (rather than only being useful in one place).

56–57

This is a bit more confusing imho. If you feel strongly I'll change it, though.

64–66

I'm going to leave this and mark it as resolved.

76–77

I'm not sure I have the correct words to describe what I'm thinking here but won't the && in _T1&& "eat" the reference part of the reference type?

@Quuxplusone ping, does this make any sense?

81

That's not a bad idea, I have to add a move for __old, but I think that's alright.

libcxx/test/std/iterators/iterator.requirements/iterator.cust/iterator.cust.swap.pass.cpp

38–42

Updated to have something similar to this. Let me know what you think.

For the record: this isn't really meant to test that we use the correct overload, though. I was just testing that it calls "the overload" it didn't really cross my mind that there would be any way for us to even call the bool& overload with an argument of type HasIterSwap. Anyway, I think it is a good idea to not have this just behave like an assignment operator or default swap impl.

85

Interesting, OK, thanks for explaining.

[zoecarver]

Update based on review.

[• Quuxplusone]

I'm unlikely to have any further comments on this, but also don't think I should count as "accepting" it, since I've mainly done superficial stuff and @tcanens had some pretty significant standardese issues (which have probably been dealt with now but I'm not claiming I'm sure of that). I'll let someone-else be the final approver.

libcxx/include/__iterator/iter_swap.h

75

Just *_VSTD::forward<_T1>(__x) = _VSTD::move(__old); — no need to cast to void here.

76–77

No; reference collapsing means that when T is int&, T&& is int& (not int&&).
However, at least for iter_value_t, the concept itself will strip cvref-qualifiers:
https://godbolt.org/z/aEEoen53P
and then for indirectly_movable_storable, I think it'd be difficult (and presumably library-UB, even if it were possible) to come up with a type where indirectly_movable_storable<T> was true and indirectly_movable_storable<T&> was false, or vice versa.
So I suppose no immediate action is needed here; let's just see if anyone files a bug.

libcxx/test/std/iterators/iterator.requirements/iterator.cust/iterator.cust.swap.pass.cpp

143

This is the only place you test std::ranges::iter_swap(lvalue, lvalue). Please expand the tests below to test them also with lvalues of type HasRangesSwapWrapper, A*, etc., at least enough to hit each different overload of iter_swap once with at least one lvalue.

You never test std::ranges::iter_swap(lvalue, rvalue) or std::ranges::iter_swap(rvalue, lvalue), but I don't think those need testing, as long as you hit the (lvalue, lvalue) cases.

libcxx/test/std/iterators/iterator.requirements/iterator.cust/unqualified_lookup_wrapper.h

70–71

move_tracker(const move_tracker&) = delete;
move_tracker& operator=(const move_tracker&) = delete;

(Deleted functions don't need constexpr.)

75

moves() still doesn't need noexcept.

[zoecarver]

@ldionne this is ready for review.

@cjdb this is ready for re-review.

libcxx/test/std/iterators/iterator.requirements/iterator.cust/iterator.cust.swap.pass.cpp

143

OK. Added a few more lvalue tests to cover the remaining two overloads.

libcxx/test/std/iterators/iterator.requirements/iterator.cust/unqualified_lookup_wrapper.h

75

I think it makes sense to leave the constexpr in this case. This is a sort of general utility.

[zoecarver]

Apply Arthur's suggested changes.

Rebase.

[ldionne]

libcxx/include/__iterator/iter_swap.h

43

I think it would be more in-line with what we usually do to say swappable_with<iter_reference_t<_T1>, iter_reference_t<_T2>> instead. WDYT?

As written, I think this is slightly incorrect (but I'm not sure it's possible to notice the difference). The Standard talks about the "reference type of E1 and E2", which is *declval<_T1&&>(), not *declval<_T1>(). But regardless, I think we can just switch to iter_reference_t<_T1> unless there's a problem with that I'm not seeing.

56–57

Agreed, I would definitely keep as-is.

69–71

I don't see how to do better, but it's awful to have to duplicate the body like that.

[zoecarver]

libcxx/include/__iterator/iter_swap.h

43

Hmm, trying to come up with an example where this doesn't work. I think it's actually 100% valid because indirectly_readable enforces that *in == iter_reference_t:

{ *in } -> std::same_as<std::iter_reference_t<In>>;

So, I'll do that.

69–71

I agree. We should really have a noexcept(auto). This wouldn't be too hard to add to clang, and then we could just let GCC users have worse codegen...

[zoecarver]

Rebase. Apply Louis' comments.

[ldionne]

LGTM with my comments applied.

libcxx/include/__iterator/iter_swap.h

69–71

If only that's how things worked 😅

libcxx/test/std/iterators/iterator.requirements/iterator.cust/iterator.cust.swap.pass.cpp

26

Based on http://eel.is/c++draft/customization.point.object#2, I would just do std::semiregular<std::remove_cv_t<IterSwapT>>.

138

Minor thing: I like to split test different test cases by enclosing them in a different scope. It makes it easier to see that they are different test cases and that they don't share anything between themselves. So I'd do:

{
    int value1 = 0;
    int value2 = 0;
    HasIterSwap a(value1), b(value2);
    std::ranges::iter_swap(a, b);
    assert(value1 == 1 && value2 == 1);
}
{
    int value1 = 0;
    int value2 = 0;
    HasRangesSwap c(value1), d(value2);
    HasRangesSwapWrapper cWrapper(c), dWrapper(d);
    std::ranges::iter_swap(cWrapper, dWrapper);
    assert(value1 == 1 && value2 == 1);
}

// etc...

[ldionne]

(and CI passing once you rebase)

[cjdb]

I'll properly review this tonight, but I don't suspect I'll have much more to add now.

libcxx/include/__iterator/iter_swap.h

56–57

FYI: we're going to be doing this in a lot of algorithms, and it's definitely part of the design of concepts.

[zoecarver]

Fix based on Louis' review comments.

[zoecarver]

Rebase off main.

This now includes two commits. That's intentional so the CI passes (for some reaosn it was failing to apply the patch before).

[zoecarver]

Rebase.

[zoecarver]

Add a modulemap entry.

[zoecarver]

Remove shadowing for GCC.

libcxx/include/CMakeLists.txt

set(files
   __functional_base
   __functional_base_03
   __hash_table
   __iterator/advance.h
   __iterator/concepts.h
   __iterator/default_sentinel.h
   __iterator/incrementable_traits.h
   __iterator/iter_move.h
+  __iterator/iter_swap.h
   __iterator/iterator_traits.h
   __iterator/next.h
   __iterator/prev.h
   __iterator/projected.h
   __iterator/readable_traits.h
   __libcpp_version
   __locale
   __memory/addressof.h

libcxx/include/__iterator/iter_swap.h

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+#ifndef _LIBCPP___ITERATOR_ITER_SWAP_H
+#define _LIBCPP___ITERATOR_ITER_SWAP_H
+
+#include <__config>
+#include <__iterator/concepts.h>
+#include <__iterator/iterator_traits.h>
+#include <__ranges/access.h>
+#include <type_traits>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+_LIBCPP_PUSH_MACROS
+#include <__undef_macros>
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#if !defined(_LIBCPP_HAS_NO_RANGES)
+
+namespace ranges {
+namespace __iter_swap {
+  template<class _I1, class _I2>
+  void iter_swap(_I1, _I2) = delete;
+
+  template<class _T1, class _T2>
+  concept __unqualified_iter_swap = requires(_T1&& __x, _T2&& __y) {
+    iter_swap(_VSTD::forward<_T1>(__x), _VSTD::forward<_T2>(__y));
+  };
+
+  template<class _T1, class _T2>
+  concept __readable_swappable =
+    indirectly_readable<_T1> && indirectly_readable<_T2> &&
+    swappable_with<iter_reference_t<_T1>, iter_reference_t<_T2>>;
+
+  struct __fn {
+    template <class _T1, class _T2>
+      requires __unqualified_iter_swap<_T1, _T2>
+    constexpr void operator()(_T1&& __x, _T2&& __y) const
+      noexcept(noexcept(iter_swap(_VSTD::forward<_T1>(__x), _VSTD::forward<_T2>(__y))))
+    {
+      (void)iter_swap(_VSTD::forward<_T1>(__x), _VSTD::forward<_T2>(__y));
+    }
+
+    template <class _T1, class _T2>
+      requires (!__unqualified_iter_swap<_T1, _T2>) &&
+               __readable_swappable<_T1, _T2>
+    constexpr void operator()(_T1&& __x, _T2&& __y) const
+      noexcept(noexcept(ranges::swap(*_VSTD::forward<_T1>(__x), *_VSTD::forward<_T2>(__y))))
+    {
+      ranges::swap(*_VSTD::forward<_T1>(__x), *_VSTD::forward<_T2>(__y));
+    }
+
+    template <class _T1, class _T2>
+      requires (!__unqualified_iter_swap<_T1, _T2> &&
+                !__readable_swappable<_T1, _T2>) &&
+               indirectly_movable_storable<_T1, _T2> &&
+               indirectly_movable_storable<_T2, _T1>
+    constexpr void operator()(_T1&& __x, _T2&& __y) const
+      noexcept(noexcept(iter_value_t<_T2>(ranges::iter_move(__y))) &&
+               noexcept(*__y = ranges::iter_move(__x)) &&
+               noexcept(*_VSTD::forward<_T1>(__x) = declval<iter_value_t<_T2>>()))
+    {
+      iter_value_t<_T2> __old(ranges::iter_move(__y));
+      *__y = ranges::iter_move(__x);
+      *_VSTD::forward<_T1>(__x) = _VSTD::move(__old);
+    }
+  };
+} // end namespace __iter_swap
+
+inline namespace __cpo {
+  inline constexpr auto iter_swap = __iter_swap::__fn{};
+} // namespace __cpo
+
+} // namespace ranges
+
+#endif // !defined(_LIBCPP_HAS_NO_RANGES)
+
+_LIBCPP_END_NAMESPACE_STD
+
+_LIBCPP_POP_MACROS
+
+#endif // _LIBCPP___ITERATOR_ITER_SWAP_H

libcxx/include/iterator

 #include <__config>
 #include <__debug>
 #include <__functional_base>
 #include <__iterator/advance.h>
 #include <__iterator/concepts.h>
 #include <__iterator/default_sentinel.h>
 #include <__iterator/incrementable_traits.h>
 #include <__iterator/iter_move.h>
+#include <__iterator/iter_swap.h>
 #include <__iterator/iterator_traits.h>
 #include <__iterator/next.h>
 #include <__iterator/prev.h>
 #include <__iterator/projected.h>
 #include <__iterator/readable_traits.h>
 #include <__memory/addressof.h>
 #include <__memory/pointer_traits.h>
 #include <compare>

libcxx/test/std/iterators/iterator.requirements/iterator.cust/iterator.cust.move/iter_move.pass.cpp

public:
   constexpr void operator++(int) noexcept { ++base_; }
 
   [[nodiscard]] constexpr bool operator==(iterator_wrapper const& other) const noexcept { return base_ == other.base_; }
 
 private:
   I base_ = I{};
 };
 
-class move_tracker {
-public:
-  move_tracker() = default;
-
-  constexpr move_tracker(move_tracker&& other) noexcept : moves_{other.moves_ + 1} { other.moves_ = 0; }
-
-  constexpr move_tracker& operator=(move_tracker&& other) noexcept {
-    moves_ = other.moves_ + 1;
-    other.moves_ = 0;
-    return *this;
-  }
-
-  constexpr move_tracker(move_tracker const& other) = delete;
-  constexpr move_tracker& operator=(move_tracker const& other) = delete;
-
-  [[nodiscard]] constexpr int moves() const noexcept { return moves_; }
-
-private:
-  int moves_ = 0;
-};
-
 template <typename I>
 constexpr void unqualified_lookup_move(I first_, I last_, I result_first_, I result_last_) {
   auto first = ::check_unqualified_lookup::unqualified_lookup_wrapper{std::move(first_)};
   auto last = ::check_unqualified_lookup::unqualified_lookup_wrapper{std::move(last_)};
   auto result_first = ::check_unqualified_lookup::unqualified_lookup_wrapper{std::move(result_first_)};
   auto result_last = ::check_unqualified_lookup::unqualified_lookup_wrapper{std::move(result_last_)};
 
   static_assert(!noexcept(std::ranges::iter_move(first)), "unqualified-lookup case not being chosen");

libcxx/test/std/iterators/iterator.requirements/iterator.cust/iterator.cust.swap.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: libcpp-no-concepts
+// UNSUPPORTED: gcc-10
+
+// template<class I>
+// unspecified iter_swap;
+
+#include <iterator>
+
+#include <array>
+#include <cassert>
+
+#include "./unqualified_lookup_wrapper.h"
+#include "test_iterators.h"
+
+using IterSwapT = decltype(std::ranges::iter_swap);
+
+static_assert(std::semiregular<std::remove_cvref_t<IterSwapT&>>);
+
+struct HasIterSwap {
+  int &value;
+  explicit HasIterSwap(int &value) : value(value) { assert(value == 0); }
+
+  friend constexpr void iter_swap(HasIterSwap& a, HasIterSwap& b) {
+    a.value = 1;
+    b.value = 1;
+  }
+  friend constexpr void iter_swap(HasIterSwap& a, int& b) {
+    a.value = 2;
+    b = 2;
+  }
+};
+
+static_assert( std::is_invocable_v<IterSwapT, HasIterSwap&, HasIterSwap&>);
+static_assert( std::is_invocable_v<IterSwapT, HasIterSwap&, int&>);
+static_assert(!std::is_invocable_v<IterSwapT, int&, HasIterSwap&>);
+
+static_assert( std::is_invocable_v<IterSwapT&, HasIterSwap&, HasIterSwap&>);
+static_assert( std::is_invocable_v<IterSwapT&, HasIterSwap&, int&>);
+static_assert(!std::is_invocable_v<IterSwapT&, int&, HasIterSwap&>);
+
+static_assert( std::is_invocable_v<IterSwapT&&, HasIterSwap&, HasIterSwap&>);
+static_assert( std::is_invocable_v<IterSwapT&&, HasIterSwap&, int&>);
+static_assert(!std::is_invocable_v<IterSwapT&&, int&, HasIterSwap&>);
+
+struct NodiscardIterSwap {
+  [[nodiscard]] friend int iter_swap(NodiscardIterSwap&, NodiscardIterSwap&) { return 0; }
+};
+
+void ensureVoidCast(NodiscardIterSwap& a, NodiscardIterSwap& b) { std::ranges::iter_swap(a, b); }
+
+struct HasRangesSwap {
+  int &value;
+  explicit HasRangesSwap(int &value) : value(value) { assert(value == 0); }
+
+  friend constexpr void swap(HasRangesSwap& a, HasRangesSwap& b) {
+    a.value = 1;
+    b.value = 1;
+  }
+  friend constexpr void swap(HasRangesSwap& a, int& b) {
+    a.value = 2;
+    b = 2;
+  }
+};
+
+struct HasRangesSwapWrapper {
+  using value_type = HasRangesSwap;
+
+  HasRangesSwap &value;
+  explicit HasRangesSwapWrapper(HasRangesSwap &value) : value(value) {}
+
+  HasRangesSwap& operator*() const { return value; }
+};
+
+static_assert( std::is_invocable_v<IterSwapT, HasRangesSwapWrapper&, HasRangesSwapWrapper&>);
+// Does not satisfy swappable_with, even though swap(X, Y) is valid.
+static_assert(!std::is_invocable_v<IterSwapT, HasRangesSwapWrapper&, int&>);
+static_assert(!std::is_invocable_v<IterSwapT, int&, HasRangesSwapWrapper&>);
+
+struct B;
+
+struct A {
+  bool value = false;
+  A& operator=(const B&) {
+    value = true;
+    return *this;
+  };
+};
+
+struct B {
+  bool value = false;
+  B& operator=(const A&) {
+    value = true;
+    return *this;
+  };
+};
+
+struct MoveOnly2;
+
+struct MoveOnly1 {
+  bool value = false;
+
+  MoveOnly1() = default;
+  MoveOnly1(MoveOnly1&&) = default;
+  MoveOnly1& operator=(MoveOnly1&&) = default;
+  MoveOnly1(const MoveOnly1&) = delete;
+  MoveOnly1& operator=(const MoveOnly1&) = delete;
+
+  MoveOnly1& operator=(MoveOnly2 &&) {
+    value = true;
+    return *this;
+  };
+};
+
+struct MoveOnly2 {
+  bool value = false;
+
+  MoveOnly2() = default;
+  MoveOnly2(MoveOnly2&&) = default;
+  MoveOnly2& operator=(MoveOnly2&&) = default;
+  MoveOnly2(const MoveOnly2&) = delete;
+  MoveOnly2& operator=(const MoveOnly2&) = delete;
+
+  MoveOnly2& operator=(MoveOnly1 &&) {
+    value = true;
+    return *this;
+  };
+};
+
+int main(int, char**) {
+  int value1 = 0;
+  int value2 = 0;
+  HasIterSwap a(value1), b(value2);
+  std::ranges::iter_swap(a, b);
+  assert(value1 == 1 && value2 == 1);
+
+  value1 = 0;
+  value2 = 0;
+  HasRangesSwap c(value1), d(value2);
+  HasRangesSwapWrapper cWrapper(c), dWrapper(d);
+  std::ranges::iter_swap(cWrapper, dWrapper);
+  assert(value1 == 1 && value2 == 1);
+
+  value1 = 0;
+  value2 = 0;
+  std::ranges::iter_swap(HasRangesSwapWrapper(c), HasRangesSwapWrapper(d));
+  assert(value1 == 1 && value2 == 1);
+
+  A e; B f;
+  A *ePtr = &e;
+  B *fPtr = &f;
+  std::ranges::iter_swap(ePtr, fPtr);
+  assert(e.value && f.value);
+
+  MoveOnly1 g; MoveOnly2 h;
+  std::ranges::iter_swap(&g, &h);
+  assert(g.value && h.value);
+
+  auto arr = std::array<move_tracker, 2>();
+  std::ranges::iter_swap(arr.begin(), arr.begin() + 1);
+  assert(arr[0].moves() == 1 && arr[1].moves() == 2);
+
+  int buff[2] = {1, 2};
+  std::ranges::iter_swap(buff + 0, buff + 1);
+  assert(buff[0] == 2 && buff[1] == 1);
+
+  std::ranges::iter_swap(cpp20_input_iterator(buff), cpp20_input_iterator(buff + 1));
+  assert(buff[0] == 1 && buff[1] == 2);
+
+  std::ranges::iter_swap(cpp17_input_iterator(buff), cpp17_input_iterator(buff + 1));
+  assert(buff[0] == 2 && buff[1] == 1);
+
+  std::ranges::iter_swap(forward_iterator(buff), forward_iterator(buff + 1));
+  assert(buff[0] == 1 && buff[1] == 2);
+
+  std::ranges::iter_swap(bidirectional_iterator(buff), bidirectional_iterator(buff + 1));
+  assert(buff[0] == 2 && buff[1] == 1);
+
+  std::ranges::iter_swap(random_access_iterator(buff), random_access_iterator(buff + 1));
+  assert(buff[0] == 1 && buff[1] == 2);
+
+  std::ranges::iter_swap(contiguous_iterator(buff), contiguous_iterator(buff + 1));
+  assert(buff[0] == 2 && buff[1] == 1);
+
+  return 0;
+}

libcxx/test/std/iterators/iterator.requirements/iterator.cust/unqualified_lookup_wrapper.h

 union some_union {
   int x;
   double y;
 };
 constexpr int iter_move(some_union& u) noexcept(false) { return u.x; }
 
 } // namespace check_unqualified_lookup
 
+class move_tracker {
+public:
+  move_tracker() = default;
+  constexpr move_tracker(move_tracker&& other) noexcept : moves_{other.moves_ + 1} { other.moves_ = 0; }
+  constexpr move_tracker& operator=(move_tracker&& other) noexcept {
+    moves_ = other.moves_ + 1;
+    other.moves_ = 0;
+    return *this;
+  }
+
+  move_tracker(move_tracker const& other) = delete;
+  move_tracker& operator=(move_tracker const& other) = delete;
+
+  constexpr int moves() const noexcept { return moves_; }
+
+private:
+  int moves_ = 0;
+};
+
 #endif // LIBCPP_TEST_STD_ITERATOR_UNQUALIFIED_LOOKUP_WRAPPER

libcxx/test/std/iterators/predef.iterators/move.iterators/move.iterator/iterator_concept_conformance.compile.pass.cpp

 
 
 static_assert(std::input_iterator<iterator>);
 static_assert(!std::forward_iterator<iterator>);
 static_assert(!std::indirectly_writable<iterator, int>);
 static_assert(std::incrementable<iterator>);
 static_assert(std::sentinel_for<iterator, iterator>);
 static_assert(std::sized_sentinel_for<iterator, iterator>);
-static_assert(std::indirectly_movable<int*, iterator>);
-static_assert(std::indirectly_movable_storable<int*, iterator>);
+static_assert(!std::indirectly_movable<int*, iterator>);
+static_assert(!std::indirectly_movable_storable<int*, iterator>);

libcxx/test/std/iterators/stream.iterators/istream.iterator/iterator_concept_conformance.compile.pass.cpp

 using iterator = std::istream_iterator<int, std::istream>;
 static_assert(std::indirectly_readable<iterator>);
 static_assert(!std::indirectly_writable<iterator, int>);
 static_assert(std::incrementable<iterator>);
 static_assert(std::input_or_output_iterator<iterator>);
 static_assert(std::sentinel_for<iterator, iterator>);
 static_assert(!std::sized_sentinel_for<iterator, iterator>);
 static_assert(std::input_iterator<iterator>);
-static_assert(std::indirectly_movable<int*, iterator>);
-static_assert(std::indirectly_movable_storable<int*, iterator>);
+static_assert(!std::indirectly_movable<int*, iterator>);
+static_assert(!std::indirectly_movable_storable<int*, iterator>);