Skip to content

Commit 0b49bfe

Browse files
hawkinswhawkinsw
committed
fixup! WIP: [libc++][ranges] Implement ranges::stride_view.
Bring into line with updates to features like [[nodiscard]] vs _LIBCPP_NODISCARD
1 parent 34f401e commit 0b49bfe

File tree

8 files changed

+83
-62
lines changed

8 files changed

+83
-62
lines changed

‎libcxx/include/__ranges/stride_view.h‎

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -72,31 +72,29 @@ class stride_view : public view_interface<stride_view<_View>> {
7272
_LIBCPP_ASSERT_UNCATEGORIZED(__stride > 0, "The value of stride must be greater than 0");
7373
}
7474

75-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI constexpr _View base() const&
75+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _View base() const&
7676
requires copy_constructible<_View>
7777
{
7878
return __base_;
7979
}
8080

81-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI constexpr _View base() && { return std::move(__base_); }
81+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _View base() && { return std::move(__base_); }
8282

83-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI constexpr range_difference_t<_View> stride() const noexcept {
84-
return __stride_;
85-
}
83+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr range_difference_t<_View> stride() const noexcept { return __stride_; }
8684

87-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI constexpr auto begin()
85+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto begin()
8886
requires(!__simple_view<_View>)
8987
{
9088
return __iterator<false>(this, ranges::begin(__base_), 0);
9189
}
9290

93-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI constexpr auto begin() const
91+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto begin() const
9492
requires range<const _View>
9593
{
9694
return __iterator<true>(this, ranges::begin(__base_), 0);
9795
}
9896

