Skip to content

: Fix the default constructor of error_category conditionally - #3139

Merged
Stephan T. Lavavej (StephanTLavavej) merged 10 commits into
microsoft:mainfrom
frederick-vs-ja:error_category-ctor
Oct 24, 2022
Merged

Stephan T. Lavavej (StephanTLavavej) merged 10 commits into
microsoft:mainfrom
frederick-vs-ja:error_category-ctor

Conversation

@frederick-vs-ja

Copy link
Copy Markdown
Contributor

Fixes #3119.

Addresses #1116 (fixes it except for /clr:pure) and unblocks one libcxx test.

This PR would complicate Vinnie Falco (@vinniefalco)'s #3044 if it gets merged.

@frederick-vs-ja
A. Jiang (frederick-vs-ja) requested a review from a team as a code owner October 1, 2022 22:57

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.

I don't love that we have to do this to keep ABI, but this is certainly better than the alternative where this isn't constexpr.

@CaseyCarter Casey Carter (CaseyCarter) added the bug Something isn't working label Oct 4, 2022
Comment thread tests/std/tests/GH_003119_error_category_ctor/test.compile.pass.cpp Outdated
Comment thread stl/inc/system_error Outdated
Comment thread stl/inc/system_error
Comment thread stl/inc/system_error Outdated
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks, this is awesome! 😻 I think we can regard #1116 as completely fixed by this (/clr:pure has virtually no users, and there's nothing more we can do here, so no need to keep a bug open; everything mentioning /clr:pure will inherently be eradicated during vNext).

I pushed a conflict-free and no-reformatting-needed merge with main, followed by a couple of minor cleanups for /clr:pure, and a small enhancement adding noexcept to the _Addr_storage ctors so the compiler won't emit EH enforcement in debug mode (as the ctors that use this are marked noexcept). FYI nicole mazzuca (@strega-nil-ms) as you previously approved.

@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

Copy link
Copy Markdown
Member

I've pushed an additional commit to silence a warning found by internal testing.

@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit 569cf9b into microsoft:main Oct 24, 2022
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks for fixing this bug we thought was unfixable! 🐞 🛠️ 😻

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

: Can we add constexpr to the default constructor of error_category without ABI breakage?

4 participants