[libcxx-commits] [PATCH] D131495: [libc++] implement "pair" section of P2321R2 `zip`

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/include/utility:87
+    template <class U, class V>
+    constexpr explicit(see below) pair(const pair<U1, U2>&&);                    // since C++23
     template <class... Args1, class... Args2>
----------------
This looks incorrect -- the template arguments are `U` and `V`, but the function arguments refer to `U1` and `U2`.


================
Comment at: libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_const_copy_convert.pass.cpp:21
+#include "test_macros.h"
+#include "../../../tuple/tuple.tuple/tuple.assign/types.h"
+
----------------
It looks like these types should be moved somewhere under `test_support`.


================
Comment at: libcxx/test/std/utilities/utility/pairs/pairs.pair/ctor_pair_U_V_const_move.pass.cpp:15
+// template <class U1, class U2>
+//   constexpr explicit(see below) pair(const pair<U2, U2>&& p);
+
----------------
Nit: `s/U2/U1/`.


================
Comment at: libcxx/test/std/utilities/utility/pairs/pairs.pair/ctor_pair_U_V_const_move.pass.cpp:20
+
+#include "../../../tuple/tuple.tuple/tuple.cnstr/convert_types.h"
+#include "test_macros.h"
----------------
Similar to the other tuple include: these types should live somewhere under `test_support`.


================
Comment at: libcxx/test/std/utilities/utility/pairs/pairs.pair/ctor_pair_U_V_const_move.pass.cpp:43
+    !std::is_convertible_v<const std::pair<X, Y>&&, std::pair<ExplicitConstructibleFrom<X>, ConvertibleFrom<Y>>>);
+static_assert(!std::is_convertible_v<const std::pair<X, Y>&&,
+                                     std::pair<ExplicitConstructibleFrom<X>, ExplicitConstructibleFrom<Y>>>);
----------------
Nit: can line breaks here be more consistent?


================
Comment at: libcxx/test/std/utilities/utility/pairs/pairs.pair/ctor_pair_U_V_ref.pass.cpp:15
+// template <class U1, class U2>
+//   constexpr explicit(see below) pair(pair<U2, U2>& p);
+
----------------
Nit: `s/U2/U1/`.


================
Comment at: libcxx/test/std/utilities/utility/pairs/pairs.pair/swap_member_const.pass.cpp:25
+concept MemberSwapNoexcept =
+    requires(T t1, T t2) {
+      { t1.swap(t2) } noexcept;
----------------
Should this be `const T&`?


================
Comment at: libcxx/test/std/utilities/utility/pairs/pairs.pair/swap_member_const.pass.cpp:41
+struct ConstSwappable {
+  mutable int i;
+  friend constexpr void swap(const ConstSwappable& lhs, const ConstSwappable& rhs) { std::swap(lhs.i, rhs.i); }
----------------
Optional: instead of using `mutable,` you could store a pointer or a reference, then modify the pointed-to value. That should fix the GCC issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131495