[libcxx-commits] [PATCH] D102037: [libcxx][views] Add drop_view.

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added inline comments.


================
Comment at: libcxx/include/__ranges/drop_view.h:37
+
+    static constexpr bool _UseCache = forward_range<_View> && !random_access_range<_View>;
+    using _Cache = optional<iterator_t<_View>>;
----------------
tcanens wrote:
> zoecarver wrote:
> > tcanens wrote:
> > > The cache needs to be used for random access ranges too unless the range is also sized.
> > Why does the range being sized matter? Doesn't a random access range provide amortized constant-time complexity for element lookup (which is the requirement for `drop_view`)?
> You need the size of the underlying view to figure out how much to drop in constant time.
> 
> If you don't know that, then you have to advance the iterator one step at a time until you either reach the end of the range or N steps, and that's linear.
Aha, that makes sense. Thanks for explaining! :)


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.drop/begin.pass.cpp:13
+
+// class std::ranges::drop_view;
+
----------------
ldionne wrote:
> Those should describe what's being tested in the test. We've always done that in libc++, and frankly it's quite useful to know exactly what's being tested (especially since we love to have subtly different overloads of functions in the stdlib).
Good point. Fixing. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102037