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

P2281R1 Clarifying Range Adaptor Objects #2197

Merged
merged 4 commits into from
Sep 25, 2021

Conversation

CaseyCarter
Copy link
Member

Which explicitly specifies that range adaptor closure objects are perfect-forwarding call wrappers, which are more fastidious about value categories of bound arguments than were our ad-hoc _Partially-applied functions.

Fixes #1983

@CaseyCarter CaseyCarter added cxx20 C++20 feature ranges C++20/23 ranges defect report Applied retroactively labels Sep 10, 2021
@CaseyCarter CaseyCarter requested a review from a team as a code owner September 10, 2021 23:27
@CaseyCarter CaseyCarter changed the title Implement P2281 P2281R1 Clarifying Range Adaptor Objects Sep 10, 2021
@miscco
Copy link
Contributor

miscco commented Sep 11, 2021

I am missing test for lazy_split_view and split_view which were the views where it does matter

Which explicitly specifies that range adaptor closure objects are perfect-forwarding call wrappers, which are more fastidious about value categories of bound arguments than were our ad-hoc `_Partial`ly-applied functions.

Fixes microsoft#1983
stl/inc/ranges Outdated Show resolved Hide resolved
stl/inc/ranges Outdated Show resolved Hide resolved
stl/inc/ranges Outdated Show resolved Hide resolved
stl/inc/ranges Outdated Show resolved Hide resolved
tests/std/tests/P0896R4_ranges_range_machinery/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P0896R4_ranges_range_machinery/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P0896R4_ranges_range_machinery/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P0896R4_ranges_range_machinery/test.cpp Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej removed their assignment Sep 17, 2021
@CaseyCarter
Copy link
Member Author

CaseyCarter commented Sep 20, 2021

I am missing test for lazy_split_view and split_view which were the views where it does matter

Obviously there are quite a few test cases for each that are still working; what additionally would you like to see tested?

EDIT: I suppose the biggest difference is that lone lvalue arguments to views::split and views::lazy_split are now captured by copy instead of reference, so changes to the original object between capture and evaluation of the range closure won't be visible in the resulting view. Would you like a test for each that validates that difference?

stl/inc/ranges Outdated Show resolved Hide resolved
stl/inc/ranges Outdated Show resolved Hide resolved
@StephanTLavavej
Copy link
Member

I'm mirroring this to an MSVC-internal PR. Please notify me if any further changes are pushed.

@StephanTLavavej
Copy link
Member

I pushed a workaround for VSO-1409853 "Standard Library Header Units: <ranges> compiler assertion: previous_element == tokenInputStack.TopOfStack(), aliastemplates.cpp 1010".

@StephanTLavavej StephanTLavavej merged commit eb70080 into microsoft:main Sep 25, 2021
@StephanTLavavej
Copy link
Member

Thanks for the clarification! 💡 🧠 😹

@CaseyCarter CaseyCarter deleted the p2281 branch September 27, 2021 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cxx20 C++20 feature defect report Applied retroactively ranges C++20/23 ranges
Projects
None yet
Development

Successfully merging this pull request may close these issues.

P2281R1 Clarifying Range Adaptor Objects
4 participants