Skip to content

Commit c9d616a

Browse files
cschreibcschreib
committed
Clarify movement of deleters
1 parent 3a15a41 commit c9d616a

File tree

2 files changed

+141
-19
lines changed

2 files changed

+141
-19
lines changed

‎include/oup/observable_unique_ptr.hpp‎

Lines changed: 115 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -103,14 +103,14 @@ class observable_unique_ptr {
103103
* \param value The pointer to own
104104
* \note This is used by make_observable_unique().
105105
*/
106-
observable_unique_ptr(control_block* ctrl, T* value) : block(ctrl), data(value) {}
106+
observable_unique_ptr(control_block* ctrl, T* value) noexcept : block(ctrl), data(value) {}
107107

108108
/// Private constructor using pre-allocated control block.
109109
/** \param ctrl The control block pointer
110110
* \param value The pointer to own
111111
* \note This is used by make_observable_unique().
112112
*/
113-
observable_unique_ptr(control_block* ctrl, T* value, Deleter del) :
113+
observable_unique_ptr(control_block* ctrl, T* value, Deleter del) noexcept :
114114
block(ctrl), data(value), deleter(del) {}
115115

116116
// Friendship is required for conversions.
@@ -175,7 +175,8 @@ class observable_unique_ptr {
175175
/// Transfer ownership by implicit casting
176176
/** \param value The pointer to take ownership from
177177
* \note After this observable_unique_ptr is created, the source
178-
* pointer is set to null and looses ownership.
178+
* pointer is set to null and looses ownership. The source deleter
179+
* is moved.
179180
*/
180181
observable_unique_ptr(observable_unique_ptr&& value) noexcept :
181182
observable_unique_ptr(value.block, value.data, std::move(value.deleter)) {
@@ -186,11 +187,14 @@ class observable_unique_ptr {
186187
/// Transfer ownership by implicit casting
187188
/** \param value The pointer to take ownership from
188189
* \note After this observable_unique_ptr is created, the source
189-
* pointer is set to null and looses ownership.
190+
* pointer is set to null and looses ownership. The source deleter
191+
* is moved. This constructor only takes part in overload resolution
192+
* if D is convertible to Deleter and U* is convertible to T*.
190193
*/
191-
template<typename U, typename D/*, typename enable = std::enable_if_t<std::is_convertible_v<U*, T*>>*/>
194+
template<typename U, typename D, typename enable =
195+
std::enable_if_t<std::is_convertible_v<U*, T*> && std::is_convertible_v<D, Deleter>>>
192196
observable_unique_ptr(observable_unique_ptr<U,D>&& value) noexcept :
193-
observable_unique_ptr(value.block, value.data) {
197+
observable_unique_ptr(value.block, value.data, std::move(value.deleter)) {
194198
value.block = nullptr;
195199
value.data = nullptr;
196200
}
@@ -199,21 +203,58 @@ class observable_unique_ptr {
199203
/** \param manager The smart pointer to take ownership from
200204
* \param value The casted pointer value to take ownership of
201205
* \note After this observable_unique_ptr is created, the source
202-
* pointer is set to null and looses ownership.
206+
* pointer is set to null and looses ownership. The deleter
207+
* is default constructed.
203208
*/
204-
template<typename U, typename D/*, typename enable = std::enable_if_t<std::is_convertible_v<U*, T*>>*/>
209+
template<typename U, typename D>
205210
observable_unique_ptr(observable_unique_ptr<U,D>&& manager, T* value) noexcept :
206211
observable_unique_ptr(manager.block, value) {
207212
manager.block = nullptr;
208213
manager.data = nullptr;
209214
}
210215

216+
/// Transfer ownership by explicit casting
217+
/** \param manager The smart pointer to take ownership from
218+
* \param value The casted pointer value to take ownership of
219+
* \param del The deleter to use in the new pointer
220+
* \note After this observable_unique_ptr is created, the source
221+
* pointer is set to null and looses ownership.
222+
*/
223+
template<typename U, typename D>
224+
observable_unique_ptr(observable_unique_ptr<U,D>&& manager, T* value, Deleter del) noexcept :
225+
observable_unique_ptr(manager.block, value, std::move(del)) {
226+
manager.block = nullptr;
227+
manager.data = nullptr;
228+
}
229+
211230
/// Transfer ownership by implicit casting
212231
/** \param value The pointer to take ownership from
213232
* \note After this observable_unique_ptr is created, the source
214233
* pointer is set to null and looses ownership.
215234
*/
216-
template<typename U, typename D/*, typename enable = std::enable_if_t<std::is_convertible_v<U*, T*>>*/>
235+
observable_unique_ptr& operator=(observable_unique_ptr&& value) noexcept {
236+
if (data) {
237+
delete_and_pop_ref_();
238+
}
239+
240+
block = value.block;
241+
value.block = nullptr;
242+
data = value.data;
243+
value.data = nullptr;
244+
deleter = std::move(value.deleter);
245+
246+
return *this;
247+
}
248+
249+
/// Transfer ownership by implicit casting
250+
/** \param value The pointer to take ownership from
251+
* \note After this observable_unique_ptr is created, the source
252+
* pointer is set to null and looses ownership. The source deleter
253+
* is moved. This operator only takes part in overload resolution
254+
* if D is convertible to Deleter and U* is convertible to T*.
255+
*/
256+
template<typename U, typename D, typename enable =
257+
std::enable_if_t<std::is_convertible_v<U*, T*> && std::is_convertible_v<D, Deleter>>>
217258
observable_unique_ptr& operator=(observable_unique_ptr<U,D>&& value) noexcept {
218259
if (data) {
219260
delete_and_pop_ref_();
@@ -452,7 +493,7 @@ class observer_ptr {
452493
}
453494

454495
/// Create a weak pointer from an owning pointer.
455-
template<typename U>
496+
template<typename U, typename enable = std::enable_if_t<std::is_convertible_v<U*, T*>>>
456497
observer_ptr(const observable_unique_ptr<U>& owner) noexcept :
457498
block(owner.block), data(owner.data) {
458499
if (block) {
@@ -463,7 +504,7 @@ class observer_ptr {
463504
/// Copy an existing observer_ptr instance
464505
/** \param value The existing weak pointer to copy
465506
*/
466-
template<typename U>
507+
template<typename U, typename enable = std::enable_if_t<std::is_convertible_v<U*, T*>>>
467508
observer_ptr(const observer_ptr<U>& value) noexcept :
468509
block(value.block), data(value.data) {
469510
if (block) {
@@ -476,14 +517,30 @@ class observer_ptr {
476517
* \note After this observable_unique_ptr is created, the source
477518
* pointer is set to null.
478519
*/
479-
template<typename U>
520+
observer_ptr(observer_ptr&& value) noexcept : block(value.block), data(value.data) {
521+
value.block = nullptr;
522+
value.data = nullptr;
523+
}
524+
525+
/// Move from an existing observer_ptr instance
526+
/** \param value The existing weak pointer to move from
527+
* \note After this observable_unique_ptr is created, the source
528+
* pointer is set to null. This constructor only takes part in
529+
* overload resolution if D is convertible to Deleter and U* is
530+
* convertible to T*.
531+
*/
532+
template<typename U, typename enable = std::enable_if_t<std::is_convertible_v<U*, T*>>>
480533
observer_ptr(observer_ptr<U>&& value) noexcept : block(value.block), data(value.data) {
481534
value.block = nullptr;
482535
value.data = nullptr;
483536
}
484537

485538
/// Point to another owning pointer.
486-
template<typename U>
539+
/** \param owner The new owner pointer to observe
540+
* \note This operator only takes part in overload resolution if D
541+
* is convertible to Deleter and U* is convertible to T*.
542+
*/
543+
template<typename U, typename enable = std::enable_if_t<std::is_convertible_v<U*, T*>>>
487544
observer_ptr& operator=(const observable_unique_ptr<U>& owner) noexcept {
488545
if (data) {
489546
pop_ref_();
@@ -501,7 +558,26 @@ class observer_ptr {
501558
/// Copy an existing observer_ptr instance
502559
/** \param value The existing weak pointer to copy
503560
*/
504-
template<typename U>
561+
observer_ptr& operator=(const observer_ptr& value) noexcept {
562+
if (data) {
563+
pop_ref_();
564+
}
565+
566+
block = value.block;
567+
data = value.data;
568+
if (block) {
569+
++block->refcount;
570+
}
571+
572+
return *this;
573+
}
574+
575+
/// Copy an existing observer_ptr instance
576+
/** \param value The existing weak pointer to copy
577+
* \note This operator only takes part in overload resolution if D
578+
* is convertible to Deleter and U* is convertible to T*.
579+
*/
580+
template<typename U, typename enable = std::enable_if_t<std::is_convertible_v<U*, T*>>>
505581
observer_ptr& operator=(const observer_ptr<U>& value) noexcept {
506582
if (data) {
507583
pop_ref_();
@@ -521,7 +597,27 @@ class observer_ptr {
521597
* \note After the assignment is complete, the source
522598
* pointer is set to null and looses ownership.
523599
*/
524-
template<typename U>
600+
observer_ptr& operator=(observer_ptr&& value) noexcept {
601+
if (data) {
602+
pop_ref_();
603+
}
604+
605+
block = value.block;
606+
value.block = nullptr;
607+
data = value.data;
608+
value.data = nullptr;
609+
610+
return *this;
611+
}
612+
613+
/// Move from an existing observer_ptr instance
614+
/** \param value The existing weak pointer to move from
615+
* \note After the assignment is complete, the source
616+
* pointer is set to null and looses ownership.
617+
* This operator only takes part in overload resolution if D
618+
* is convertible to Deleter and U* is convertible to T*.
619+
*/
620+
template<typename U, typename enable = std::enable_if_t<std::is_convertible_v<U*, T*>>>
525621
observer_ptr& operator=(observer_ptr<U>&& value) noexcept {
526622
if (data) {
527623
pop_ref_();
@@ -631,12 +727,14 @@ bool operator!= (std::nullptr_t, const observer_ptr<T>& value) noexcept {
631727
return !value.expired();
632728
}
633729

634-
template<typename T, typename U>
730+
template<typename T, typename U, typename enable =
731+
std::enable_if_t<std::is_convertible_v<U*, T*> || std::is_convertible_v<T*, U*>>>
635732
bool operator== (const observer_ptr<T>& first, const observer_ptr<U>& second) noexcept {
636733
return first.get() == second.get();
637734
}
638735

639-
template<typename T, typename U>
736+
template<typename T, typename U, typename enable =
737+
std::enable_if_t<std::is_convertible_v<U*, T*> || std::is_convertible_v<T*, U*>>>
640738
bool operator!= (const observer_ptr<T>& first, const observer_ptr<U>& second) noexcept {
641739
return first.get() != second.get();
642740
}

‎tests/runtime_tests.cpp‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ TEST_CASE("owner implicit conversion constructor with deleter", "[owner_construc
132132
REQUIRE(instances_derived == 1);
133133
REQUIRE(instances_deleter == 2);
134134
REQUIRE(ptr.get() != nullptr);
135-
REQUIRE(ptr.get_deleter().state_ == 0);
135+
REQUIRE(ptr.get_deleter().state_ == 42);
136136
}
137137

138138
REQUIRE(instances == 0);
@@ -164,7 +164,7 @@ TEST_CASE("owner explicit conversion constructor", "[owner_construction]") {
164164
REQUIRE(instances_derived == 0);
165165
}
166166

167-
TEST_CASE("owner explicit conversion constructor with deleter", "[owner_construction]") {
167+
TEST_CASE("owner explicit conversion constructor with default deleter", "[owner_construction]") {
168168
{
169169
test_ptr_with_deleter ptr_orig{new test_object_derived, test_deleter{42}};
170170
{
@@ -187,6 +187,30 @@ TEST_CASE("owner explicit conversion constructor with deleter", "[owner_construc
187187
REQUIRE(instances_deleter == 0);
188188
}
189189

190+
TEST_CASE("owner explicit conversion constructor with custom deleter", "[owner_construction]") {
191+
{
192+
test_ptr_with_deleter ptr_orig{new test_object_derived, test_deleter{42}};
193+
{
194+
test_ptr_derived_with_deleter ptr(std::move(ptr_orig),
195+
dynamic_cast<test_object_derived*>(ptr_orig.get()),
196+
test_deleter{43});
197+
REQUIRE(instances == 1);
198+
REQUIRE(instances_derived == 1);
199+
REQUIRE(instances_deleter == 2);
200+
REQUIRE(ptr.get() != nullptr);
201+
REQUIRE(ptr.get_deleter().state_ == 43);
202+
}
203+
204+
REQUIRE(instances == 0);
205+
REQUIRE(instances_derived == 0);
206+
REQUIRE(instances_deleter == 1);
207+
}
208+
209+
REQUIRE(instances == 0);
210+
REQUIRE(instances_derived == 0);
211+
REQUIRE(instances_deleter == 0);
212+
}
213+
190214
TEST_CASE("owner move assignment operator", "[owner_assignment]") {
191215
{
192216
test_ptr ptr_orig(new test_object);

0 commit comments

Comments
 (0)