Skip to content

make _Get_unwrapped noexcept when possible #2991

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
Merged
merged 12 commits into from
Aug 16, 2022
Merged

make _Get_unwrapped noexcept when possible #2991

merged 12 commits into from
Aug 16, 2022

Conversation

strega-nil-ms
Copy link
Contributor

@strega-nil-ms strega-nil-ms commented Aug 3, 2022 •
edited
Loading

additionally:

  • add _Unwrapped() && noexcept to path::iterator
  • add _Unwrapped() && to reverse_iterator
  • add conditional noexcepts to:
    • counted_iterator
    • take_view::_Sentinel
    • take_while_view::_Sentinel
    • move_iterator

Fixes #2989

Requires testing

@strega-nil
make _Get_unwrapped noexcept when possible
36fcea2 
additionally, add conditional noexcepts to a lot of `_Unwrapped()`s
@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Aug 3, 2022
@CaseyCarter
Copy link
Contributor

CaseyCarter commented Aug 3, 2022 •
edited
Loading

I want to say that path::iterator::_Unwrapped should be rvalue-qualified and noexcept, which is consistent with the design to support unwrapping move-only input iterators, but then they couldn't be sentinels. Sentinels and forward iterators must support non-modifying lvalue unwrap. (As if I needed another reason to hate path::iterator.)

We should consider adding a _Unwrapped() && noexcept for perf.

@strega-nil-ms strega-nil-ms marked this pull request as ready for review August 5, 2022 18:49
@strega-nil-ms strega-nil-ms requested a review from a team as a code owner August 5, 2022 18:49
@strega-nil
moar testing
e53cbba 
plus, fix reverse_iterator up with an `_Unwrapped() &&`
@strega-nil-ms strega-nil-ms mentioned this pull request Aug 5, 2022
Open
@strega-nil strega-nil mentioned this pull request Aug 6, 2022
Closed
CaseyCarter
CaseyCarter suggested changes Aug 6, 2022
View reviewed changes
CaseyCarter
CaseyCarter suggested changes Aug 9, 2022
View reviewed changes
@strega-nil-ms strega-nil-ms mentioned this pull request Aug 9, 2022
Merged
CaseyCarter
CaseyCarter approved these changes Aug 12, 2022
View reviewed changes
StephanTLavavej
StephanTLavavej reviewed Aug 12, 2022
View reviewed changes
strega-nil-ms pushed a commit to miscco/STL that referenced this pull request Aug 12, 2022
@strega-nil
make clean merge with microsoft#2991
561cd62
@StephanTLavavej StephanTLavavej self-assigned this Aug 16, 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 ebbc990 into microsoft:main Aug 16, 2022
@StephanTLavavej
Copy link
Member

Thanks for this PR - let's call it a wrap! 😹 🎁 🎥

@strega-nil-ms strega-nil-ms deleted the is_nothrow_unwrapped 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

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

+1 more reviewer

@CaseyCarter CaseyCarter CaseyCarter approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@StephanTLavavej StephanTLavavej

Labels
enhancement Something can be improved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

_Get_unwrapped et. al. should (almost) always be noexcept
4 participants
@strega-nil-ms @CaseyCarter @StephanTLavavej @strega-nil