[libcxx-commits] [PATCH] D106507: [libcxx][ranges] Add ranges::take_view.

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added inline comments.


================
Comment at: libcxx/include/__iterator/counted_iterator.h:58
 template<class>
 struct __counted_iterator_value_type {};
 
----------------
Review note: this file and the one below (common_view.h) have/will be updated with the changes shown here in other patches. 


================
Comment at: libcxx/include/__ranges/take_view.h:53
+    _LIBCPP_HIDE_FROM_ABI
+    constexpr auto begin() requires (!__simple_view<_View>) {
+      if constexpr (sized_range<_View>) {
----------------
Chris, before you ask, I did try to factor this out into a static member function, but ran into a couple of issues (first, I had to inline `size`, second, it didn't work well for move only views). 

😉 


================
Comment at: libcxx/include/__ranges/take_view.h:135
+    _LIBCPP_HIDE_FROM_ABI
+    constexpr explicit __sentinel(sentinel_t<_Base> __end) : __end_(__end) {} // TODO: why not move this one too?
+
----------------
We move `__end` in the other constructor, why not here too? We don't use it later. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106507