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

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver accepted this revision as: zoecarver.
zoecarver added a comment.

A few nits but other than that this LGTM. Let's finally get this landed (as soon as Louis signs off on it)!



================
Comment at: libcxx/test/std/ranges/range.access/range.access.begin/begin.pass.cpp:27
+
+struct Incomplete;
+
----------------
You can remove this now that incomplete arrays are tested elsewhere. 


================
Comment at: libcxx/test/std/ranges/range.access/range.access.begin/incomplete.compile.verify.cpp:24
+template <class T>
+requires(!std::invocable<begin_t&, T>)
+void f() {}
----------------
I still feel like this is a bit of an awkward way to test this.. why not call it in the body of the function?


================
Comment at: libcxx/test/std/ranges/range.access/range.access.cbegin/incomplete.compile.verify copy.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
It looks like this file is duplicated. It's named "verify copy.cpp"


================
Comment at: libcxx/test/std/ranges/range.access/range.access.end/end.cpp:22
+
+using RangeEndT = decltype(std::ranges::end)&;
+using RangeCEndT = decltype(std::ranges::cend)&;
----------------
Why are we making this a reference type?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100255