WIP: [libc++][ranges] Implement ranges::slide_view
#67146
Conversation
llvmbot
added
the
libc++
Continue implementation and add first smoke tests.
You can test this locally with the following command:git-clang-format --diff 5b8204b2215f293f69dded5be2774bec7bb1fde0 9a617a98330ea5b8f4992041cbeea393a8039678 -- libcxx/include/__ranges/slide_view.h libcxx/test/std/ranges/range.adaptors/range.slide.view/adaptor.pass.cppView the diff from clang-format here.diff --git a/libcxx/test/std/ranges/range.adaptors/range.slide.view/adaptor.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.slide.view/adaptor.pass.cpp
index 6d075d25f027..a4fb9784a6d8 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.slide.view/adaptor.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.slide.view/adaptor.pass.cpp
@@ -10,7 +10,6 @@
// std::views::slide
-
#include "test_iterators.h"
#include <cassert>
#include <concepts>
|
Add slide_view to the module map.
Make clang-format happy on blank line in test.
Implement slide_view::__sentinel.
Fix typo in test.
|
This fails to compile the following example: |
There was a problem hiding this comment.
This pull request appears to have been in a draft state for a while, and you might be blocked on stride_view (#65200). I left these comments to potentially unblock you on this pull rqeuests, as both the stride_view and slide_view features proposed for C++23 are useful features. In addition, it seems that slide_view::iterator lacks a few operators.
Take your time! No need to rush.
Reference: Draft C++ Standard: [range.slide]
| _LIBCPP_HIDE_FROM_ABI constexpr auto begin() | ||
| requires(!(__simple_view<_View> && __slide_caches_nothing<const _View>) && __slide_caches_last<_View>) | ||
| { | ||
| auto __first = ranges::begin(__base_); | ||
| if (!__cached_begin_.__has_value()) { | ||
| __cached_begin_.__emplace(ranges::next(__first, __n_ - 1, ranges::end(__base_))); | ||
| } | ||
| return __iterator<false>(__first, __cached_begin_, __n_); | ||
| } | ||
|
|
||
| _LIBCPP_HIDE_FROM_ABI constexpr auto begin() | ||
| requires(!(__simple_view<_View> && __slide_caches_nothing<const _View>)) | ||
| { | ||
| return __iterator<false>(ranges::begin(__base_), __n_); | ||
| } |
There was a problem hiding this comment.
I think these methods can be merged with if constexpr.
| _LIBCPP_HIDE_FROM_ABI constexpr _View base() && { return std::move(__base_); } | ||
|
|
||
| _LIBCPP_HIDE_FROM_ABI constexpr auto begin() | ||
| requires(!(__simple_view<_View> && __slide_caches_nothing<const _View>) && __slide_caches_last<_View>) |
There was a problem hiding this comment.
I think the constraint should be __slide_caches_first<_View> instead of __slide_caches_last<_View>.
|
|
||
| _LIBCPP_HIDE_FROM_ABI friend constexpr bool operator==(const __iterator& __x, const __iterator& __y) { | ||
| if constexpr (__slide_caches_first<_Base>) { | ||
| __x.__last_ == __y.__last_; |
There was a problem hiding this comment.
| __x.__last_ == __y.__last_; | |
| return __x.__last_ == __y.__last_; |
| using _Cache = _If<!(__slide_caches_nothing<_View>), __non_propagating_cache<iterator_t<_View>>, __empty_cache>; | ||
| _Cache __cached_begin_; | ||
| _Cache __cached_end_; |
There was a problem hiding this comment.
I think these caches can't exist at the same time, so I think we can make either of them an __empty_cache.
Thank you for the feedback! Yes, I am blocked on stride_view so I will double down and get back to work on this! Thank you, again, for the feedback!! Will |
No description provided.