-
Notifications
You must be signed in to change notification settings - Fork 115
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
[oneDPL][ranges] A fix for ranges::transform algo - it requires all input and output ranges to be sized. #2096
base: main
Are you sure you want to change the base?
Conversation
ef4027d
to
7498faf
Compare
6bd798a
to
32189b5
Compare
…anges to be sized (according to p3179r4)
32189b5
to
b9607c9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me leaving aside a suggestion for std_ranges_transform.pass
.
Let me leave a link to the paper and a relevant section for other reviewers and my future self: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p3179r4.html#require_bounded_ranges
In the case of two input ranges or sequences, for a few algorithms - namely, mismatch, equal, and transform - it could be sufficient for just one range to be bounded and the other assumed to have at least as many elements as the bounded one.
...
However, SG9 decided to require sized_range for all inputs, with the plan to relax these constraints for transform once there is a way to statically detect infinite ranges like views::repeat (as opposed to finite unsized ranges, such as null terminated strings).
c5965f7
to
44c17de
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
This PR changes public API, though.
I would vote for merging it as is, because:
- It makes the algorithm compliant with the recent developments in C++ standard.
- This algorithm is new (it's been in oneDPL for 2 releases), which lowers the risk.
However, lets wait for the opinion of others, as we discussed separately.
It seems like we also need a fix for the oneDPL specification as well: As for how to deal with any deprecation, or removal since it has already been released both in the spec and the code, it warrants some discussion. |
A fix according to p3179r4.
It requires all input and output ranges to be sized for ranges::transform algo.