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

Add Non-exception Throwing Specifier to ranges::ssize #4231

Merged
merged 3 commits into from
Dec 7, 2023

Conversation

PhiliaTheCat
Copy link
Contributor

@PhiliaTheCat PhiliaTheCat commented Dec 2, 2023

Fixes #4107.

Non-exception throwing specifier added in stl/inc/xutility:3357

Besides, some improper fomats are changed in stl/inc/xutility

@PhiliaTheCat PhiliaTheCat requested a review from a team as a code owner December 2, 2023 02:40
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja left a comment

Choose a reason for hiding this comment

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

It would be better to use GitHub's close/fix/resolve syntax (e.g. saying Fixes #4107) in the description of this PR (instead of the commit message).

stl/inc/xutility Outdated Show resolved Hide resolved
@frederick-vs-ja
Copy link
Contributor

Also, it would be better to add STATIC_ASSERT(noexcept(ranges::ssize(t)) == Nothrow); immediately below this line, which would properly test the corrected exception specification.

STATIC_ASSERT(noexcept(ranges::size(t)) == Nothrow);

@PhiliaTheCat
Copy link
Contributor Author

@microsoft-github-policy-service agree

@StephanTLavavej StephanTLavavej added bug Something isn't working ranges C++20/23 ranges labels Dec 3, 2023
@StephanTLavavej StephanTLavavej changed the title Fixes #4107: Add Non-exception Throwing Specifier Add Non-exception Throwing Specifier to ranges::ssize Dec 3, 2023
@StephanTLavavej
Copy link
Member

Looks great, thanks! I updated your PR description to properly link to the issue that it resolves, and updated the PR title to mention what ranges function is being changed.

I think this needs only one maintainer approval. We merge PRs simultaneously to the GitHub and MSVC-internal repos, batched up in a semi-manual process to save time. Your PR will be part of the next batch, probably next week.

@StephanTLavavej StephanTLavavej self-assigned this Dec 7, 2023
@StephanTLavavej
Copy link
Member

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

@PhiliaTheCat
Copy link
Contributor Author

PhiliaTheCat commented Dec 7, 2023 via email

@StephanTLavavej StephanTLavavej merged commit 706b425 into microsoft:main Dec 7, 2023
35 checks passed
@StephanTLavavej
Copy link
Member

Thanks for fixing ranges::ssize and congratulations on your first microsoft/STL commit! 🎉 😻 🚀

This will ship in VS 2022 17.10 Preview 1.

@PhiliaTheCat PhiliaTheCat deleted the PhiliaTheCat/issue4107 branch December 8, 2023 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ranges C++20/23 ranges
Projects
None yet
Development

Successfully merging this pull request may close these issues.

<xutility>: ranges::_Ssize_fn::operator() should have a noexcept-specifier
4 participants