Page MenuHomePhabricator

shared_ptr deleter requirements (2802)
Needs ReviewPublic

Authored by zoecarver on May 22 2019, 5:04 PM.

Details

Reviewers
mclow.lists
ldionne
EricWF
Group Reviewers
Restricted Project
Summary

This patch implements 2802. Requires _Deleter to have call operator and be move constructible. Based on D62233.

Refs PR37637.

Diff Detail

Event Timeline

zoecarver created this revision.May 22 2019, 5:04 PM
EricWF requested changes to this revision.May 22 2019, 11:21 PM
EricWF added a subscriber: EricWF.

This patch is missing the main point of LWG 2802, and that is making std::shared_ptr support move-only deleters. You need to make this test that constructors and make functions accepting a deleter accept this code:

template <class ValueT>
struct MyDeleter {
  MyDeleter() = delete;
  MyDeleter(MyDeleter const&) = delete;
  MyDeleter(MyDeleter&&) = default;

  explicit MyDeleter(secret_type) {} // so you can construct it for the test.

  void operator()(ValueT*); 
};

Additionally, the static asserts you're adding are required to be SFINAE checks according to the standard.
This was added by LWG 2875 (https://cplusplus.github.io/LWG/lwg-defects.html#2875)

include/memory
3719 ↗(On Diff #200839)

Does this compile in C++03, C++11, or C++14?

This revision now requires changes to proceed.May 22 2019, 11:21 PM
zoecarver updated this revision to Diff 201094.May 23 2019, 4:52 PM
  • fix static asserts
  • make compliant with C++11
  • fix 2875
  • update tests

I will fix the remaining C++03 issues in D62233.

zoecarver updated this revision to Diff 226156.Oct 23 2019, 9:38 AM
zoecarver edited the summary of this revision. (Show Details)

I rewrote this patch so that it doesn't require my (some, now obsolete) other patches. This patch does now rely on the fix from D69344 for C++03 move-constructibility. Also, I updated the tests.

Herald added a project: Restricted Project. · View Herald TranscriptOct 23 2019, 9:38 AM
Herald added a subscriber: christof. · View Herald Transcript
zoecarver updated this revision to Diff 226158.Oct 23 2019, 9:41 AM

Don't mark 2875 as complete. That would requires some of my other patches so, I'll fix that issue down the road.

zoecarver marked 2 inline comments as done.Oct 23 2019, 9:42 AM
zoecarver added inline comments.
libcxx/include/memory
2201

Ignore this, it's part of D69344.

libcxx/test/libcxx/utilities/compressed_pair/move_const.pass.cpp
1 ↗(On Diff #226156)

Also part of D69344.

  • Rebase off master.
  • Dissable in C++03 mode.
Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 10 2020, 12:12 PM
zoecarver edited the summary of this revision. (Show Details)Jun 10 2020, 12:12 PM

In C++03 __compressed_pair doesn't have an implicit move constructor. So, I disabled the tests in C++03.

  • Mark as complete in cxx1z status
zoecarver edited the summary of this revision. (Show Details)Jun 10 2020, 12:15 PM
zoecarver edited the summary of this revision. (Show Details)
zoecarver marked an inline comment as done.
zoecarver added inline comments.
libcxx/www/cxx1z_status.html
461

The rest of the changes in this issue are addressed in D81414.