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

_STD-qualify _Ugly non-member function calls in <variant> #3148

Merged
merged 3 commits into from
Oct 12, 2022

Conversation

CaseyCarter
Copy link
Member

Qualifies calls to namespace-scope functions in <variant>, and enables the libc++ ADL test that now passes.

Partially addresses #140

@CaseyCarter CaseyCarter added the enhancement Something can be improved label Oct 10, 2022
@CaseyCarter CaseyCarter requested a review from a team as a code owner October 10, 2022 20:32
@CaseyCarter CaseyCarter added bug Something isn't working and removed enhancement Something can be improved labels Oct 10, 2022
@CaseyCarter CaseyCarter added this to Final Review in Code Reviews Oct 10, 2022
@strega-nil-ms strega-nil-ms moved this from Final Review to Ready To Merge in Code Reviews Oct 11, 2022
@StephanTLavavej StephanTLavavej self-assigned this Oct 11, 2022
@StephanTLavavej
Copy link
Member

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

@StephanTLavavej
Copy link
Member

I've had to push an additional commit to fix internal test failures with /clr. I was a bad kitty and didn't investigate the root cause (or mark this as TRANSITION), but it appears that /clr is deciding to instantiate stuff at the end of the TU, and this ends up requiring completeness. Since this is a fairly niche use case, I figured that just skipping the affected portion for /clr would be strictly better than the status quo where it's unconditionally skipped.

@CaseyCarter
Copy link
Member Author

I was a bad kitty and didn't investigate the root cause (or mark this as TRANSITION)

I'm the last person you'll have to convince that we should do any extra work where /clr is concerned.

@StephanTLavavej StephanTLavavej merged commit cdf9746 into microsoft:main Oct 12, 2022
Code Reviews automation moved this from Ready To Merge to Done Oct 12, 2022
@StephanTLavavej
Copy link
Member

Thanks for investigating and fixing this! 🕵️ 🛠️ 🎉

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
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants