[libcxx-commits] [PATCH] D105753: [libcxx][ranges] Add `ranges::common_view`.

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added inline comments.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.common.view/begin.pass.cpp:93-98
+constexpr auto operator-(sentinel_wrapper<RandomAccessIter> sent, RandomAccessIter iter) {
+  return sent.base().base() - iter.base();
+}
+constexpr auto operator-(RandomAccessIter iter, sentinel_wrapper<RandomAccessIter> sent) {
+  return iter.base() - sent.base().base();
+}
----------------
Quuxplusone wrote:
> I'd prefer to see these as hidden friends of `RandomAccessIter`, or know the technical reason why they can't be. Ditto lines 72-77 above.
RandomAccessIter is a type alias to `random_access_iterator`. I think we both agree we shouldn't add these as hidden friends to `random_access_iterator`.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.common.view/begin.pass.cpp:115
+    std::ranges::common_view<SizedForwardView> comm(SizedForwardView{buffer});
+    if (!std::is_constant_evaluated())
+      assert(*comm.begin() == 1);
----------------
Quuxplusone wrote:
> Why do we need this `if`? What piece is not constexpr-friendly here?
Basically none of common_iterator is constexpr. //Sigh//.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105753