Skip to content

implement LWG-3471: polymorphic_allocator::allocate does not satisfy Cpp17Allocator requirements - #2567

Merged
Stephan T. Lavavej (StephanTLavavej) merged 3 commits into
microsoft:mainfrom
fsb4000:fix2550
Feb 18, 2022
Merged

Stephan T. Lavavej (StephanTLavavej) merged 3 commits into
microsoft:mainfrom
fsb4000:fix2550

Conversation

@fsb4000

Copy link
Copy Markdown
Contributor

Fixes #2550

I saw that the changes were in "Previous resolution [SUPERSEDED]" but as far as I understand the new resolution is needed because of changing the wording and the proposed implementation stays the same.

std::launder is in <new>

<xmemory> includes <new>

<xpolymorphic_allocator.h> includes <xmemory>

@fsb4000
Igor Zhukov (fsb4000) requested a review from a team as a code owner February 14, 2022 07:07
Comment thread stl/inc/xpolymorphic_allocator.h Outdated
@timsong-cpp

timsong-cpp commented Feb 14, 2022

Copy link
Copy Markdown
Contributor

The original wording is incorrect (and the launder has no effect - laundering a pointer to byte that is already pointing to a live std::byte accomplishes precisely nothing).

The intended implementation (taking advantage of [intro.object]/13) is

    return ::operator new(_Bytes, _Ptr);

Co-authored-by: timsong-cpp <rs2740@gmail.com>
@CaseyCarter Casey Carter (CaseyCarter) added the LWG Library Working Group issue label Feb 14, 2022

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 can't imagine how we would test for this, so I won't ask for test coverage.

@StephanTLavavej

Copy link
Copy Markdown
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. (I'll handle verifying that this works with the recent LLVM update.)

Stephan T. Lavavej (StephanTLavavej) added a commit to StephanTLavavej/STL that referenced this pull request Feb 17, 2022
@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit b92f2c8 into microsoft:main Feb 18, 2022
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks for implementing another LWG issue resolution! ✅ 💯 🎉

@fsb4000
Igor Zhukov (fsb4000) deleted the fix2550 branch February 18, 2022 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LWG Library Working Group issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LWG-3471 polymorphic_allocator::allocate does not satisfy Cpp17Allocator requirements

5 participants