Skip to content

P2281R1 Clarifying Range Adaptor Objects#2197

Merged
StephanTLavavej merged 4 commits into
microsoft:mainfrom
CaseyCarter:p2281
Sep 25, 2021
Merged

P2281R1 Clarifying Range Adaptor Objects#2197
StephanTLavavej merged 4 commits into
microsoft:mainfrom
CaseyCarter:p2281

Conversation

@CaseyCarter
Copy link
Copy Markdown
Contributor

@CaseyCarter CaseyCarter commented Sep 10, 2021

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
StephanTLavavej
StephanTLavavej reviewed Sep 11, 2021
View reviewed changes
Comment thread tests/std/tests/P0475R1_P0591R4_uses_allocator_construction/test.cpp
@miscco
Copy link
Copy Markdown
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

@CaseyCarter
Implement P2281
9e89f5a 
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
@StephanTLavavej StephanTLavavej self-assigned this Sep 15, 2021
StephanTLavavej
StephanTLavavej requested changes Sep 17, 2021
View reviewed changes
Comment thread stl/inc/ranges Outdated
Comment thread stl/inc/ranges Outdated
Comment thread stl/inc/ranges Outdated
Comment thread stl/inc/ranges Outdated
Comment thread tests/std/tests/P0896R4_ranges_range_machinery/test.cpp Outdated
Comment thread tests/std/tests/P0896R4_ranges_range_machinery/test.cpp Outdated
Comment thread tests/std/tests/P0896R4_ranges_range_machinery/test.cpp Outdated
Comment thread tests/std/tests/P0896R4_ranges_range_machinery/test.cpp Outdated
@StephanTLavavej StephanTLavavej removed their assignment Sep 17, 2021
@CaseyCarter
Copy link
Copy Markdown
Contributor Author

CaseyCarter commented Sep 20, 2021 •
edited
Loading

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?

@StephanTLavavej StephanTLavavej self-assigned this Sep 21, 2021
StephanTLavavej
StephanTLavavej reviewed Sep 22, 2021
View reviewed changes
Comment thread stl/inc/ranges Outdated
Comment thread stl/inc/ranges Outdated
@StephanTLavavej
Copy link
Copy Markdown
Member

StephanTLavavej commented Sep 23, 2021

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

@StephanTLavavej
Copy link
Copy Markdown
Member

StephanTLavavej commented Sep 23, 2021

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
Copy Markdown
Member

StephanTLavavej commented Sep 25, 2021

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

Reviewers

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

+1 more reviewer

@barcharcraz barcharcraz barcharcraz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@StephanTLavavej StephanTLavavej

Labels

cxx20 C++20 feature defect report Applied retroactively ranges C++20/23 ranges

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

P2281R1 Clarifying Range Adaptor Objects

4 participants

@CaseyCarter @miscco @StephanTLavavej @barcharcraz