[libc++][ranges] Implement P1899 `ranges::stride_view`. #65200

hawkinsw · 2023-09-02T06:55:25Z

Implement ranges::stride_view in libc++. Migrating to this GitHub from Phabricator (https://reviews.llvm.org/D156924).

Closes #105198

hawkinsw · 2023-09-02T23:47:21Z

Thank you all for the feedback on this work. I am really excited to get to participate. As I said, I have committed small amounts of code in the past but this is my first attempt at doing a significant amount of implementation. I have already learned a ton and look forward to learning more!

hawkinsw · 2023-09-04T14:21:51Z

I just used a fixup! commit to add some additional code. I followed the new best practices and I hope that I did it correctly. Please let me know if I did not follow the instructions properly!

hawkinsw · 2023-09-07T00:30:07Z

I know how busy you all must be with the transition to GitHub. I am going to continue to work on writing tests and look forward to any feedback you have!

hawkinsw · 2023-09-11T21:52:11Z

Removed the WIP tag (and added an amend fixup commit to accomplish the same thing with in the git realm). I can't wait to hear how I can improve the implementation!!

Will

EricWF · 2023-09-06T21:06:03Z

libcxx/include/__ranges/stride_view.h

+  using difference_type = range_difference_t<_Base>;
+  using value_type      = range_value_t<_Base>;
+  using iterator_concept =
+      _If<random_access_range<_Base>,


None of this needs the cost of lazy SFINAE. and that's a lot of instantiations. Can we reduce it some?

@EricWF Sorry to have to ask ... I know what each of the words "lazy" and "SFINAE" means but in combination I am not entirely sure to what you are referring. Could you point me to something that I use to educate myself so that I can fix this antipattern? Sorry again to have to ask!

@EricWF I now realized what you meant and believe that the fixup I am about to submit will resolve this! Thanks again for the feedback!

I won't be so presumptuous as to Resolve this conversation, but let me know if what I just changed seems better.

libcxx/include/__ranges/stride_view.h

JMazurkiewicz

Quick review of product code. Also, please rebase.

I'm not maintainer, but I've already implemented `views::stride` in microsoft/STL/pull/2981, so my comments might be useful :)

JMazurkiewicz · 2023-09-06T20:53:29Z

libcxx/modules/std/ranges.inc

+    // [range.stride], stride view
+    using std::ranges::stride_view;
+
+    namespace views {
+      using std::ranges::views::stride;
+    }
+


std module is available in C++20 as an extensions, we should add language guards:

Suggested change

// [range.stride], stride view

using std::ranges::stride_view;

namespace views {

using std::ranges::views::stride;

}

#if _LIBCPP_STD_VER >= 23

// [range.stride], stride view

using std::ranges::stride_view;

namespace views {

using std::ranges::views::stride;

}

#endif // _LIBCPP_STD_VER >= 23

I just want to make sure that I understand:

In C++20 there is an extension that would enable the use of this module. std::ranges::stride_view is only available as of C++23. So, in the case where this module was used as an extension in a C++20-compatible compiler, without this guard there would be a problem.

I hope that I am understanding correctly! Great catch!

In C++20 there is an extension that would enable the use of this module.

Yes, it is documented here: https://libcxx.llvm.org/Modules.html

std::ranges::stride_view is only available as of C++23. So, in the case where this module was used as an extension in a C++20-compatible compiler, without this guard there would be a problem.

Yes, I've got bitten by it too while implementing views::chunk_by.

Thank you! I appreciate you pointing this out and I am glad that I understood your point. I believe that I have addressed the comment. If so, would you mind if I marked the conversation resolved?

libcxx/include/module.modulemap.in

libcxx/include/__ranges/stride_view.h

hawkinsw · 2023-09-13T21:41:42Z

@JMazurkiewicz Thank you for your incredibly helpful comments. I have a few things to do at $dayjob but I will address them as soon as possible. Thank you again for taking the time to comment.

hawkinsw · 2023-09-22T19:18:37Z

Hello again and sorry to bother. I would love to make any changes to the implementation that you think are necessary. I have really enjoyed the opportunity to implement stride_view and I have already started on slide_view!

Zingam · 2023-09-29T05:25:06Z

Hello again and sorry to bother. I would love to make any changes to the implementation that you think are necessary. I have really enjoyed the opportunity to implement stride_view and I have already started on slide_view!

Please update the status page and any papers: libcxx/docs/Status/RangesViews.csv
Also this PR needs a lot more tests.

hawkinsw · 2023-09-29T06:15:58Z

Hello again and sorry to bother. I would love to make any changes to the implementation that you think are necessary. I have really enjoyed the opportunity to implement stride_view and I have already started on slide_view!

Please update the status page and any papers: libcxx/docs/Status/RangesViews.csv Also this PR needs a lot more tests.

Thank you for the feedback! Are you asking for an update to remove you as an implementer? Refer to the this URL for the PR? I believe that the paper listed in the CSV is the right one? I am sorry if I am missing something obvious.

Zingam · 2023-09-30T14:18:39Z

Thank you for the feedback! Are you asking for an update to remove you as an implementer? Refer to the this URL for the PR? I believe that the paper listed in the CSV is the right one? I am sorry if I am missing something obvious.

Please change the URL to point to this PR, also mark which papers this PR implements as complete. You probably need to add the paper to the release notes, when this PR is approved.

var-const

Sorry about the long delay! I only went through the stride_view.h header and haven't looked at the tests closely yet, but I wanted to give you the feedback I have so far. Thanks a lot for all the work on the patch!

libcxx/include/__ranges/stride_view.h

var-const · 2023-10-03T02:54:59Z

libcxx/include/__ranges/stride_view.h

+
+public:
+  _LIBCPP_HIDE_FROM_ABI constexpr explicit stride_view(_View __base, range_difference_t<_View> __stride)
+      : __base_(std::move(__base)), __stride_(__stride) {}


Can we test with a move-only __base?

I believe that I addressed this in a93fef8

libcxx/include/__ranges/stride_view.h

var-const · 2023-10-03T03:41:52Z

libcxx/include/__ranges/stride_view.h

+struct __fn {
+  template <viewable_range _Range>
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Range&& __range, range_difference_t<_Range> __n) const
+      noexcept(noexcept(stride_view{std::forward<_Range>(__range), __n}))


This would be slightly easier to read if the three repetitions of the expression were vertically aligned. I think the best way to achieve that is using a /* */ comment like this -- see e.g. as_rvalue_view.h for an example.

Clever use of the block comment! I believe that this is addressed in accfb24af355238748e22653b46ed3c08db2208d.

libcxx/include/__ranges/stride_view.h

github-actions · 2023-10-06T12:58:19Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff HEAD~1 HEAD --extensions h,,cpp,inc -- libcxx/include/__ranges/stride_view.h libcxx/test/libcxx/ranges/range.adaptors/range.stride.view/ctor.assert.pass.cpp libcxx/test/libcxx/ranges/range.adaptors/range.stride.view/iterator/base.nodiscard.verify.cpp libcxx/test/libcxx/ranges/range.adaptors/range.stride.view/iterator/dereference.nodiscard.verify.cpp libcxx/test/libcxx/ranges/range.adaptors/range.stride.view/iterator/dereference.pass.cpp libcxx/test/libcxx/ranges/range.adaptors/range.stride.view/iterator/increment.pass.cpp libcxx/test/libcxx/ranges/range.adaptors/range.stride.view/iterator/iter_move.nodiscard.verify.cpp libcxx/test/libcxx/ranges/range.adaptors/range.stride.view/iterator/operator.nodiscard.verify.cpp libcxx/test/libcxx/ranges/range.adaptors/range.stride.view/iterator/operator_plus_equal.pass.cpp libcxx/test/libcxx/ranges/range.adaptors/range.stride.view/iterator/subscript.nodiscard.verify.cpp libcxx/test/libcxx/ranges/range.adaptors/range.stride.view/nodiscard.verify.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/adaptor.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/base.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/begin.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/borrowing.compile.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/concept.verify.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/ctad.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/ctor.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/end.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/iterator/base.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/iterator/ctor.copy.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/iterator/ctor.default.verify.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/iterator/decrement.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/iterator/equal.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/iterator/greater_than.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/iterator/increment.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/iterator/iter_move.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/iterator/iter_swap.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/iterator/less_than.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/iterator/operator.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/iterator/plus.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/iterator/plus_equal.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/size.verify.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/stride.pass.cpp libcxx/test/std/ranges/range.adaptors/range.stride.view/types.h libcxx/include/ranges libcxx/include/version libcxx/modules/std/ranges.inc libcxx/test/std/language.support/support.limits/support.limits.general/ranges.version.compile.pass.cpp libcxx/test/std/language.support/support.limits/support.limits.general/version.version.compile.pass.cpp

View the diff from clang-format here.

diff --git a/libcxx/test/std/ranges/range.adaptors/range.stride.view/begin.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.stride.view/begin.pass.cpp
index 71d2325c8..c032e1e94 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.stride.view/begin.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.stride.view/begin.pass.cpp
@@ -46,12 +46,12 @@ static_assert(HasOnlyConstBegin<std::ranges::stride_view<SimpleCommonConstView>>
 static_assert(HasConstAndNonConstBegin<std::ranges::stride_view<UnSimpleConstView>>);
 
 constexpr bool test() {
-  const auto unsimple_const_view    = UnSimpleConstView();
-  const auto sv_unsimple_const = std::ranges::stride_view(unsimple_const_view, 1);
+  const auto unsimple_const_view = UnSimpleConstView();
+  const auto sv_unsimple_const   = std::ranges::stride_view(unsimple_const_view, 1);
   static_assert(std::same_as<decltype(*sv_unsimple_const.begin()), double&>);
 
-  auto simple_const_view    = SimpleCommonConstView();
-  auto sv_simple_const = std::ranges::stride_view(simple_const_view, 1);
+  auto simple_const_view = SimpleCommonConstView();
+  auto sv_simple_const   = std::ranges::stride_view(simple_const_view, 1);
   static_assert(std::same_as<decltype(*sv_simple_const.begin()), int&>);
 
   return true;

Clang format changes.

Add another operator (+) test.

Fix verifier error triggered by bad auto format.

Explicitly import vector where needed (for modules).

Improve base nodiscard test (and fix CI).

Make format happy.

Remove libcxx/include/libcxx.imp from git.

Update status and release documentation.

Use constexpr if for operator- overload.

Classify assertions.

Cleanup formatting.

Simplify ctor tests.

Consolidate class nodiscard verify tests.

Remove constraint on MoveOnlyView.

Fix formatting.

Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

Update includes. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

Mark internal typedef-like name as _LIBCPP_NODEBUG. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

Make clang-format happy. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

Base nodiscard test cleanup. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

base test cleanups. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

Make clang-format happy. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

Update begin tests. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

Finish renaming UnsimpleConstView to UnSimpleConstView. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

Final cleanup of begin tests. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

Begin cleanup of end tests. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

Cleanup unneeded includes in types.h. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

hawkinsw requested a review from a team as a code owner September 2, 2023 06:55

cor3ntin added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. ranges Issues related to `<ranges>` labels Sep 2, 2023

var-const self-assigned this Sep 2, 2023

hawkinsw changed the title ~~WIP: [libc++][ranges] Implement ranges::stride_view.~~ [libc++][ranges] Implement ranges::stride_view. Sep 11, 2023

EricWF reviewed Sep 12, 2023

View reviewed changes

hawkinsw requested a review from EricWF September 13, 2023 15:46

JMazurkiewicz reviewed Sep 13, 2023

View reviewed changes

hawkinsw requested a review from JMazurkiewicz September 15, 2023 18:32

hawkinsw force-pushed the stride_view branch from f06de96 to a3279a3 Compare September 15, 2023 22:23

hawkinsw requested a review from a team as a code owner September 15, 2023 22:23

hawkinsw self-assigned this Sep 27, 2023

var-const requested changes Oct 3, 2023

View reviewed changes

var-const removed their assignment Oct 3, 2023

hawkinsw force-pushed the stride_view branch from 4e5b784 to 606879f Compare October 6, 2023 12:47

hawkinsw force-pushed the stride_view branch from edb49ee to afb9798 Compare October 13, 2023 18:14

hawkinsw force-pushed the stride_view branch 4 times, most recently from f33b89b to 861dbbb Compare October 31, 2023 22:13

hawkinsw added 18 commits May 28, 2025 08:14

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

00ef029

Clang format changes.

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

9423244

Clang format changes.

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

f9ca4de

Add another operator (+) test.

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

2693216

Fix verifier error triggered by bad auto format.

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

ef9a0f7

Explicitly import vector where needed (for modules).

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

e9cafbe

Improve base nodiscard test (and fix CI).

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

3f67838

Make format happy.

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

932dcc4

Remove libcxx/include/libcxx.imp from git.

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

969a918

Update status and release documentation.

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

17b4528

Use constexpr if for operator- overload.

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

f58c054

Classify assertions.

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

efa675f

Cleanup formatting.

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

ac2af2b

Simplify ctor tests.

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

2dd15f0

Consolidate class nodiscard verify tests.

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

f4c144d

Consolidate class nodiscard verify tests.

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

f178464

Remove constraint on MoveOnlyView.

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

48e83b4

Fix formatting.

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

b0b0f40

Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

hawkinsw force-pushed the stride_view branch from 6f763c3 to b0b0f40 Compare May 30, 2025 12:31

hawkinsw added 11 commits May 30, 2025 08:41

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

7b5fd4b

Update includes. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

0ca6686

Mark internal typedef-like name as _LIBCPP_NODEBUG. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

ba32eb3

Make clang-format happy. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

404430d

Base nodiscard test cleanup. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

2b185c5

base test cleanups. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

62a38a9

Make clang-format happy. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

8dbf313

Update begin tests. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

983a5db

Finish renaming UnsimpleConstView to UnSimpleConstView. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

635b6f3

Final cleanup of begin tests. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

94b89c4

Begin cleanup of end tests. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

fixup! WIP: [libc++][ranges] Implement ranges::stride_view.

f315c84

Cleanup unneeded includes in types.h. Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

[libc++][ranges] Implement P1899 ranges::stride_view. #65200

Are you sure you want to change the base?

[libc++][ranges] Implement P1899 ranges::stride_view. #65200

Conversation

hawkinsw commented Sep 2, 2023 • edited by ldionne Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hawkinsw commented Sep 2, 2023

Uh oh!

hawkinsw commented Sep 4, 2023

Uh oh!

hawkinsw commented Sep 7, 2023

Uh oh!

hawkinsw commented Sep 11, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JMazurkiewicz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hawkinsw commented Sep 13, 2023

Uh oh!

hawkinsw commented Sep 22, 2023

Uh oh!

Zingam commented Sep 29, 2023

Uh oh!

hawkinsw commented Sep 29, 2023

Uh oh!

Zingam commented Sep 30, 2023

Uh oh!

var-const left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Oct 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

[libc++][ranges] Implement P1899 `ranges::stride_view`. #65200

[libc++][ranges] Implement P1899 `ranges::stride_view`. #65200

hawkinsw commented Sep 2, 2023 •

edited by ldionne

Loading

JMazurkiewicz left a comment •

edited

Loading

github-actions bot commented Oct 6, 2023 •

edited

Loading