diff --git a/include/core/SkRefCnt.h b/include/core/SkRefCnt.h index d33117751e..c3f724fa8d 100644 --- a/include/core/SkRefCnt.h +++ b/include/core/SkRefCnt.h @@ -11,6 +11,7 @@ #include "../private/SkAtomics.h" #include "../private/SkUniquePtr.h" #include "SkTypes.h" +#include #include /** \class SkRefCntBase @@ -241,6 +242,8 @@ template class sk_sp { /** Supports safe bool idiom. Obsolete with explicit operator bool. */ using unspecified_bool_type = T* sk_sp::*; public: + using element_type = T; + sk_sp() : fPtr(nullptr) {} sk_sp(std::nullptr_t) : fPtr(nullptr) {} @@ -249,8 +252,7 @@ public: * created sk_sp both have a reference to it. */ sk_sp(const sk_sp& that) : fPtr(SkSafeRef(that.get())) {} - template ::value>> + template ::value>> sk_sp(const sk_sp& that) : fPtr(SkSafeRef(that.get())) {} /** @@ -259,8 +261,7 @@ public: * No call to ref() or unref() will be made. */ sk_sp(sk_sp&& that) : fPtr(that.release()) {} - template ::value>> + template ::value>> sk_sp(sk_sp&& that) : fPtr(that.release()) {} /** @@ -274,6 +275,7 @@ public: */ ~sk_sp() { SkSafeUnref(fPtr); + SkDEBUGCODE(fPtr = nullptr); } sk_sp& operator=(std::nullptr_t) { this->reset(); return *this; } @@ -287,8 +289,7 @@ public: this->reset(SkSafeRef(that.get())); return *this; } - template ::value>> + template ::value>> sk_sp& operator=(const sk_sp& that) { this->reset(SkSafeRef(that.get())); return *this; @@ -303,21 +304,12 @@ public: this->reset(that.release()); return *this; } - template ::value>> + template ::value>> sk_sp& operator=(sk_sp&& that) { this->reset(that.release()); return *this; } - bool operator==(std::nullptr_t) const { return this->get() == nullptr; } - bool operator!=(std::nullptr_t) const { return this->get() != nullptr; } - - template - bool operator==(const sk_sp& that) const { return this->get() == that.get(); } - template - bool operator!=(const sk_sp& that) const { return this->get() != that.get(); } - T& operator*() const { SkASSERT(this->get() != nullptr); return *this->get(); @@ -338,8 +330,12 @@ public: * No call to ref() will be made. */ void reset(T* ptr = nullptr) { - SkSafeUnref(fPtr); + // Calling fPtr->unref() may call this->~() or this->reset(T*). + // http://wg21.cmeerw.net/lwg/issue998 + // http://wg21.cmeerw.net/lwg/issue2262 + T* oldPtr = fPtr; fPtr = ptr; + SkSafeUnref(oldPtr); } /** @@ -353,10 +349,82 @@ public: return ptr; } + void swap(sk_sp& that) /*noexcept*/ { + using std::swap; + swap(fPtr, that.fPtr); + } + private: T* fPtr; }; +template inline void swap(sk_sp& a, sk_sp& b) /*noexcept*/ { + a.swap(b); +} + +template inline bool operator==(const sk_sp& a, const sk_sp& b) { + return a.get() == b.get(); +} +template inline bool operator==(const sk_sp& a, std::nullptr_t) /*noexcept*/ { + return !a; +} +template inline bool operator==(std::nullptr_t, const sk_sp& b) /*noexcept*/ { + return !b; +} + +template inline bool operator!=(const sk_sp& a, const sk_sp& b) { + return a.get() != b.get(); +} +template inline bool operator!=(const sk_sp& a, std::nullptr_t) /*noexcept*/ { + return static_cast(a); +} +template inline bool operator!=(std::nullptr_t, const sk_sp& b) /*noexcept*/ { + return static_cast(b); +} + +template inline bool operator<(const sk_sp& a, const sk_sp& b) { + // Provide defined total order on sk_sp. + // http://wg21.cmeerw.net/lwg/issue1297 + // http://wg21.cmeerw.net/lwg/issue1401 . + return std::less>()(a.get(), b.get()); +} +template inline bool operator<(const sk_sp& a, std::nullptr_t) { + return std::less()(a.get(), nullptr); +} +template inline bool operator<(std::nullptr_t, const sk_sp& b) { + return std::less()(nullptr, b.get()); +} + +template inline bool operator<=(const sk_sp& a, const sk_sp& b) { + return !(b < a); +} +template inline bool operator<=(const sk_sp& a, std::nullptr_t) { + return !(nullptr < a); +} +template inline bool operator<=(std::nullptr_t, const sk_sp& b) { + return !(b < nullptr); +} + +template inline bool operator>(const sk_sp& a, const sk_sp& b) { + return b < a; +} +template inline bool operator>(const sk_sp& a, std::nullptr_t) { + return nullptr < a; +} +template inline bool operator>(std::nullptr_t, const sk_sp& b) { + return b < nullptr; +} + +template inline bool operator>=(const sk_sp& a, const sk_sp& b) { + return !(a < b); +} +template inline bool operator>=(const sk_sp& a, std::nullptr_t) { + return !(a < nullptr); +} +template inline bool operator>=(std::nullptr_t, const sk_sp& b) { + return !(nullptr < b); +} + template sk_sp sk_make_sp(Args&&... args) { return sk_sp(new T(std::forward(args)...)); diff --git a/include/private/SkTLogic.h b/include/private/SkTLogic.h index a11953705c..b38fd50435 100644 --- a/include/private/SkTLogic.h +++ b/include/private/SkTLogic.h @@ -62,6 +62,8 @@ template using add_cv_t = typename std::add_cv::type; template using add_pointer_t = typename std::add_pointer::type; template using add_lvalue_reference_t = typename std::add_lvalue_reference::type; +template using common_type_t = typename std::common_type::type; + template ::value || is_function::value || std::is_array::value> struct is_convertible_detector { diff --git a/tests/RefCntTest.cpp b/tests/RefCntTest.cpp index d3cda7f38d..2932913c3d 100644 --- a/tests/RefCntTest.cpp +++ b/tests/RefCntTest.cpp @@ -275,6 +275,80 @@ DEF_TEST(sk_sp, reporter) { check(reporter, 1, 1, 1, 0); paint.set(nullptr); check(reporter, 1, 2, 1, 1); + + { + sk_sp empty; + sk_sp notEmpty = sk_make_sp(); + REPORTER_ASSERT(reporter, empty == sk_sp()); + + REPORTER_ASSERT(reporter, notEmpty != empty); + REPORTER_ASSERT(reporter, empty != notEmpty); + + REPORTER_ASSERT(reporter, nullptr == empty); + REPORTER_ASSERT(reporter, empty == nullptr); + REPORTER_ASSERT(reporter, empty == empty); + + REPORTER_ASSERT(reporter, nullptr <= empty); + REPORTER_ASSERT(reporter, empty <= nullptr); + REPORTER_ASSERT(reporter, empty <= empty); + + REPORTER_ASSERT(reporter, nullptr >= empty); + REPORTER_ASSERT(reporter, empty >= nullptr); + REPORTER_ASSERT(reporter, empty >= empty); + } + + { + sk_sp a = sk_make_sp(); + sk_sp b = sk_make_sp(); + REPORTER_ASSERT(reporter, a != b); + REPORTER_ASSERT(reporter, (a < b) != (b < a)); + REPORTER_ASSERT(reporter, (b > a) != (a > b)); + REPORTER_ASSERT(reporter, (a <= b) != (b <= a)); + REPORTER_ASSERT(reporter, (b >= a) != (a >= b)); + + REPORTER_ASSERT(reporter, a == a); + REPORTER_ASSERT(reporter, a <= a); + REPORTER_ASSERT(reporter, a >= a); + } + + // http://wg21.cmeerw.net/lwg/issue998 + { + class foo : public SkRefCnt { + public: + foo() : bar(this) {} + void reset() { bar.reset(); } + private: + sk_sp bar; + }; + // The following should properly delete the object and not cause undefined behavior. + // This is an ugly example, but the same issue can arise in more subtle ways. + (new foo)->reset(); + } + + // https://crrev.com/0d4ef2583a6f19c3e61be04d36eb1a60b133832c + { + struct StructB; + struct StructA : public SkRefCnt { + sk_sp b; + }; + + struct StructB : public SkRefCnt { + sk_sp a; + ~StructB() override {}; // Some clang versions don't emit this implicitly. + }; + + // Create a reference cycle. + StructA* a = new StructA; + a->b.reset(new StructB); + a->b->a.reset(a); + + // Break the cycle by calling reset(). This will cause |a| (and hence, |a.b|) + // to be deleted before the call to reset() returns. This tests that the + // implementation of sk_sp::reset() doesn't access |this| after it + // deletes the underlying pointer. This behaviour is consistent with the + // definition of unique_ptr::reset in C++11. + a->b.reset(); + } } namespace {