Skip to content

implemented the proposed resolution for LWG-2762 - #2376

Merged
Stephan T. Lavavej (StephanTLavavej) merged 5 commits into
microsoft:mainfrom
OmarSaber98:LWG_2762
Dec 9, 2021
Merged

implemented the proposed resolution for LWG-2762#2376
Stephan T. Lavavej (StephanTLavavej) merged 5 commits into
microsoft:mainfrom
OmarSaber98:LWG_2762

Conversation

@OmarSaber98

Copy link
Copy Markdown
Contributor

Fixes #2257

@OmarSaber98
Omar (OmarSaber98) requested a review from a team as a code owner December 4, 2021 16:47
@ghost

Deleted user (ghost) commented Dec 4, 2021

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@CaseyCarter Casey Carter (CaseyCarter) added the LWG Library Working Group issue label Dec 4, 2021

@CaseyCarter Casey Carter (CaseyCarter) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need a bit of test coverage - basically just a static_assert(noexcept(meow)) where meow is a call to each of these functions. Otherwise this looks great.

Comment thread stl/inc/memory Outdated
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks! I went ahead and pushed comment changes to the test, as it had commented-out lines like // ASSERT_NOT_NOEXCEPT(*opt); (explaining why libcxx couldn't test that, due to their GCC coverage) which are being completely superseded by your STATIC_ASSERTs that these expressions are now required to be noexcept. FYI Casey Carter (@CaseyCarter).

@OmarSaber98

Copy link
Copy Markdown
Contributor Author

I tried running the tests on compiler explorer before committing and on the latest msvc the tests were failing with /std:c++latest and /std:c++20 flags but was confused when i found that with /std:c++17 flag the tests were passing.

A reproduction of that can be found here: https://compiler-explorer.com/z/eGbjYof3M

@AlexGuteniev

Alex Guteniev (AlexGuteniev) commented Dec 7, 2021

Copy link
Copy Markdown
Contributor

A reproduction of that can be found here: https://compiler-explorer.com/z/eGbjYof3M

/permissive effect. Default of /permissive is different.
Add /permissive- to C++17 to make it fail or add /permissive to C++20 to make it pass.

@StephanTLavavej

Copy link
Copy Markdown
Member

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

@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit 524cab2 into microsoft:main Dec 9, 2021
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks for implementing this LWG issue resolution, and congratulations on your second microsoft/STL commit! 😻 ✨ 🚀

This will ship in VS 2022 17.2 Preview 2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LWG Library Working Group issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LWG-2762 unique_ptr operator*() should be noexcept

4 participants