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

ADL-proof implementation of uninitialized memory algorithms #4374

Merged
merged 9 commits into from Feb 13, 2024

Conversation

frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Feb 8, 2024

Separated from #4004. Towards #140 and #1596.

There's a separated commit addressing cases noted in #4367 (comment). Note that EDG currently incorrectly thinks Derived* and Base* are subtractable under some conditions, which should be is reported as DevCom-10581519 and should be cited as VSO-XXXX.

Unfortunately, currently there doesn't seem possible to verify that _Copy_memcpy_distance and _Copy_memcpy_count calls are properly qualified, because the comparison between tagged_derived<holder<incomplete>>* and unreachable_sentinel_t is ill-formed due to ADL.

As a workaround for DevCom-10456452, the __builtin_launder call is qualified.

This PR changes one line _Destroy_range in the same way as #4373, which shouldn't raise merge conflict IIUC.

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner February 8, 2024 09:36
@github-actions github-actions bot added this to Initial Review in Code Reviews Feb 8, 2024
@frederick-vs-ja frederick-vs-ja changed the title Adl-proof implementation of uninitialized memory algorithms ADL-proof implementation of uninitialized memory algorithms Feb 8, 2024
@StephanTLavavej StephanTLavavej added the bug Something isn't working label Feb 8, 2024
@StephanTLavavej StephanTLavavej self-assigned this Feb 8, 2024
@StephanTLavavej
Copy link
Member

Thanks! I pushed a few changes, and I think this is ready to go now.

@StephanTLavavej StephanTLavavej removed their assignment Feb 8, 2024
@StephanTLavavej StephanTLavavej moved this from Initial Review to Ready To Merge in Code Reviews Feb 8, 2024
@StephanTLavavej StephanTLavavej self-assigned this Feb 12, 2024
@StephanTLavavej
Copy link
Member

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

@StephanTLavavej StephanTLavavej merged commit 6682109 into microsoft:main Feb 13, 2024
35 checks passed
Code Reviews automation moved this from Ready To Merge to Done Feb 13, 2024
@StephanTLavavej
Copy link
Member

Thanks for fixing uninitialized_meow()! 🎉 🐈 🛠️

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
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants