[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:106
+
+      auto __tmp = ranges::next(ranges::begin(__base_), __count_, ranges::end(__base_));
+      if constexpr (_UseCache)
----------------
tcanens wrote:
> zoecarver wrote:
> > tcanens wrote:
> > > For the random-access-and-sized case, `begin` should not call `ranges::next` - that can be linear for sized-but-not-sized-sentinel ranges. It should just return `ranges::begin(__base_) + std::min(__count_, ranges::distance(__base_))`. Ditto below.
> > I think it's OK to use next because we can ensure that next will always use something equivalent to `ranges::begin(__base_) + std::min(__count_, ranges::distance(__base_))` (even if it's technically permitted to do something slower by the standard). I'd rather not add a special case here. 
> > I think it's OK to use next because we can ensure that next will always use something equivalent to `ranges::begin(__base_) + std::min(__count_, ranges::distance(__base_))` (even if it's technically permitted to do something slower by the standard). 
> 
> The point is that you can't. For a sized-but-not-sized-sentinel range, the size information is only known from the range itself. Once you decompose the range into an iterator and a sentinel for `ranges::next`, that information is gone.
> 
> 
I see what you're getting at. So we'd need to change the condition to be `sized_sentinel_for` (not `sized_range`) so that it matches what `ranges::advance` does (or, better yet, do what you suggested and take advantage of the sized range inside of this function). 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102037