Details
- Reviewers
jdoerfert huixie90 ldionne - Group Reviewers
Restricted Project - Commits
- rGd406c6493e9e: [libc++][ranges] Implement `ranges::is_heap{,_until}`.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
Almost LGTM thanks!
libcxx/include/__algorithm/is_heap.h | ||
---|---|---|
31 | i think perhaps not to pass the comp_ref type explicitly since the __is_heap_until already takes it by reference? | |
libcxx/include/__algorithm/ranges_is_heap.h | ||
59 | nit: | |
libcxx/test/std/algorithms/alg.sorting/alg.heap.operations/is.heap/ranges_is_heap.pass.cpp | ||
147–148 | most other tests use a different type to test projection. perhaps it is a good idea to adopt the same approach, since having different types sound more realistic use case |
libcxx/include/__algorithm/ranges_is_heap.h | ||
---|---|---|
59 | 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. |
libcxx/include/__algorithm/ranges_is_heap.h | ||
---|---|---|
59 | 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 |
clang-format not found in user’s local PATH; not linting file.