diff options
author | Murray Cumming <murrayc@murrayc.com> | 2016-04-10 11:59:00 +0200 |
---|---|---|
committer | Murray Cumming <murrayc@murrayc.com> | 2016-12-11 21:43:40 +0100 |
commit | 8d7a0dc8428677def97b5f7b68c690830c244192 (patch) | |
tree | 91e0f8d060f6d929fea9e20949e7682fe0bb40b9 | |
parent | ac50f9c7fc7624d578a0656ae535ba05cd4c7bc4 (diff) | |
download | glibmm-8d7a0dc8428677def97b5f7b68c690830c244192.tar.gz |
RefPtr: Make this an alias for shared_ptr<> instead.
Specifying a Deleter in make_refptr_for_instance().
And changing RefPtr::cast_dynamic<>() to std::dynamic_pointer_cast<>().
The glibmm_refptr test then becomes rather silly, and should maybe
just be removed.
-rw-r--r-- | glib/glibmm/object.h | 4 | ||||
-rw-r--r-- | glib/glibmm/refptr.h | 479 | ||||
-rw-r--r-- | glib/glibmm/value.h | 4 | ||||
-rw-r--r-- | glib/glibmm/weakref.h | 6 | ||||
-rw-r--r-- | tests/giomm_tls_client/main.cc | 2 | ||||
-rw-r--r-- | tests/glibmm_refptr/main.cc | 38 |
6 files changed, 54 insertions, 479 deletions
diff --git a/glib/glibmm/object.h b/glib/glibmm/object.h index 9b4dda29..181664e0 100644 --- a/glib/glibmm/object.h +++ b/glib/glibmm/object.h @@ -257,7 +257,7 @@ public: static GType value_type() { return T::get_base_type(); } - void set(const CppType& data) { set_object(data.operator->()); } + void set(const CppType& data) { set_object(data.get()); } CppType get() const { return Glib::RefPtr<T>::cast_dynamic(get_object_copy()); } }; @@ -276,7 +276,7 @@ public: static GType value_type() { return T::get_base_type(); } - void set(const CppType& data) { set_object(const_cast<T*>(data.operator->())); } + void set(const CppType& data) { set_object(const_cast<T*>(data.get())); } CppType get() const { return Glib::RefPtr<T>::cast_dynamic(get_object_copy()); } }; #endif /* GLIBMM_HAVE_DISAMBIGUOUS_CONST_TEMPLATE_SPECIALIZATIONS */ diff --git a/glib/glibmm/refptr.h b/glib/glibmm/refptr.h index 4ded0b7a..eee7f176 100644 --- a/glib/glibmm/refptr.h +++ b/glib/glibmm/refptr.h @@ -20,11 +20,22 @@ #include <glibmmconfig.h> #include <glib.h> -#include <utility> +#include <memory> namespace Glib { +class ObjectBase; + +template <class T_CppObject> +void RefPtrDeleter(T_CppObject* object) +{ + if (!object) + return; + + object->unreference(); +} + /** RefPtr<> is a reference-counting shared smartpointer. * * Some objects in gtkmm are obtained from a shared @@ -49,467 +60,27 @@ namespace Glib * book for further information. */ template <class T_CppObject> -class RefPtr -{ -private: -#ifndef DOXYGEN_SHOULD_SKIP_THIS - /** Helper class for disallowing use of Glib::RefPtr with certain classes. - * - * Disallow for instance in Gtk::Widget and its subclasses. - * Glib::RefPtr<T>::is_allowed_type::value is false if - * T:dont_allow_use_in_glib_refptr_ is a public type, else it's true. - * Example: - * @code - * using dont_allow_use_in_glib_refptr_ = int; - * @endcode - */ - class is_allowed_type - { - private: - struct big - { - int memory[64]; - }; - - static big check(...); - - // If X::dont_allow_use_in_glib_refptr_ is not a type, this check() overload - // is ignored because of the SFINAE rule (Substitution Failure Is Not An Error). - template <typename X> - static typename X::dont_allow_use_in_glib_refptr_ check(X* obj); - - public: - static const bool value = sizeof(check(static_cast<T_CppObject*>(nullptr))) == sizeof(big); - }; - - static_assert(is_allowed_type::value, "Glib::RefPtr must not be used with this class."); -#endif /* DOXYGEN_SHOULD_SKIP_THIS */ - -public: - /** Default constructor - * - * Afterwards it will be null and use of -> will cause a segmentation fault. - */ - inline RefPtr() noexcept; - - /// Destructor - decrements reference count. - inline ~RefPtr() noexcept; - - /// For use only by the \::create() methods. - explicit inline RefPtr(T_CppObject* pCppObject) noexcept; - - /** Copy constructor - * - * This increments the shared reference count. - */ - inline RefPtr(const RefPtr& src) noexcept; - - /** Move constructor - */ - inline RefPtr(RefPtr&& src) noexcept; - - /** Move constructor (from different, but castable type). - */ - template <class T_CastFrom> - inline RefPtr(RefPtr<T_CastFrom>&& src) noexcept; - - /** Copy constructor (from different, but castable type). - * - * Increments the reference count. - */ - template <class T_CastFrom> - inline RefPtr(const RefPtr<T_CastFrom>& src) noexcept; - - /** Swap the contents of two RefPtr<>. - * This method swaps the internal pointers to T_CppObject. This can be - * done safely without involving a reference/unreference cycle and is - * therefore highly efficient. - */ - inline void swap(RefPtr& other) noexcept; - - /// Copy from another RefPtr: - inline RefPtr& operator=(const RefPtr& src) noexcept; - - /// Move assignment operator: - inline RefPtr& operator=(RefPtr&& src) noexcept; - - /// Move assignment operator (from different, but castable type): - template <class T_CastFrom> - inline RefPtr& operator=(RefPtr<T_CastFrom>&& src) noexcept; - - /** Copy from different, but castable type). - * - * Increments the reference count. - */ - template <class T_CastFrom> - inline RefPtr& operator=(const RefPtr<T_CastFrom>& src) noexcept; - - /// Tests whether the RefPtr<> point to the same underlying instance. - inline bool operator==(const RefPtr& src) const noexcept; - - /// See operator==(). - inline bool operator!=(const RefPtr& src) const noexcept; - - /** Dereferencing. - * - * Use the methods of the underlying instance like so: - * <code>refptr->memberfun()</code>. - */ - inline T_CppObject* operator->() const noexcept; - - /** Test whether the RefPtr<> points to any underlying instance. - * - * Mimics usage of ordinary pointers: - * @code - * if (ptr) - * do_something(); - * @endcode - */ - inline explicit operator bool() const noexcept; - -#ifndef GLIBMM_DISABLE_DEPRECATED - /// @deprecated Use reset() instead because this leads to confusion with clear() methods on the - /// underlying class. For instance, people use .clear() when they mean ->clear(). - inline void clear() noexcept; -#endif // GLIBMM_DISABLE_DEPRECATED - - /** Set underlying instance to nullptr, decrementing reference count of existing instance - * appropriately. - * @newin{2,16} - */ - inline void reset() noexcept; - - /** Release the ownership of underlying instance. - * - * RefPtr's underlying instance is set to nullptr, therefore underlying object can't be accessed - * through this RefPtr anymore. - * @return an underlying instance. - * - * Most users should not use release(). It can spoil the automatic destruction - * of the managed object. A legitimate use is if you immediately give RefPtr's - * reference to another object. - */ - inline T_CppObject* release() noexcept G_GNUC_WARN_UNUSED_RESULT; - - /** Dynamic cast to derived class. - * - * The RefPtr can't be cast with the usual notation so instead you can use - * @code - * ptr_derived = RefPtr<Derived>::cast_dynamic(ptr_base); - * @endcode - */ - template <class T_CastFrom> - static inline RefPtr cast_dynamic(const RefPtr<T_CastFrom>& src) noexcept; - - /** Static cast to derived class. - * - * Like the dynamic cast; the notation is - * @code - * ptr_derived = RefPtr<Derived>::cast_static(ptr_base); - * @endcode - */ - template <class T_CastFrom> - static inline RefPtr cast_static(const RefPtr<T_CastFrom>& src) noexcept; - - /** Cast to non-const. - * - * The RefPtr can't be cast with the usual notation so instead you can use - * @code - * ptr_unconst = RefPtr<UnConstType>::cast_const(ptr_const); - * @endcode - */ - template <class T_CastFrom> - static inline RefPtr cast_const(const RefPtr<T_CastFrom>& src) noexcept; - - /** Compare based on the underlying instance address. - * - * This is needed in code that requires an ordering on - * RefPtr<T_CppObject> instances, e.g. std::set<RefPtr<T_CppObject> >. - * - * Without these, comparing two RefPtr<T_CppObject> instances - * is still syntactically possible, but the result is semantically - * wrong, as p1 REL_OP p2 is interpreted as (bool)p1 REL_OP (bool)p2. - */ - inline bool operator<(const RefPtr& src) const noexcept; - - /// See operator<(). - inline bool operator<=(const RefPtr& src) const noexcept; - - /// See operator<(). - inline bool operator>(const RefPtr& src) const noexcept; - - /// See operator<(). - inline bool operator>=(const RefPtr& src) const noexcept; - -private: - T_CppObject* pCppObject_; -}; - -#ifndef DOXYGEN_SHOULD_SKIP_THIS - -// RefPtr<>::operator->() comes first here since it's used by other methods. -// If it would come after them it wouldn't be inlined. - -template <class T_CppObject> -inline T_CppObject* RefPtr<T_CppObject>::operator->() const noexcept -{ - return pCppObject_; -} - -template <class T_CppObject> -inline RefPtr<T_CppObject>::RefPtr() noexcept : pCppObject_(nullptr) -{ -} - -template <class T_CppObject> -inline RefPtr<T_CppObject>::~RefPtr() noexcept -{ - if (pCppObject_) - pCppObject_->unreference(); // This could cause pCppObject to be deleted. -} - -template <class T_CppObject> -inline RefPtr<T_CppObject>::RefPtr(T_CppObject* pCppObject) noexcept : pCppObject_(pCppObject) -{ -} - -template <class T_CppObject> -inline RefPtr<T_CppObject>::RefPtr(const RefPtr& src) noexcept : pCppObject_(src.pCppObject_) -{ - if (pCppObject_) - pCppObject_->reference(); -} - -template <class T_CppObject> -inline RefPtr<T_CppObject>::RefPtr(RefPtr&& src) noexcept : pCppObject_(src.pCppObject_) -{ - src.pCppObject_ = nullptr; -} - -template <class T_CppObject> -template <class T_CastFrom> -inline RefPtr<T_CppObject>::RefPtr(RefPtr<T_CastFrom>&& src) noexcept : pCppObject_(src.release()) -{ -} - -// The templated ctor allows copy construction from any object that's -// castable. Thus, it does downcasts: -// base_ref = derived_ref -template <class T_CppObject> -template <class T_CastFrom> -inline RefPtr<T_CppObject>::RefPtr(const RefPtr<T_CastFrom>& src) noexcept : - // A different RefPtr<> will not allow us access to pCppObject_. We need - // to add a get_underlying() for this, but that would encourage incorrect - // use, so we use the less well-known operator->() accessor: - pCppObject_(src.operator->()) -{ - if (pCppObject_) - pCppObject_->reference(); -} - -template <class T_CppObject> -inline void -RefPtr<T_CppObject>::swap(RefPtr& other) noexcept -{ - T_CppObject* const temp = pCppObject_; - pCppObject_ = other.pCppObject_; - other.pCppObject_ = temp; -} - -template <class T_CppObject> -inline RefPtr<T_CppObject>& -RefPtr<T_CppObject>::operator=(const RefPtr& src) noexcept -{ - // In case you haven't seen the swap() technique to implement copy - // assignment before, here's what it does: - // - // 1) Create a temporary RefPtr<> instance via the copy ctor, thereby - // increasing the reference count of the source object. - // - // 2) Swap the internal object pointers of *this and the temporary - // RefPtr<>. After this step, *this already contains the new pointer, - // and the old pointer is now managed by temp. - // - // 3) The destructor of temp is executed, thereby unreferencing the - // old object pointer. - // - // This technique is described in Herb Sutter's "Exceptional C++", and - // has a number of advantages over conventional approaches: - // - // - Code reuse by calling the copy ctor. - // - Strong exception safety for free. - // - Self assignment is handled implicitely. - // - Simplicity. - // - It just works and is hard to get wrong; i.e. you can use it without - // even thinking about it to implement copy assignment whereever the - // object data is managed indirectly via a pointer, which is very common. +using RefPtr = std::shared_ptr<T_CppObject>; - RefPtr<T_CppObject> temp(src); - this->swap(temp); - return *this; -} - -template <class T_CppObject> -inline RefPtr<T_CppObject>& -RefPtr<T_CppObject>::operator=(RefPtr&& src) noexcept -{ - RefPtr<T_CppObject> temp(std::move(src)); - this->swap(temp); - src.pCppObject_ = nullptr; - - return *this; -} - -template <class T_CppObject> -template <class T_CastFrom> -inline RefPtr<T_CppObject>& -RefPtr<T_CppObject>::operator=(RefPtr<T_CastFrom>&& src) noexcept -{ - if (pCppObject_) - pCppObject_->unreference(); - pCppObject_ = src.release(); - - return *this; -} - -template <class T_CppObject> -template <class T_CastFrom> -inline RefPtr<T_CppObject>& -RefPtr<T_CppObject>::operator=(const RefPtr<T_CastFrom>& src) noexcept -{ - RefPtr<T_CppObject> temp(src); - this->swap(temp); - return *this; -} - -template <class T_CppObject> -inline bool -RefPtr<T_CppObject>::operator==(const RefPtr& src) const noexcept -{ - return (pCppObject_ == src.pCppObject_); -} - -template <class T_CppObject> -inline bool -RefPtr<T_CppObject>::operator!=(const RefPtr& src) const noexcept -{ - return (pCppObject_ != src.pCppObject_); -} - -template <class T_CppObject> -inline RefPtr<T_CppObject>::operator bool() const noexcept -{ - return (pCppObject_ != nullptr); -} - -#ifndef GLIBMM_DISABLE_DEPRECATED -template <class T_CppObject> -inline void -RefPtr<T_CppObject>::clear() noexcept -{ - reset(); -} -#endif // GLIBMM_DISABLE_DEPRECATED - -template <class T_CppObject> -inline void -RefPtr<T_CppObject>::reset() noexcept -{ - RefPtr<T_CppObject> temp; // swap with an empty RefPtr<> to clear *this - this->swap(temp); -} - -template <class T_CppObject> -inline T_CppObject* -RefPtr<T_CppObject>::release() noexcept -{ - T_CppObject* tmp = pCppObject_; - pCppObject_ = nullptr; - return tmp; -} - -template <class T_CppObject> -template <class T_CastFrom> -inline RefPtr<T_CppObject> -RefPtr<T_CppObject>::cast_dynamic(const RefPtr<T_CastFrom>& src) noexcept -{ - T_CppObject* const pCppObject = dynamic_cast<T_CppObject*>(src.operator->()); - - if (pCppObject) - pCppObject->reference(); - - return RefPtr<T_CppObject>(pCppObject); -} - -template <class T_CppObject> -template <class T_CastFrom> -inline RefPtr<T_CppObject> -RefPtr<T_CppObject>::cast_static(const RefPtr<T_CastFrom>& src) noexcept -{ - T_CppObject* const pCppObject = static_cast<T_CppObject*>(src.operator->()); - - if (pCppObject) - pCppObject->reference(); - - return RefPtr<T_CppObject>(pCppObject); -} - -template <class T_CppObject> -template <class T_CastFrom> -inline RefPtr<T_CppObject> -RefPtr<T_CppObject>::cast_const(const RefPtr<T_CastFrom>& src) noexcept -{ - T_CppObject* const pCppObject = const_cast<T_CppObject*>(src.operator->()); - - if (pCppObject) - pCppObject->reference(); - - return RefPtr<T_CppObject>(pCppObject); -} - -template <class T_CppObject> -inline bool -RefPtr<T_CppObject>::operator<(const RefPtr& src) const noexcept -{ - return (pCppObject_ < src.pCppObject_); -} - -template <class T_CppObject> -inline bool -RefPtr<T_CppObject>::operator<=(const RefPtr& src) const noexcept -{ - return (pCppObject_ <= src.pCppObject_); -} - -template <class T_CppObject> -inline bool -RefPtr<T_CppObject>::operator>(const RefPtr& src) const noexcept -{ - return (pCppObject_ > src.pCppObject_); -} - -template <class T_CppObject> -inline bool -RefPtr<T_CppObject>::operator>=(const RefPtr& src) const noexcept -{ - return (pCppObject_ >= src.pCppObject_); -} - -#endif /* DOXYGEN_SHOULD_SKIP_THIS */ - -/** @relates Glib::RefPtr */ -template <class T_CppObject> -inline void -swap(RefPtr<T_CppObject>& lhs, RefPtr<T_CppObject>& rhs) noexcept +/** This would not be useful, + * because application code should not new these objects anyway. + * And it is not useful inside glibmm or gtkmm code because + * the constructors are protected, so can't be called from this utilility + * function. + * +template <class T_CppObject, class... T_Arg> +RefPtr<T_CppObject> +make_refptr(T_Arg... arg) { - lhs.swap(rhs); + return RefPtr<T_CppObject>(new T_CppObject(arg...)); } +*/ template <class T_CppObject> RefPtr<T_CppObject> make_refptr_for_instance(T_CppObject* object) { - return RefPtr<T_CppObject>(object); + return RefPtr<T_CppObject>(object, &RefPtrDeleter<T_CppObject>); } } // namespace Glib diff --git a/glib/glibmm/value.h b/glib/glibmm/value.h index 80c885a0..8f983ceb 100644 --- a/glib/glibmm/value.h +++ b/glib/glibmm/value.h @@ -239,7 +239,7 @@ public: static GType value_type() { return T::get_base_type(); } - void set(const CppType& data) { set_object(data.operator->()); } + void set(const CppType& data) { set_object(data.get()); } CppType get() const { return Glib::RefPtr<T>::cast_dynamic(get_object_copy()); } }; @@ -258,7 +258,7 @@ public: static GType value_type() { return T::get_base_type(); } - void set(const CppType& data) { set_object(const_cast<T*>(data.operator->())); } + void set(const CppType& data) { set_object(const_cast<T*>(data.get())); } CppType get() const { return Glib::RefPtr<T>::cast_dynamic(get_object_copy()); } }; #endif // GLIBMM_HAVE_DISAMBIGUOUS_CONST_TEMPLATE_SPECIALIZATIONS diff --git a/glib/glibmm/weakref.h b/glib/glibmm/weakref.h index f4226cf7..99d6c662 100644 --- a/glib/glibmm/weakref.h +++ b/glib/glibmm/weakref.h @@ -247,7 +247,7 @@ WeakRef<T_CppObject>::WeakRef(WeakRef<T_CastFrom>&& src) noexcept : pCppObject_( template <typename T_CppObject> template <typename T_CastFrom> WeakRef<T_CppObject>::WeakRef(const RefPtr<T_CastFrom>& src) noexcept - : pCppObject_(src.operator->()), + : pCppObject_(src.get()), gobject_(nullptr) { if (pCppObject_) @@ -287,8 +287,6 @@ template <typename T_CppObject> WeakRef<T_CppObject>& WeakRef<T_CppObject>::operator=(WeakRef&& src) noexcept { - // See RefPtr for an explanation of the swap() technique to implement - // copy assignment and move assignment. // This technique is inefficient for copy assignment of WeakRef, // because it involves copy construction + destruction, i.e. in a typical // case g_weak_ref_init() + g_weak_ref_clear(), when a g_weak_ref_set() @@ -322,7 +320,7 @@ template <typename T_CastFrom> WeakRef<T_CppObject>& WeakRef<T_CppObject>::operator=(const RefPtr<T_CastFrom>& src) noexcept { - T_CppObject* pCppObject = src.operator->(); + T_CppObject* pCppObject = src.get(); set(pCppObject, nullptr); return *this; } diff --git a/tests/giomm_tls_client/main.cc b/tests/giomm_tls_client/main.cc index 5f133253..beedd4c1 100644 --- a/tests/giomm_tls_client/main.cc +++ b/tests/giomm_tls_client/main.cc @@ -98,7 +98,7 @@ main(int, char**) << address->get_port() << "." << std::endl; } - auto conn = Glib::RefPtr<Gio::TcpConnection>::cast_dynamic(Gio::SocketConnection::create(socket)); + auto conn = std::dynamic_pointer_cast<Gio::TcpConnection>(Gio::SocketConnection::create(socket)); if (!conn || !conn->is_connected()) { diff --git a/tests/glibmm_refptr/main.cc b/tests/glibmm_refptr/main.cc index 793f8af1..050db351 100644 --- a/tests/glibmm_refptr/main.cc +++ b/tests/glibmm_refptr/main.cc @@ -101,16 +101,18 @@ test_refptr_copy_constructor() g_assert_cmpint(refSomething->max_ref_count(), ==, 1); { + //The reference count should not change, + //because we only take and release a single reference: Glib::RefPtr<Something> refSomething2(refSomething); - g_assert_cmpint(refSomething->ref_count(), ==, 2); - g_assert_cmpint(refSomething2->ref_count(), ==, 2); - g_assert_cmpint(refSomething->max_ref_count(), ==, 2); + g_assert_cmpint(refSomething->ref_count(), ==, 1); + g_assert_cmpint(refSomething2->ref_count(), ==, 1); + g_assert_cmpint(refSomething->max_ref_count(), ==, 1); } // Test the refcount after other references should have been released // when other RefPtrs went out of scope: g_assert_cmpint(refSomething->ref_count(), ==, 1); - g_assert_cmpint(refSomething->max_ref_count(), ==, 2); + g_assert_cmpint(refSomething->max_ref_count(), ==, 1); } static void @@ -121,16 +123,18 @@ test_refptr_assignment_operator() g_assert_cmpint(refSomething->max_ref_count(), ==, 1); { + //The reference count should not change, + //because we only take and release a single reference: Glib::RefPtr<Something> refSomething2 = refSomething; - g_assert_cmpint(refSomething->ref_count(), ==, 2); - g_assert_cmpint(refSomething2->ref_count(), ==, 2); - g_assert_cmpint(refSomething->max_ref_count(), ==, 2); + g_assert_cmpint(refSomething->ref_count(), ==, 1); + g_assert_cmpint(refSomething2->ref_count(), ==, 1); + g_assert_cmpint(refSomething->max_ref_count(), ==, 1); } // Test the refcount after other references should have been released // when other RefPtrs went out of scope: g_assert_cmpint(refSomething->ref_count(), ==, 1); - g_assert_cmpint(refSomething->max_ref_count(), ==, 2); + g_assert_cmpint(refSomething->max_ref_count(), ==, 1); } static Glib::RefPtr<Something> @@ -148,23 +152,25 @@ static void test_refptr_with_parent_copy_constructor() { // We use get_something() because test_refptr_with_parent_move_constructor() does. + //The reference count should not change, + //because we only take and release a single reference: Glib::RefPtr<Something> refSomething = get_something(); - g_assert_cmpint(refSomething->ref_count(), ==, 2); // 1 here and 1 inside get_something() - g_assert_cmpint(refSomething->max_ref_count(), ==, 2); + g_assert_cmpint(refSomething->ref_count(), ==, 1); + g_assert_cmpint(refSomething->max_ref_count(), ==, 1); { Parent parent(refSomething); g_assert(!parent.was_constructed_via_move_constructor()); g_assert(parent.was_constructed_via_copy_constructor()); g_assert_cmpint( - parent.something_ref_count(), ==, 3); // 1 here, 1 in parent, and 1 inside get_something() - g_assert_cmpint(parent.something_max_ref_count(), ==, 3); + parent.something_ref_count(), ==, 1); + g_assert_cmpint(parent.something_max_ref_count(), ==, 1); } // Test the refcount after other references should have been released // when other RefPtrs went out of scope: - g_assert_cmpint(refSomething->ref_count(), ==, 2); // 1 here and 1 inside get_something() - g_assert_cmpint(refSomething->max_ref_count(), ==, 3); + g_assert_cmpint(refSomething->ref_count(), ==, 1); + g_assert_cmpint(refSomething->max_ref_count(), ==, 1); } static void @@ -173,8 +179,8 @@ test_refptr_with_parent_move_constructor() Parent parent(get_something()); g_assert(parent.was_constructed_via_move_constructor()); g_assert(!parent.was_constructed_via_copy_constructor()); - g_assert_cmpint(parent.something_ref_count(), ==, 2); // 1 in parent and 1 inside get_something() - g_assert_cmpint(parent.something_max_ref_count(), ==, 2); + g_assert_cmpint(parent.something_ref_count(), ==, 1); + g_assert_cmpint(parent.something_max_ref_count(), ==, 1); } static void |