Skip to content

Conversation

@CaseyCarter
Copy link
Contributor

@CaseyCarter CaseyCarter commented Nov 12, 2020

By defining polymorphic_allocator::destroy - despite that it's equivalent to the default in allocator_traits - as deprecated. Also hack up the internal _Has_no_alloc_destroy trait so our optimization that avoids calling destroy on trivially destructible types keeps working with polymorphic_allocator.

Fixes #1454 and fixes #753.

By defining `polymorphic_allocator::destroy` - despite that it's equivalent to the default in `allocator_traits` - as deprecated. Also hack up the internal `_Has_no_alloc_destroy` trait so our optimization that avoids calling `destroy` on trivially destructible types keeps working with `polymorphic_allocator`.

Fixes microsoft#1454.
@CaseyCarter CaseyCarter added the LWG Library Working Group issue label Nov 12, 2020
@CaseyCarter CaseyCarter requested a review from a team as a code owner November 12, 2020 06:03
@CaseyCarter CaseyCarter mentioned this pull request Nov 12, 2020
52 tasks
@CaseyCarter CaseyCarter self-assigned this Nov 12, 2020
@CaseyCarter CaseyCarter changed the title Implement LWG-3036 LWG-3036 polymorphic_allocator::destroy is extraneous Nov 18, 2020
@CaseyCarter CaseyCarter removed their assignment Dec 2, 2020
@StephanTLavavej StephanTLavavej self-assigned this Dec 2, 2020
Add missing spaces.

Cite LWG-3036.

Add parens after functions.
@StephanTLavavej
Copy link
Member

Looks good to me, thanks for implementing this (especially the _Has_no_alloc_destroy optimization). I pushed a few changes:

  • Explain that this "is deprecated in C++17 by LWG-3036" and change the macros to CXX17.
  • Add a space at the end of the sentence mentioning allocator_traits::destroy as an alternative. Add another in the warning STL4021 message above.
  • Add parentheses after destroy_at and allocator_traits::destroy to match the style used in other deprecation warning messages.
  • In test code, mark the constructor S(bool&) as explicit.

@StephanTLavavej StephanTLavavej removed their assignment Dec 3, 2020
@StephanTLavavej

This comment has been minimized.

@azure-pipelines

This comment has been minimized.

@cbezault cbezault assigned StephanTLavavej and unassigned cbezault Dec 3, 2020
@StephanTLavavej StephanTLavavej merged commit 2aa944b into microsoft:master Dec 3, 2020
@StephanTLavavej
Copy link
Member

Thanks again for implementing this user-requested member function, deprecating it so users will decide they don't want it after all 😹, and ensuring that the STL continues to generate efficient code! 🚀

@CaseyCarter CaseyCarter deleted the lwg3036 branch December 3, 2020 23:19
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-3036 polymorphic_allocator::destroy is extraneous <memory_resource>: std::pmr::polymorphic_allocator<T>::destroy missing

4 participants