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

Strengthen exception specifications for stream types #3314

Merged
merged 10 commits into from
Jan 22, 2023

Conversation

frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Dec 31, 2022

This PR partially addresses #3278, but also strengthens exception specifications for some other functions. References to working draft are also updated to WG21-N4928.

Move assignment operators and swap functions are generally marked with noexcept. But I think move assignment operators of basic_filebuf and basic_(i|o)fstream shouldn't be noexcept, because they call basic_filebuf::close which may in turn call virtual functions that are possibly overridden. In other words, exception may be thrown from these move assignment operators. basic_syncbuf's should neither be noexcept due to LWG-3498.

Many move constructors remain non-noexcept, because they need to copy-construct a basic_streambuf, and hence need to dynamically allocate storage for locale. Maybe we should fix this in vNext.


Currently EDG wrongly calculates std::is_nothrow_move_assignable_v<std::osyncstream> and std::is_nothrow_move_assignable_v<std::wosyncstream> as false, so these two static_asserts are skipped for EDG.

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner December 31, 2022 10:06
Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

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

Apart from the noexcept on osfx, this looks good; I can't guarantee I didn't miss something, though, since it's quite a lot of code...

stl/inc/fstream Show resolved Hide resolved
stl/inc/fstream Show resolved Hide resolved
stl/inc/ostream Show resolved Hide resolved
@strega-nil-ms strega-nil-ms added the enhancement Something can be improved label Jan 3, 2023
stl/inc/sstream Outdated Show resolved Hide resolved
stl/inc/spanstream Outdated Show resolved Hide resolved
stl/inc/spanstream Outdated Show resolved Hide resolved
stl/inc/strstream Outdated Show resolved Hide resolved
tests/std/tests/GH_001858_iostream_exception/test.cpp Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej removed their assignment Jan 21, 2023
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Jan 21, 2023

@strega-nil-ms FYI, I pushed a conflict-free merge with main, 3 comment changes for stable names, one this-> removal, and one 🐿️ removal after you approved.

@StephanTLavavej StephanTLavavej self-assigned this Jan 21, 2023
@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 31defd3 into microsoft:main Jan 22, 2023
@StephanTLavavej
Copy link
Member

Thanks for strengthening the STL! 🦾 🏋️ 📈

@frederick-vs-ja frederick-vs-ja deleted the iostream-noexcept branch January 22, 2023 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants