[libcxx-commits] [PATCH] D100255: [libcxx] adds `range` access CPOs

[Tim Song via Phabricator via libcxx-commits]

tcanens added inline comments.


================
Comment at: libcxx/include/__ranges/access.h:67
+  requires __is_complete<remove_all_extents_t<_Tp> >
+  [[nodiscard]] constexpr _Tp* operator()(_Tp (&__t)[_Np]) const noexcept {
+    return __t + 0;
----------------
This needs to reject const rvalue arrays.


================
Comment at: libcxx/include/__ranges/access.h:75
+  _LIBCPP_NOEXCEPT_RETURN(
+    _VSTD::__decay_copy(_VSTD::forward<_Tp>(__t).begin())
+  )
----------------
It'd be nice to avoid an extra move when `begin` returns a prvalue (as they almost always do).


================
Comment at: libcxx/include/__ranges/access.h:120
+  [[nodiscard]] constexpr auto operator()(_Tp (&__t)[_Np]) const noexcept {
+    return __t + extent_v<_Tp[_Np]>;
+  }
----------------
Isn't `extent_v<_Tp[_Np]>` just `_Np`?


================
Comment at: libcxx/include/__ranges/access.h:155
+  requires invocable<decltype(ranges::begin), _Tp const&&>
+  [[nodiscard]] constexpr auto operator()(remove_reference_t<_Tp>&& __t) const
+  _LIBCPP_NOEXCEPT_RETURN(
----------------
This is never going to be called since `_Tp` can't be deduced.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100255