Skip to content
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

<ranges>: Temporarily disable join_view for non-forward_ranges, pending resolution of LWG-3698 #2727

Merged
merged 4 commits into from
May 24, 2022

Conversation

cpplearner
Copy link
Contributor

As pointed out in LWG-3698, join_view is currently broken for "stashing iterators" (iterators that return a reference to a member object). The issue mentions that join_view would need to store the outer iterator in a non-propagating-cache for input ranges.

The fix might change the layout of join_view (and of its iterator type, if we don't want to store a redundant outer iterator). So I believe that it is useful to flag the affected uses of join_view.

This requires users to define a macro if they don't depend on the ABI of join_view and want to use join_view for input ranges. I hope it's not too much of a problem.

@cpplearner cpplearner requested a review from a team as a code owner May 17, 2022 10:45
@StephanTLavavej StephanTLavavej added the ranges C++20/23 ranges label May 18, 2022
stl/inc/ranges Outdated Show resolved Hide resolved
@cpplearner
Copy link
Contributor Author

FYI here's what I think the fix would roughly look like: https://github.com/cpplearner/STL/tree/lwg3698.

@StephanTLavavej
Copy link
Member

I'm speculatively mirroring this to the MSVC-internal repo - further changes can be pushed, but please notify me.

Copy link
Contributor

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

I'm applying a clarification to the error message to make the consequences of overriding clear.

stl/inc/ranges Outdated Show resolved Hide resolved
@CaseyCarter CaseyCarter removed their assignment May 22, 2022
stl/inc/ranges Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej merged commit 2966813 into microsoft:main May 24, 2022
@StephanTLavavej
Copy link
Member

Thanks for noticing this issue and preventing users from being bitten by it! 🐍 🦇 🧛

@cpplearner cpplearner deleted the jv branch May 28, 2022 10:30
fsb4000 pushed a commit to fsb4000/STL that referenced this pull request Aug 13, 2022
… pending resolution of LWG-3698 (microsoft#2727)

Co-authored-by: Casey Carter <[email protected]>
Co-authored-by: timsong-cpp <[email protected]>
cpplearner added a commit to cpplearner/STL that referenced this pull request Feb 14, 2023
…range`s, pending resolution of LWG-3698 (microsoft#2727)"

This reverts commit 2966813.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ranges C++20/23 ranges
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants