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.

Sign up for GitHub

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

Jump to bottom

Add unwrapping to move_sentinel #3098

Merged
merged 4 commits into from
Sep 13, 2022
Merged

Add unwrapping to move_sentinel #3098

merged 4 commits into from
Sep 13, 2022

Conversation

strega-nil-ms
Copy link
Contributor

fixes #3009

There are two new concepts - _Weakly_unwrappable_sentinel and _Weakly_unwrappable_iterator - these are useful for implementation of wrapper types which are unwrappable when their underlying type is unwrappable (like move_iterator/move_sentinel)

@strega-nil-ms strega-nil-ms requested a review from a team as a code owner September 12, 2022 17:08
@strega-nil-ms strega-nil-ms added enhancement Something can be improved performance Must go faster labels Sep 12, 2022
@strega-nil-ms strega-nil-ms added the ranges C++20/23 ranges label Sep 12, 2022
@StephanTLavavej StephanTLavavej removed the enhancement Something can be improved label Sep 12, 2022
@StephanTLavavej
Copy link
Member

Adjusting labels - we usually use enhancement only when no more specific label like performance applies.

StephanTLavavej
StephanTLavavej reviewed Sep 12, 2022
View reviewed changes
Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good, all I found were 3 tiny nitpicks, and IIRC you also prefer brace construction when possible, so I'll validate and push changes to save time. (I'd request changes if I were less sure you'd prefer them.)

stl/inc/iterator Outdated Show resolved Hide resolved
stl/inc/iterator Outdated Show resolved Hide resolved
tests/std/tests/GH_002992_unwrappable_iter_sent_pairs/test.compile.pass.cpp Show resolved Hide resolved
@StephanTLavavej StephanTLavavej self-assigned this Sep 12, 2022
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit ac129e5 into microsoft:main Sep 13, 2022
@StephanTLavavej
Copy link
Member

Thanks for this ranges performance improvement! 🚀 🎉 😻 🎁

CaseyCarter pushed a commit to CaseyCarter/STL that referenced this pull request Oct 6, 2022
@strega-nil-ms @strega-nil
@StephanTLavavej @CaseyCarter
Add unwrapping to move_sentinel (microsoft#3098)
0425072 
Co-authored-by: Nicole Mazzuca <mazzucan@outlook.com>
Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>
@strega-nil-ms strega-nil-ms deleted the unwrap-move_sentinel branch December 19, 2022 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CaseyCarter CaseyCarter CaseyCarter approved these changes

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

Assignees

@StephanTLavavej StephanTLavavej

Labels
performance Must go faster ranges C++20/23 ranges
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

<iterator>: move_sentinel could be unwrappable
4 participants
@strega-nil-ms @StephanTLavavej @CaseyCarter @strega-nil