Skip to content

Conversation

@akukanov
Copy link
Contributor

Reviewers of the C++26 working draft have found that, citing,

The default template argument for the type of the new value in range::replace and ranges::replace_if should not have projections applied.

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.

@akukanov akukanov added the DPL label Oct 28, 2025
@akukanov akukanov changed the title [oneDPL] Fix the default template argument for new_value in replace[_if] [oneDPL] Fix the default template argument for the new value type in replace[_if] Oct 28, 2025
@danhoeflinger
Copy link
Contributor

I think this looks like a good fix for a real issue.

Copy link
Contributor

@rarutyun rarutyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment on lines +871 to +872
typename T1 = /*projected-value-type*/<std::ranges::iterator_t<R>, Proj>,
typename T2 = std::ranges::range_value_t<R>>
Copy link
Contributor

@dmitriy-sobolev dmitriy-sobolev Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@dmitriy-sobolev dmitriy-sobolev Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants