[libcxx-commits] [PATCH] D66103: implement std::to_array (P0325R3)

[Marshall Clow via Phabricator via libcxx-commits]

mclow.lists requested changes to this revision.
mclow.lists added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/array:500
+_LIBCPP_CONSTEXPR typename enable_if<
+    !is_array<_Tp>::value && is_constructible<_Tp, _Tp&>::value,
+    array<remove_cv_t<_Tp>, __n>
----------------
Why `is_constructible<_Tp, _Tp&>`? - this seems wrong.
At the very least, the second one should be `const T&`.
But even better would be `is_copy_constructible<_Tp>` - because that's what you're going to do.



================
Comment at: libcxx/include/array:510
+_LIBCPP_CONSTEXPR typename enable_if<
+    !is_array<_Tp>::value && is_move_constructible<_Tp>::value,
+    array<remove_cv_t<_Tp>, __n>
----------------
You can use the `_v` versions here; this is C++20


================
Comment at: libcxx/test/std/containers/sequences/array/array.creation/to_array.pass.cpp:20
+
+struct MoveOnly
+{
----------------
We have a nice move-only type in "MoveOnly.h".
Is there some reason why you didn't want to use that?


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D66103