[libcxx-commits] [PATCH] D116199: [libc++] Fix ranges::{cbegin, cend} for rvalues.

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__ranges/access.h:160
     template <class _Tp>
-    requires is_rvalue_reference_v<_Tp> && invocable<decltype(ranges::begin), _Tp const&&>
+      requires is_rvalue_reference_v<_Tp&&> && invocable<decltype(ranges::begin), _Tp const&&>
     [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Tp&& __t) const
----------------
Nit, pre-existing: I would swap `is_rvalue_reference_v` and `invocable` to align the `invocalbe`s of this `operator()` and the above.


================
Comment at: libcxx/test/std/ranges/range.access/begin.pass.cpp:133
   assert(std::ranges::begin(std::move(c)) == &globalBuff[0]);
+  assert(std::ranges::cbegin(std::move(c)) == &globalBuff[0]);
 
----------------
Is anywhere checked that `ranges::cbegin()` actually returns a const iterator?


================
Comment at: libcxx/test/std/ranges/range.access/end.pass.cpp:254
   EndFunction aa{};
+  static_assert(!std::is_invocable_v<RangeEndT, decltype((aa))>);
   assert(std::ranges::cend(aa) == &aa.x);
----------------
What is the reason for `(())`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116199