[libc++] Use std::ranges::swap_ranges in swap CPO

[jloser]

Summary

As the comment notes, now that we have implemented std::ranges::swap_ranges,
we can apply it when swapping C-style arrays in the swap CPO when there is
support for ranges.

[ldionne]

libcxx/include/__concepts/swappable.h

82–85

I think this is what you want?

[philnik]

libcxx/include/__concepts/swappable.h

82–85

[jloser]

Actually use std::ranges::swap_ranges instead of std::swap_ranges.

libcxx/include/__concepts/swappable.h

82–85

Oops, yes. Fixed - thanks!

[• Quuxplusone]

LGTM if CI is happy. I skimmed the existing tests for ranges::swap and it looks like arrays are covered decently well; but have you also looked at the tests? Do we need any more coverage for this?

[jloser]

In D119445#3311448, @Quuxplusone wrote:

LGTM if CI is happy. I skimmed the existing tests for ranges::swap and it looks like arrays are covered decently well; but have you also looked at the tests? Do we need any more coverage for this?

Looks like CI isn't happy as this introduces a header cycle as shown by https://buildkite.com/llvm-project/libcxx-ci/builds/8635#9ef9a18c-716c-4962-9f54-27108fa79d51. We'll need to fix that.

As for the tests, I briefly looked at the array tests and they seemed OK - nothing immediately came to mind that I'd like to add (at least as a result of us using ranges::swap_ranges for swappable arrays).

[ldionne]

This LGTM, however I would like to see this again after rebasing on top of D118736. D118736 is a must-have, and I think both are going to be in conflict, so we might have to postpone doing this. Or we could do something like:

#if defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
  // use for loop
#else
  // use ranges::swap_ranges
#endif

Once we stop using _LIBCPP_HAS_NO_INCOMPLETE_RANGES, we can refactor this into only the #else branch.

[jloser]

In D119445#3320632, @ldionne wrote:

This LGTM, however I would like to see this again after rebasing on top of D118736. D118736 is a must-have, and I think both are going to be in conflict, so we might have to postpone doing this. Or we could do something like:

#if defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
  // use for loop
#else
  // use ranges::swap_ranges
#endif

Once we stop using _LIBCPP_HAS_NO_INCOMPLETE_RANGES, we can refactor this into only the #else branch.

The #ifdef dance you proposed seems reasonable, and then can clean it up once we stop using _LIBCPP_HAS_NO_INCOMPLETE_RANGES. I haven't had a chance yet to look into fixing the header cycle issue this exposes when including #include <__algorithm/ranges_swap_ranges.h> from <__concepts/swappable.h>. I'll rebase and fix the cycle sometime this week.

[jloser]

Rebase and add #ifdef.

There isn't a "straightforward" way to fix the header cycle since __concepts/movable.h depends on __concepts/swappable.h

libcxx/include/__concepts/swappable.h

 //===----------------------------------------------------------------------===//
 //
 // 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___CONCEPTS_SWAPPABLE_H
 #define _LIBCPP___CONCEPTS_SWAPPABLE_H
 
+#include <__algorithm/ranges_swap_ranges.h>
 #include <__concepts/assignable.h>
 #include <__concepts/class_or_enum.h>
 #include <__concepts/common_reference_with.h>
 #include <__concepts/constructible.h>
 #include <__config>
 #include <__utility/exchange.h>
 #include <__utility/forward.h>
 #include <__utility/move.h>
struct __fn {
     }
 
     // 2.2   Otherwise, if `E1` and `E2` are lvalues of array types with equal extent and...
     template<class _Tp, class _Up, size_t _Size>
       requires __swappable_arrays<_Tp, _Up, _Size>
     constexpr void operator()(_Tp(& __t)[_Size], _Up(& __u)[_Size]) const
       noexcept(noexcept((*this)(*__t, *__u)))
     {
-      // TODO(cjdb): replace with `ranges::swap_ranges`.
+#if defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
       for (size_t __i = 0; __i < _Size; ++__i) {
         (*this)(__t[__i], __u[__i]);
       }
+#else
+      ranges::swap_ranges(__t, __u);
+#endif
     }
 
     // 2.3   Otherwise, if `E1` and `E2` are lvalues of the same type `T` that models...
     template<__exchangeable _Tp>
     constexpr void operator()(_Tp& __x, _Tp& __y) const
       noexcept(is_nothrow_move_constructible_v<_Tp> && is_nothrow_move_assignable_v<_Tp>)
     {
       __y = _VSTD::exchange(__x, _VSTD::move(__y));