[libcxx-commits] [PATCH] D130547: [libc++][ranges] Implement `ranges::is_heap{, _until}`.

[Hui via Phabricator via libcxx-commits]

huixie90 added inline comments.


================
Comment at: libcxx/include/__algorithm/ranges_is_heap.h:59
   bool operator()(_Range&& __range, _Comp __comp = {}, _Proj __proj = {}) const {
-    // TODO: implement
-    (void)__range; (void)__comp; (void)__proj;
-    return {};
+    return __is_heap_fn_impl(ranges::begin(__range), ranges::end(__range), __comp, __proj);
   }
----------------
ldionne wrote:
> huixie90 wrote:
> > nit:
> > for ranges that do model `sized_range` but their iter/sent pair do not model `sized_sentinel_for`, by doing this it lost its size information and possibly as a result the `ranges::next` call becomes linear whereas it could be constant.  What do you think?
> That is an interesting corner case, however that would also require the range's iterator to be a `random_access_iterator` but not a `sized_sentinel_for<It, Sent>`. That seems rather broken to me.
> 
> For example, imagine a null-terminated string where the iterators are random access (roughly `char*`), but comparing against the end sentinel checks whether `*it == '\0'`. If the range is a `sized_range`, it means that it also stores the size of the string. In a case like that, I can't imagine why one would implement the iterator and the sentinel as different types, because the size is known anyway so `end()` can be implemented as `begin() + size()` trivially.
> 
> If I'm not imaginative enough, please feel free to enlighten me, however if this is such a tiny corner case, perhaps it is not worth increasing the complexity of our implementation to handle it.
@ldionne 

```
    auto v = std::ranges::iota_view(5, 7L);
    using iter = std::ranges::iterator_t<decltype(v)>;
    using sent = std::ranges::sentinel_t<decltype(v)>;

    static_assert(std::ranges::sized_range<decltype(v)>);
    static_assert(std::random_access_iterator<iter>);
    static_assert(!std::sized_sentinel_for<sent, iter>);
```
https://godbolt.org/z/7bd91z5c4

Yes it is a corner case and yes `iota_view` is weird. so I am happy to NAD this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130547