[libcxx-commits] [PATCH] D102020: [libcxx][ranges] Add class ref_view.

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver marked 17 inline comments as done.
zoecarver added a comment.

Okay, I //think// that's everything.



================
Comment at: libcxx/include/__ranges/ref_view.h:63
+
+    // TODO: This needs to use contiguous_range.
+    constexpr auto data() const
----------------
Quuxplusone wrote:
> Has `contiguous_range` landed yet? If so, please DO this TODO; otherwise ok, not a blocker.
No, another thing I need to do :(


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.ref.view.pass.cpp:66
+  struct sentinel {
+    friend constexpr bool operator==(sentinel, const cpp17_input_iterator<int*> iter) { return iter.base() == globalBuff + 8; }
+    friend constexpr std::ptrdiff_t operator-(sentinel, cpp17_input_iterator<int*>) { return -8; }
----------------
Quuxplusone wrote:
> Pass-by-const-value alert! Remove `const`.
Removed, but it is at least more OK in this situation where the argument is named and used. 


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.ref.view.pass.cpp:98-105
+template<class R>
+concept EmptyInvocable = requires (std::ranges::ref_view<R> view) { view.empty(); };
+
+template<class R>
+concept SizeInvocable = requires (std::ranges::ref_view<R> view) { view.size(); };
+
+template<class R>
----------------
Quuxplusone wrote:
> Naming nit: `EmptyIsInvocable`, or `HasEmpty`; but let's avoid any potential confusion with the jargon use of `Invocable` as a noun. I'd go with `HasEmpty`, myself; it's shortest.
We're not really asking if the member exists, it always does exist. We're asking whether we can call/invoke it. Also, I don't really see how `EmptyIsInvocable` is any better than `EmptyInvocable`. (Not done.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102020