[libcxx-commits] [PATCH] D116621: [libc++][P2321R2] Add const overloads to tuple swap

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added a comment.

P2321R2 "zip" <http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2321r2.html#tuple> is a big paper, so it makes sense to chunk it up into smaller PRs like this. But I think this PR is //too// small. The notions of "const-swappable" and "const-assignable" seem very tightly coupled to each other.  Therefore I'm pretty confident in asserting that whatever PR implements const-swap for T, should also implement const-assign for T.

It might even make sense to implement all the tuple stuff and all the pair stuff together in this same PR, although I'm not at all confident of that.



================
Comment at: libcxx/test/std/utilities/tuple/tuple.tuple/tuple.special/non_member_swap_const.pass.cpp:21-25
+struct ConstSwappable {
+  mutable int i;
+};
+
+constexpr void swap(const ConstSwappable& lhs, const ConstSwappable& rhs) { std::swap(lhs.i, rhs.i); }
----------------
```
struct S {
  int *calls;
  friend constexpr void swap(S& a, S& b) {
    *calls += 1;
  }
};
struct CS {
  int *calls;
  friend constexpr void swap(const CS& a, const CS& b) {
    *calls += 1;
  }
};
```
And then test various mixtures of `S`, `CS`, and `S&`; and for the behavior tests, I wouldn't focus on the ultimate effects (like `a.i == 1`) but instead on whether they're actually calling the user's ADL `swap` function correctly:
```
static_assert( std::is_swappable_v<std::tuple<>>);
static_assert( std::is_swappable_v<std::tuple<S>>);
static_assert( std::is_swappable_v<std::tuple<CS>>);
static_assert( std::is_swappable_v<std::tuple<S&>>);
static_assert( std::is_swappable_v<std::tuple<CS, S>>);
static_assert( std::is_swappable_v<std::tuple<CS, S&>>);
static_assert( std::is_swappable_v<const std::tuple<>>);
static_assert(!std::is_swappable_v<const std::tuple<S>>);
static_assert( std::is_swappable_v<const std::tuple<CS>>);
static_assert( std::is_swappable_v<const std::tuple<S&>>);
static_assert(!std::is_swappable_v<const std::tuple<CS, S>>);
static_assert( std::is_swappable_v<const std::tuple<CS, S&>>);
{
  int cs_calls = 0;
  int s_calls = 0;
  S s1{s_calls};
  S s2{s_calls};
  const std::tuple<CS, S&> t1 = { CS{cs_calls}, s1 };
  const std::tuple<CS, S&> t2 = { CS{cs_calls}, s2 };
  swap(t1, t2);
  assert(cs_calls == 1);
  assert(s_calls == 1);
}
```
And then I guess there should be some tests for is_nothrow_swappable, too; and for a type that isn't swappable at all.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116621/new/

https://reviews.llvm.org/D116621