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

Fix pair::swap(const pair&) and tuple::swap(const tuple&) errors with __declspec(dllexport) #3045

Merged
merged 2 commits into from
Aug 22, 2022

Conversation

StephanTLavavej
Copy link
Member

Fixes #3013, a high-priority customer-reported bug (see DevCom-10109884).

Normally, member functions of class templates are instantiated "on demand". (I like to remember this rule by thinking of list<T>, which does not require T to be less-than comparable until list<T>::sort() is called.) However, if a user derives from a Standard Library class (which we don't encourage, but which is widely done) and then dllexports their derived class, that attempts to instantiate all member functions (that are not templated themselves).

By adding member functions (as required by the Standard) that won't compile when force-instantiated like this, #2687 regressed this scenario. The fix is simple (and something we've used elsewhere): add template <int = 0> to make instantiation truly on-demand. Conveniently, we already have a test we can expand.

(Note: I explored using requires to fix this, but that's more verbose and ran into other compiler errors.)

@StephanTLavavej StephanTLavavej added bug Something isn't working high priority Important! labels Aug 19, 2022
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner August 19, 2022 22:52
@StephanTLavavej StephanTLavavej self-assigned this Aug 20, 2022
@StephanTLavavej
Copy link
Member Author

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

@AraHaan
Copy link

AraHaan commented Aug 20, 2022

I think this looks good.

Copy link

@evelynharthbrooke evelynharthbrooke left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@StephanTLavavej StephanTLavavej merged commit febb643 into microsoft:main Aug 22, 2022
@StephanTLavavej StephanTLavavej deleted the swap-til-you-drop branch August 22, 2022 21:29
@CaseyCarter
Copy link
Member

CaseyCarter commented Aug 22, 2022

Thanks for fixing this pair of issues - I'm sure our customers were about to pull their hair out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high priority Important!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

<utility>: pair::swap(const pair&) interacts badly with __declspec(dllexport)
5 participants
  翻译: