Skip to content

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

Merged
StephanTLavavej merged 3 commits intomicrosoft:mainfrom
CaseyCarter:variant_140
Oct 12, 2022
Merged

_STD-qualify _Ugly non-member function calls in <variant>#3148
StephanTLavavej merged 3 commits intomicrosoft:mainfrom
CaseyCarter:variant_140

Conversation

@CaseyCarter
Copy link
Contributor

@CaseyCarter CaseyCarter commented Oct 10, 2022

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 mentioned this pull request Oct 10, 2022
Open
CaseyCarter
CaseyCarter commented Oct 10, 2022
View reviewed changes
@StephanTLavavej StephanTLavavej self-assigned this Oct 11, 2022
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Oct 11, 2022

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

@StephanTLavavej
Copy link
Member

StephanTLavavej commented Oct 12, 2022

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
Contributor Author

CaseyCarter commented Oct 12, 2022

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
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Oct 12, 2022

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

@CaseyCarter CaseyCarter deleted the variant_140 branch October 12, 2022 19:03
@StephanTLavavej StephanTLavavej mentioned this pull request Nov 7, 2022
Merged
@frederick-vs-ja frederick-vs-ja mentioned this pull request Nov 6, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

+1 more reviewer

@strega-nil-ms strega-nil-ms strega-nil-ms approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@StephanTLavavej StephanTLavavej

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@CaseyCarter @StephanTLavavej @strega-nil-ms