-
Notifications
You must be signed in to change notification settings - Fork 115
RFC for zip_view implementation, for oneDPL C++20 #1931
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 13 commits
09bd199
b56c860
761ecb7
7ef15f4
e792b5b
786d5c5
171e5fe
f9a2e45
4a701fc
cc3eac4
52acfac
526a603
518aee0
234dbdb
c20ef9f
aec760b
9ab570b
300e13c
f51297e
4cade93
7b882fb
2e9c98d
214dcde
1f7b132
720f1d2
416010b
28aed7e
1ce2407
20d82e8
87a32a8
c629dff
68d67f6
14118c6
e8fcc7c
13a814f
8e140b0
cc89449
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
# zip_view Support for the oneDPL Range APIs with C++20 | ||
|
||
## Introduction | ||
`std::ranges::zip_view` is powerful utility enables developers to combine two or more ranges into a single view, | ||
where each element is represented as a tuple containing corresponding elements from each input range. | ||
|
||
## Motivations | ||
`std::ranges::zip_view` is a convenient way to combine multiple ranges into a single view, where each element of | ||
the resulting range is a tuple containing one element from each of the input ranges. This can be particularly | ||
useful for iterating over multiple collections in parallel. `std::ranges::zip_view` is introduced starting with C++23, | ||
but there are many users who work with C++20 standard yet. So, oneDPL introduces `oneapi::dpl::ranges::zip_view`, | ||
MikeDvorskiy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
which the same API and functionality as `std::ranges::zip_view`. | ||
MikeDvorskiy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
### Key Requirements | ||
akukanov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
`oneapi::dpl::ranges::zip_view` should be: | ||
- compilable with C++20 version (minimum) | ||
dmitriy-sobolev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- API compliant with `std::ranges::zip_view` | ||
MikeDvorskiy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- random accessible view; the "underlying" views also should be random accessible | ||
dmitriy-sobolev marked this conversation as resolved.
Show resolved
Hide resolved
akukanov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- in case of a device usage: a device copyable view itself and the "underlying" views also should be device copyable | ||
akukanov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
`oneapi::dpl::ranges::zip_view::iterator` should be: | ||
- value-swappable (https://en.cppreference.com/w/cpp/named_req/ValueSwappable) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps we should explicitly add |
||
- convertible to `oneapi::dpl::zip_iterator` | ||
MikeDvorskiy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- abble to use with the non-range algorithms | ||
MikeDvorskiy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
### Implementation proposal | ||
- `oneapi::dpl::ranges::zip_view` is designed as C++ class, which represents a range adaptor (see C++ Range Library). | ||
MikeDvorskiy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
This class encapsulates a tuple-like type to keep a combination of two or more ranges. | ||
- The implementation provides all necessary operators to satisfy 'random accessible view' requirement. | ||
- To provide a device copyability requirement `oneapi::dpl::__internal::tuple` is proposed as tuple-like type underhood. | ||
MikeDvorskiy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- To provide a value-swappable requirement `oneapi::dpl::__internal::tuple` is proposed as a dereferenced value for | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When it comes to the specification, this will need to be described as an unspecified tuple which satisfies some set of requirements, unless we want to make the move to specify our internal tuple and make it public (which I doubt we want to do). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the context of this, I was looking at the specification for our current There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are moving towards specifying the tuple-like type better, most likely as a type that satisfies certain concepts / requirements. That should be done in an new proposal though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes.. I've mentioned it in the RFC document update. |
||
`oneapi::dpl::ranges::zip_view::iterator`, due to the standard `std::tuple` C++20 is not swappable type. | ||
MikeDvorskiy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- Usage of C++ concepts is desirable to write type requirements for types, methods and members of the class. | ||
- C++20 is minimum supported version for the class. It allows to use modern C++ things like concepts and others. | ||
MikeDvorskiy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
### Test coverage | ||
|
||
- `oneapi::dpl::ranges::zip_view` is tested itself, base functionality. | ||
akukanov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- should be tested with at least one oneDPL range based algorithm. | ||
- should be tested with at least one oneDPL iterator based algorithm. |
Uh oh!
There was an error while loading. Please reload this page.