[oneDPL] Fix the default template argument for the new value type in replace[_if] #658
Conversation
akukanov
changed the title
|
I think this looks like a good fix for a real issue. |
rarutyun
left a comment
rarutyun
left a comment
There was a problem hiding this comment.
The interesting part of this PR is that we don't have any tests that fail but that was something we have discussed offline already
| typename T1 = /*projected-value-type*/<std::ranges::iterator_t<R>, Proj>, | ||
| typename T2 = std::ranges::range_value_t<R>> |
There was a problem hiding this comment.
This asymmetry between old_value and new_value seems odd at first, given their names.
However, I understand that this asymmetry is necessary. It's like replacing by key. We specify a key (projection applied), old_value, and we specify a key-value pair, new_value, to replace a found entry with.
I agree that we should act proactively. But, I would anticipate more changes in the standard, e.g. different names of the arguments, which is not crucial.
There was a problem hiding this comment.
The default template argument for the type of the new value in range::replace and ranges::replace_if should not have projections applied.
Where can I find these discussions of the c++ draft?
There was a problem hiding this comment.
https://cplusplus.github.io/LWG/lwg-active.html#4444 is the official issue, opened just recently. Our changes are aligned with the proposed resolution.
4 participants
Reviewers of the C++26 working draft have found that, citing,
While this comment has not yet been resolved, it seems correct: the standard wording for these algorithms does not imply that the new value can be used as an argument to the projection or in an expression with projected values.
It seems appropriate therefore to proactively fix the respective signatures in the oneDPL specification.