From a1f7d1cd13575633ee9bd195acedd16a55bbb495 Mon Sep 17 00:00:00 2001 From: JiaShuo Date: Thu, 19 Dec 2019 10:21:22 +0800 Subject: [PATCH 1/6] fix autoref_ptr_test --- src/core/tests/autoref_ptr_test.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/core/tests/autoref_ptr_test.cpp b/src/core/tests/autoref_ptr_test.cpp index 66720c6716..aa2df64c4a 100644 --- a/src/core/tests/autoref_ptr_test.cpp +++ b/src/core/tests/autoref_ptr_test.cpp @@ -34,13 +34,12 @@ class Derived : public SelfAssign class ScopedRefPtrToSelf : public dsn::ref_counter { public: - ScopedRefPtrToSelf() : self_ptr_(this) {} + ScopedRefPtrToSelf() {} static bool was_destroyed() { return was_destroyed_; } static void reset_was_destroyed() { was_destroyed_ = false; } - dsn::ref_ptr self_ptr_; private: friend class dsn::ref_counter; @@ -152,9 +151,9 @@ TEST(RefCountedUnitTest, ScopedRefPtrToSelfPointerAssignment) { ScopedRefPtrToSelf::reset_was_destroyed(); - ScopedRefPtrToSelf *check = new ScopedRefPtrToSelf(); + dsn::ref_ptr check(new ScopedRefPtrToSelf()); EXPECT_FALSE(ScopedRefPtrToSelf::was_destroyed()); - check->self_ptr_ = nullptr; + check = nullptr; EXPECT_TRUE(ScopedRefPtrToSelf::was_destroyed()); } @@ -162,12 +161,12 @@ TEST(RefCountedUnitTest, ScopedRefPtrToSelfMoveAssignment) { ScopedRefPtrToSelf::reset_was_destroyed(); - ScopedRefPtrToSelf *check = new ScopedRefPtrToSelf(); + dsn::ref_ptr check(new ScopedRefPtrToSelf()); EXPECT_FALSE(ScopedRefPtrToSelf::was_destroyed()); // Releasing |check->self_ptr_| will delete |check|. // The move assignment operator must assign |check->self_ptr_| first then // release |check->self_ptr_|. - check->self_ptr_ = dsn::ref_ptr(); + check = dsn::ref_ptr(); EXPECT_TRUE(ScopedRefPtrToSelf::was_destroyed()); } From 7fb53ffb5be5441563c8c578d00887b855bf026c Mon Sep 17 00:00:00 2001 From: JiaShuo Date: Thu, 19 Dec 2019 10:45:33 +0800 Subject: [PATCH 2/6] format --- src/core/tests/autoref_ptr_test.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/tests/autoref_ptr_test.cpp b/src/core/tests/autoref_ptr_test.cpp index aa2df64c4a..d1e8804972 100644 --- a/src/core/tests/autoref_ptr_test.cpp +++ b/src/core/tests/autoref_ptr_test.cpp @@ -40,7 +40,6 @@ class ScopedRefPtrToSelf : public dsn::ref_counter static void reset_was_destroyed() { was_destroyed_ = false; } - private: friend class dsn::ref_counter; ~ScopedRefPtrToSelf() { was_destroyed_ = true; } From c2e7e943415661c2374fe55c4072f685904209ab Mon Sep 17 00:00:00 2001 From: JiaShuo Date: Thu, 19 Dec 2019 13:26:42 +0800 Subject: [PATCH 3/6] fix --- include/dsn/utility/autoref_ptr.h | 41 +++++++++++++++++++---------- src/core/tests/autoref_ptr_test.cpp | 12 +++++---- 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/include/dsn/utility/autoref_ptr.h b/include/dsn/utility/autoref_ptr.h index c525574cb6..1ab6ecdab9 100644 --- a/include/dsn/utility/autoref_ptr.h +++ b/include/dsn/utility/autoref_ptr.h @@ -148,16 +148,17 @@ class ref_ptr if (_obj == obj) return *this; - if (nullptr != _obj) { - _obj->release_ref(); - } - + T *old = _obj; _obj = obj; - if (obj != nullptr) { + if (_obj != nullptr) { _obj->add_ref(); } + if (old != nullptr) { + old->release_ref(); + } + return *this; } @@ -166,13 +167,18 @@ class ref_ptr { if (_obj == obj) return *this; - if (nullptr != _obj) { - _obj->release_ref(); - } + + T *old = _obj; _obj = obj; + if (_obj != nullptr) { _obj->add_ref(); } + + if (old != nullptr) { + old->release_ref(); + } + return *this; } @@ -190,23 +196,30 @@ class ref_ptr return *this; } - if (nullptr != _obj) { - _obj->release_ref(); - } + T *old = _obj; _obj = obj._obj; obj._obj = nullptr; + + if (old != nullptr) { + old->release_ref(); + } + return *this; } template ref_ptr &operator=(ref_ptr &&obj) { - if (nullptr != _obj) { - _obj->release_ref(); - } + T *old = _obj; + _obj = obj._obj; obj._obj = nullptr; + + if (old != nullptr) { + old->release_ref(); + } + return *this; } diff --git a/src/core/tests/autoref_ptr_test.cpp b/src/core/tests/autoref_ptr_test.cpp index d1e8804972..66720c6716 100644 --- a/src/core/tests/autoref_ptr_test.cpp +++ b/src/core/tests/autoref_ptr_test.cpp @@ -34,12 +34,14 @@ class Derived : public SelfAssign class ScopedRefPtrToSelf : public dsn::ref_counter { public: - ScopedRefPtrToSelf() {} + ScopedRefPtrToSelf() : self_ptr_(this) {} static bool was_destroyed() { return was_destroyed_; } static void reset_was_destroyed() { was_destroyed_ = false; } + dsn::ref_ptr self_ptr_; + private: friend class dsn::ref_counter; ~ScopedRefPtrToSelf() { was_destroyed_ = true; } @@ -150,9 +152,9 @@ TEST(RefCountedUnitTest, ScopedRefPtrToSelfPointerAssignment) { ScopedRefPtrToSelf::reset_was_destroyed(); - dsn::ref_ptr check(new ScopedRefPtrToSelf()); + ScopedRefPtrToSelf *check = new ScopedRefPtrToSelf(); EXPECT_FALSE(ScopedRefPtrToSelf::was_destroyed()); - check = nullptr; + check->self_ptr_ = nullptr; EXPECT_TRUE(ScopedRefPtrToSelf::was_destroyed()); } @@ -160,12 +162,12 @@ TEST(RefCountedUnitTest, ScopedRefPtrToSelfMoveAssignment) { ScopedRefPtrToSelf::reset_was_destroyed(); - dsn::ref_ptr check(new ScopedRefPtrToSelf()); + ScopedRefPtrToSelf *check = new ScopedRefPtrToSelf(); EXPECT_FALSE(ScopedRefPtrToSelf::was_destroyed()); // Releasing |check->self_ptr_| will delete |check|. // The move assignment operator must assign |check->self_ptr_| first then // release |check->self_ptr_|. - check = dsn::ref_ptr(); + check->self_ptr_ = dsn::ref_ptr(); EXPECT_TRUE(ScopedRefPtrToSelf::was_destroyed()); } From b10f626b61904ad8e5072be61f07bb7356156f23 Mon Sep 17 00:00:00 2001 From: JiaShuo Date: Thu, 19 Dec 2019 18:09:44 +0800 Subject: [PATCH 4/6] fix base google code --- include/dsn/utility/autoref_ptr.h | 84 +++---------------------------- 1 file changed, 8 insertions(+), 76 deletions(-) diff --git a/include/dsn/utility/autoref_ptr.h b/include/dsn/utility/autoref_ptr.h index 1ab6ecdab9..caa2f1288a 100644 --- a/include/dsn/utility/autoref_ptr.h +++ b/include/dsn/utility/autoref_ptr.h @@ -120,7 +120,8 @@ class ref_ptr _obj->add_ref(); } - template + template ::value>::type> ref_ptr(const ref_ptr &r) { _obj = r.get(); @@ -130,7 +131,8 @@ class ref_ptr ref_ptr(ref_ptr &&r) : _obj(r._obj) { r._obj = nullptr; } - template + template ::value>::type> ref_ptr(ref_ptr &&r) : _obj(r._obj) { r._obj = nullptr; @@ -143,85 +145,15 @@ class ref_ptr } } - ref_ptr &operator=(T *obj) - { - if (_obj == obj) - return *this; - - T *old = _obj; - _obj = obj; - - if (_obj != nullptr) { - _obj->add_ref(); - } - - if (old != nullptr) { - old->release_ref(); - } - - return *this; - } + ref_ptr &operator=(T *obj) { return *this = ref_ptr(obj); } - template - ref_ptr &operator=(U *obj) + ref_ptr &operator=(ref_ptr r) noexcept { - if (_obj == obj) - return *this; - - T *old = _obj; - _obj = obj; - - if (_obj != nullptr) { - _obj->add_ref(); - } - - if (old != nullptr) { - old->release_ref(); - } - + swap(r); return *this; } - ref_ptr &operator=(const ref_ptr &obj) { return operator=(obj._obj); } - - template - ref_ptr &operator=(const ref_ptr &obj) - { - return operator=(obj._obj); - } - - ref_ptr &operator=(ref_ptr &&obj) - { - if (this == &obj) { - return *this; - } - - T *old = _obj; - - _obj = obj._obj; - obj._obj = nullptr; - - if (old != nullptr) { - old->release_ref(); - } - - return *this; - } - - template - ref_ptr &operator=(ref_ptr &&obj) - { - T *old = _obj; - - _obj = obj._obj; - obj._obj = nullptr; - - if (old != nullptr) { - old->release_ref(); - } - - return *this; - } + void swap(ref_ptr &r) noexcept { std::swap(_obj, r._obj); } T *get() const { return _obj; } From a65d9885a5bcc363d102c10449439ea562b9f3d2 Mon Sep 17 00:00:00 2001 From: JiaShuo Date: Thu, 19 Dec 2019 19:50:35 +0800 Subject: [PATCH 5/6] fix bug --- include/dsn/utility/autoref_ptr.h | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/include/dsn/utility/autoref_ptr.h b/include/dsn/utility/autoref_ptr.h index caa2f1288a..8cd20c1f45 100644 --- a/include/dsn/utility/autoref_ptr.h +++ b/include/dsn/utility/autoref_ptr.h @@ -35,8 +35,10 @@ #pragma once +#include #include #include +#include namespace dsn { class ref_counter @@ -106,13 +108,6 @@ class ref_ptr _obj->add_ref(); } - template - ref_ptr(U *obj) : _obj(obj) - { - if (nullptr != _obj) - _obj->add_ref(); - } - ref_ptr(const ref_ptr &r) { _obj = r.get(); @@ -133,7 +128,7 @@ class ref_ptr template ::value>::type> - ref_ptr(ref_ptr &&r) : _obj(r._obj) + ref_ptr(ref_ptr &&r) noexcept : _obj(r._obj) { r._obj = nullptr; } @@ -147,13 +142,22 @@ class ref_ptr ref_ptr &operator=(T *obj) { return *this = ref_ptr(obj); } - ref_ptr &operator=(ref_ptr r) noexcept + ref_ptr &operator=(ref_ptr r) noexcept { swap(r); return *this; } - void swap(ref_ptr &r) noexcept { std::swap(_obj, r._obj); } + template ::value>::type> + ref_ptr &operator=(ref_ptr r) noexcept + { + ref_ptr p(r); + swap(p); + return *this; + } + + void swap(ref_ptr &r) noexcept { std::swap(_obj, r._obj); } T *get() const { return _obj; } From 2dd62520169c1c3efcf820d21484e6635fe340b5 Mon Sep 17 00:00:00 2001 From: JiaShuo Date: Thu, 19 Dec 2019 19:56:44 +0800 Subject: [PATCH 6/6] fix bug --- include/dsn/utility/autoref_ptr.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/dsn/utility/autoref_ptr.h b/include/dsn/utility/autoref_ptr.h index 8cd20c1f45..a08ebc6ee4 100644 --- a/include/dsn/utility/autoref_ptr.h +++ b/include/dsn/utility/autoref_ptr.h @@ -35,10 +35,10 @@ #pragma once -#include #include #include #include +#include namespace dsn { class ref_counter