99-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI constexpr auto end()
97+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto end()
10098
requires(!__simple_view<_View>)
10199
{
102100
if constexpr (common_range<_View> && sized_range<_View> && forward_range<_View>) {
@@ -109,7 +107,7 @@ class stride_view : public view_interface<stride_view<_View>> {
109107
}
110108
}
111109

112-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI constexpr auto end() const
110+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto end() const
113111
requires(range<const _View>)
114112
{
115113
if constexpr (common_range<const _View> && sized_range<const _View> && forward_range<const _View>) {
@@ -122,13 +120,13 @@ class stride_view : public view_interface<stride_view<_View>> {
122120
}
123121
}
124122

125-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI constexpr auto size()
123+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto size()
126124
requires sized_range<_View>
127125
{
128126
return std::__to_unsigned_like(ranges::__div_ceil(ranges::distance(__base_), __stride_));
129127
}
130128

131-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI constexpr auto size() const
129+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto size() const
132130
requires sized_range<const _View>
133131
{
134132
return std::__to_unsigned_like(ranges::__div_ceil(ranges::distance(__base_), __stride_));
@@ -201,12 +199,10 @@ class stride_view<_View>::__iterator : public __stride_iterator_category<_View>
201199
__stride_(__i.__stride_),
202200
__missing_(__i.__missing_) {}
203201

204-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI constexpr iterator_t<_Base> const& base() const& noexcept {
205-
return __current_;
206-
}
207-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI constexpr iterator_t<_Base> base() && { return std::move(__current_); }
202+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr iterator_t<_Base> const& base() const& noexcept { return __current_; }
203+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr iterator_t<_Base> base() && { return std::move(__current_); }
208204

209-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI constexpr decltype(auto) operator*() const {
205+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr decltype(auto) operator*() const {
210206
_LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__current_ != __end_, "Cannot dereference an iterator at the end.");
211207
return *__current_;
212208
}
@@ -267,7 +263,7 @@ class stride_view<_View>::__iterator : public __stride_iterator_category<_View>
267263
return *this += -__n;
268264
}
269265

270-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI constexpr decltype(auto) operator[](difference_type __n) const
266+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr decltype(auto) operator[](difference_type __n) const
271267
requires random_access_range<_Base>
272268
{
273269
return *(*this + __n);
@@ -313,41 +309,38 @@ class stride_view<_View>::__iterator : public __stride_iterator_category<_View>
313309
return __x.__current_ <=> __y.__current_;
314310
}
315311

316-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI friend constexpr __iterator
317-
operator+(__iterator const& __i, difference_type __s)
312+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI friend constexpr __iterator operator+(__iterator const& __i, difference_type __s)
318313
requires random_access_range<_Base>
319314
{
320315
auto __r = __i;
321316
__r += __s;
322317
return __r;
323318
}
324-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI friend constexpr __iterator
325-
operator+(difference_type __s, __iterator const& __i)
319+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI friend constexpr __iterator operator+(difference_type __s, __iterator const& __i)
326320
requires random_access_range<_Base>
327321
{
328322
auto __r = __i;
329323
__r += __s;
330324
return __r;
331325
}
332326

333-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI friend constexpr __iterator
334-
operator-(__iterator const& __i, difference_type __s)
327+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI friend constexpr __iterator operator-(__iterator const& __i, difference_type __s)
335328
requires random_access_range<_Base>
336329
{
337330
auto __r = __i;
338331
__r -= __s;
339332
return __r;
340333
}
341334

342-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI friend constexpr difference_type
335+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI friend constexpr difference_type
343336
operator-(__iterator const& __x, __iterator const& __y)
344337
requires sized_sentinel_for<iterator_t<_Base>, iterator_t<_Base>> && forward_range<_Base>
345338
{
346339
auto __n = __x.__current_ - __y.__current_;
347340
return (__n + __x.__missing_ - __y.__missing_) / __x.__stride_;
348341
}
349342

350-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI friend constexpr difference_type
343+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI friend constexpr difference_type
351344
operator-(__iterator const& __x, __iterator const& __y)
352345
requires sized_sentinel_for<iterator_t<_Base>, iterator_t<_Base>>
353346
{
@@ -358,20 +351,20 @@ class stride_view<_View>::__iterator : public __stride_iterator_category<_View>
358351
return ranges::__div_ceil(__n, __x.__stride_);
359352
}
360353

361-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI friend constexpr difference_type
354+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI friend constexpr difference_type
362355
operator-(default_sentinel_t, __iterator const& __x)
363356
requires sized_sentinel_for<sentinel_t<_Base>, iterator_t<_Base>>
364357
{
365358
return ranges::__div_ceil(__x.__end_ - __x.__current_, __x.__stride_);
366359
}
367-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI friend constexpr difference_type
360+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI friend constexpr difference_type
368361
operator-(__iterator const& __x, default_sentinel_t __y)
369362
requires sized_sentinel_for<sentinel_t<_Base>, iterator_t<_Base>>
370363
{
371364
return -(__y - __x);
372365
}
373366

374-
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI friend constexpr range_rvalue_reference_t<_Base>
367+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI friend constexpr range_rvalue_reference_t<_Base>
375368
iter_move(__iterator const& __it) noexcept(noexcept(ranges::iter_move(__it.__current_))) {
376369
return ranges::iter_move(__it.__current_);
377370
}

‎libcxx/test/libcxx/ranges/range.adaptors/range.stride.view/iterator/operator.nodiscard.verify.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,16 @@ constexpr bool test_forward_operator_minus() {
4747

4848
sv_zero_offset_begin + // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
4949
1;
50-
1 + sv_zero_offset_begin; // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
50+
1 + sv_zero_offset_begin; // expected-warning {{ignoring return value of function declared with 'nodiscard'
51+
// attribute}}
5152

5253
sv_one_offset_begin - 1; // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
5354

5455
sv_one_offset_begin - // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
5556
sv_zero_offset_begin;
5657

57-
std::default_sentinel_t() - // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
58+
std::default_sentinel_t() - // expected-warning {{ignoring return value of function declared with 'nodiscard'
59+
// attribute}}
5860
sv_zero_offset_begin;
5961

6062
sv_zero_offset_begin - // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}

‎libcxx/test/std/ranges/range.adaptors/range.stride.view/adaptor.pass.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,9 @@ constexpr bool test() {
134134
}
135135

136136
// A final sanity check.
137-
{ static_assert(std::same_as<decltype(std::views::stride), decltype(std::ranges::views::stride)>); }
137+
{
138+
static_assert(std::same_as<decltype(std::views::stride), decltype(std::ranges::views::stride)>);
139+
}
138140

139141
return true;
140142
}

‎libcxx/test/std/ranges/range.adaptors/range.stride.view/ctad.pass.cpp‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,17 @@ constexpr bool testCTAD() {
3131
using BaseRange = BasicTestRange<cpp17_input_iterator<int*>>;
3232
using BaseView = BasicTestView<int*>;
3333

34-
auto base_view = BaseView(a, a + 5);
35-
auto base_range = BaseRange(cpp17_input_iterator<int*>(a), cpp17_input_iterator<int*>(a + 5));
34+
auto base_view = BaseView(a, a + 5);
35+
auto base_view_for_move = BaseView(a, a + 5);
36+
37+
auto base_range = BaseRange(cpp17_input_iterator<int*>(a), cpp17_input_iterator<int*>(a + 5));
38+
auto base_range_for_move = BaseRange(cpp17_input_iterator<int*>(a), cpp17_input_iterator<int*>(a + 5));
3639

3740
auto copied_stride_base_view = std::ranges::stride_view(base_view, 2);
38-
auto moved_stride_base_view = std::ranges::stride_view(std::move(base_view), 2);
41+
auto moved_stride_base_view = std::ranges::stride_view(std::move(base_view_for_move), 2);
3942

4043
auto copied_stride_base_range = std::ranges::stride_view(base_range, 2);
41-
auto moved_stride_base_range = std::ranges::stride_view(std::move(base_range), 2);
44+
auto moved_stride_base_range = std::ranges::stride_view(std::move(base_range_for_move), 2);
4245

4346
static_assert(std::same_as< decltype(copied_stride_base_view), std::ranges::stride_view<BaseView>>);
4447
static_assert(std::same_as< decltype(moved_stride_base_view), std::ranges::stride_view<BaseView>>);

‎libcxx/test/std/ranges/range.adaptors/range.stride.view/iterator/base.pass.cpp‎

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ constexpr bool base_const() {
4747
return true;
4848
}
4949

50-
bool base_move() {
50+
constexpr bool base_move() {
5151
// Keep track of how many times the original iterator is moved
5252
// and/or copied during the test.
5353
int move_counter = 0;
@@ -59,23 +59,35 @@ bool base_move() {
5959
auto stop = SizedInputIter();
6060

6161
auto view = BasicTestView<SizedInputIter>{start, stop};
62-
auto sv = std::ranges::stride_view<BasicTestView<SizedInputIter>>(view, 1);
63-
auto svi = sv.begin();
62+
assert(move_counter == 0);
63+
// One copies of _start_ occurs when it is copied to the basic test view's member variable.
64+
assert(copy_counter == 1);
65+
66+
auto sv = std::ranges::stride_view<BasicTestView<SizedInputIter>>(view, 1);
67+
// There is a copy of _view_ made when it is passed by value.
68+
// There is a move done of _view_ when it is used as the initial value of __base.
69+
assert(move_counter == 1);
70+
assert(copy_counter == 2);
71+
72+
auto svi = sv.begin();
73+
// Another copy of _start_ when begin uses the iterator to the first element
74+
// of the view underlying sv as the by-value parameter to the stride view iterator's
75+
// constructor.
76+
assert(copy_counter == 3);
77+
// Another move of __start_ happens right after that when it is std::move'd to
78+
// become the first __current of the iterator.
79+
assert(move_counter == 2);
6480

65-
// Reset the move/copy counters so that they reflect *only* whether the
66-
// base() member function moved or copied the iterator.
67-
move_counter = 0;
68-
copy_counter = 0;
6981
[[maybe_unused]] auto result = std::move(svi).base();
70-
71-
// Ensure that base std::move'd the iterator.
72-
assert(*result.move_counter == 1);
73-
assert(*result.copy_counter == 0);
82+
// Ensure that base std::move'd the iterator and did not copy it.
83+
assert(move_counter == 3);
84+
assert(copy_counter == 3);
7485
return true;
7586
}
7687

77-
bool base_copy() {
78-
// See above.
88+
constexpr bool base_copy() {
89+
// See base_move() for complete description of when/why
90+
// moves/copies take place..
7991
int move_counter = 0;
8092
int copy_counter = 0;
8193
auto start = SizedInputIter();
@@ -84,27 +96,37 @@ bool base_copy() {
8496
start.copy_counter = &copy_counter;
8597
auto stop = SizedInputIter();
8698

87-
auto view = BasicTestView<SizedInputIter>{start, stop};
88-
auto sv = std::ranges::stride_view<BasicTestView<SizedInputIter>>(view, 1);
89-
[[maybe_unused]] auto svi = sv.begin();
99+
auto view = BasicTestView<SizedInputIter>{start, stop};
100+
assert(move_counter == 0);
101+
assert(copy_counter == 1);
90102

91-
// See above.
92-
move_counter = 0;
93-
copy_counter = 0;
94-
[[maybe_unused]] const SizedInputIter& result = svi.base();
103+
auto sv = std::ranges::stride_view<BasicTestView<SizedInputIter>>(view, 1);
104+
assert(move_counter == 1);
105+
assert(copy_counter == 2);
106+
107+
[[maybe_unused]] auto svi = sv.begin();
108+
assert(copy_counter == 3);
109+
assert(move_counter == 2);
95110

111+
[[maybe_unused]] const SizedInputIter result = svi.base();
96112
// Ensure that base did _not_ std::move'd the iterator.
97-
assert(*result.move_counter == 0);
98-
assert(*result.copy_counter == 0);
113+
assert(move_counter == 2);
114+
assert(copy_counter == 4);
115+
99116
return true;
100117
}
101118

102119
int main(int, char**) {
103120
base_noexcept();
104121
static_assert(base_noexcept());
122+
105123
base_const();
106124
static_assert(base_const());
125+
107126
base_move();
127+
static_assert(base_move());
128+
108129
base_copy();
130+
static_assert(base_copy());
109131
return 0;
110132
}

‎libcxx/test/std/ranges/range.adaptors/range.stride.view/iterator/increment.pass.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,7 @@ int main(int, char**) {
196196

197197
{
198198
int arr[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
199-
test_non_forward_operator_increment(
200-
SizedInputIter(arr), SizedInputIter(arr + 3), SizedInputIter(arr + 10));
199+
test_non_forward_operator_increment(SizedInputIter(arr), SizedInputIter(arr + 3), SizedInputIter(arr + 10));
201200
}
202201

203202
test_properly_handling_missing();

‎libcxx/test/std/ranges/range.adaptors/range.stride.view/iterator/plus_equal.pass.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,9 @@ constexpr bool test_random_access_operator_plus_equal(Iter begin, Iter end, Diff
7575
assert(*sv_bv_begin == *sv_bv_offset_begin);
7676
assert(*sv_bv_begin_after_distance == *sv_bv_offset_begin);
7777

78-
auto big_step = (end - 1) - begin;
78+
auto big_step = (end - 1) - begin;
7979
auto stride_view_over_base_view_big_step = std::ranges::stride_view(base_view, big_step);
80-
sv_bv_begin = stride_view_over_base_view_big_step.begin();
80+
sv_bv_begin = stride_view_over_base_view_big_step.begin();
8181

8282
// This += should move us into a position where the __missing_ will come into play.
8383
// Do a -= 1 here to confirm that the __missing_ is taken into account.

‎libcxx/test/std/ranges/range.adaptors/range.stride.view/types.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ struct BasicTestViewOrRange : MaybeView<IsView> {
188188
Iter begin_{};
189189
Iter end_{};
190190

191-
constexpr BasicTestViewOrRange(Iter b, Iter e) : begin_(b), end_(e) {}
191+
constexpr BasicTestViewOrRange(const Iter& b, const Iter& e) : begin_(b), end_(e) {}
192192

193193
constexpr Iter begin() { return begin_; }
194194
constexpr Iter begin() const { return begin_; }

0 commit comments

Comments
 (0)