[Scummvm-git-logs] scummvm master -> 5cb75fdaecdd061f93e45196caaab0b819b2827d

dreammaster noreply at scummvm.org
Tue Apr 26 01:20:27 UTC 2022


This automated email contains information about 8 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
9fa32ac47d COMMON: Fix WeakPtr preventing object deletion
bd23944fd1 COMMON: Fix SharedPtr natvis
be5a7cb8e7 COMMON: Fix up WeakPtr logic
c8d0186752 COMMON: Fix WeakPtr missing copy constructor
22b72dba52 COMMON: Remove BasePtr base class from SharedPtr and WeakPtr.  Fix missing refCount method in SharedPtr.
2432c67426 COMMON: Fix double destruction when using a deleter.
a1f2121bb8 COMMON: Remove empty BasePtrTrackerDeletionImpl destructor
5cb75fdaec COMMON: Remove implicit bool conversion from WeakPtr


Commit: 9fa32ac47dc2a1391d6d0880aaa5069a6c8008ab
    https://github.com/scummvm/scummvm/commit/9fa32ac47dc2a1391d6d0880aaa5069a6c8008ab
Author: elasota (ejlasota at gmail.com)
Date: 2022-04-25T18:20:19-07:00

Commit Message:
COMMON: Fix WeakPtr preventing object deletion

Changed paths:
    common/ptr.h
    test/common/ptr.h


diff --git a/common/ptr.h b/common/ptr.h
index 340e2b230ea..5e27b22d3da 100644
--- a/common/ptr.h
+++ b/common/ptr.h
@@ -40,29 +40,74 @@ namespace Common {
  * @{
  */
 
-class BasePtrDeletionInternal {
+class BasePtrTrackerInternal {
 public:
-	virtual ~BasePtrDeletionInternal() {}
+	typedef int RefValue;
+
+	BasePtrTrackerInternal() : _weakRefCount(1), _strongRefCount(1) {}
+	virtual ~BasePtrTrackerInternal() {}
+
+	void incWeak() {
+		_weakRefCount++;
+	}
+
+	void decWeak() {
+		if (--_weakRefCount == 0)
+			delete this;
+	}
+
+	void incStrong() {
+		_strongRefCount++;
+	}
+
+	void decStrong() {
+		if (--_strongRefCount == 0) {
+			destructObject();
+			decWeak();
+		}
+	}
+
+	bool isAlive() const {
+		return _strongRefCount > 0;
+	}
+
+	RefValue getStrongCount() const {
+		return _strongRefCount;
+	}
+
+protected:
+	virtual void destructObject() = 0;
+
+private:
+	RefValue _weakRefCount; // Weak ref count + 1 if object ref count > 0
+	RefValue _strongRefCount;
 };
 
 template<class T>
-class BasePtrDeletionImpl : public BasePtrDeletionInternal {
+class BasePtrTrackerImpl : public BasePtrTrackerInternal {
 public:
-	BasePtrDeletionImpl(T *ptr) : _ptr(ptr) {}
-	~BasePtrDeletionImpl() {
+	BasePtrTrackerImpl(T *ptr) : _ptr(ptr) {}
+
+protected:
+	void destructObject() override {
 		STATIC_ASSERT(sizeof(T) > 0, SharedPtr_cannot_delete_incomplete_type);
 		delete _ptr;
 	}
-private:
+
 	T *_ptr;
 };
 
 template<class T, class DL>
-class BasePtrDeletionDeleterImpl : public BasePtrDeletionInternal {
+class BasePtrTrackerDeletionImpl : public BasePtrTrackerInternal {
 public:
-	BasePtrDeletionDeleterImpl(T *ptr, DL d) : _ptr(ptr), _deleter(d) {}
-	~BasePtrDeletionDeleterImpl() { _deleter(_ptr); }
+	BasePtrTrackerDeletionImpl(T *ptr, DL d) : _ptr(ptr), _deleter(d) {}
+	~BasePtrTrackerDeletionImpl() { _deleter(_ptr); }
+
 private:
+	void destructObject() override {
+		_deleter(_ptr);
+	}
+
 	T *_ptr;
 	DL _deleter;
 };
@@ -79,36 +124,32 @@ class BasePtr : public SafeBool<BasePtr<T> > {
 	template<class T2> friend class BasePtr;
 #endif
 public:
-	typedef int RefValue;
 	typedef T ValueType;
 	typedef T *PointerType;
 	typedef T &ReferenceType;
 
-	BasePtr() : _refCount(nullptr), _deletion(nullptr), _pointer(nullptr) {
+	BasePtr() : _tracker(nullptr), _pointer(nullptr) {
 	}
 
-	explicit BasePtr(std::nullptr_t) : _refCount(nullptr), _deletion(nullptr), _pointer(nullptr) {
+	explicit BasePtr(std::nullptr_t) : _tracker(nullptr), _pointer(nullptr) {
 	}
 
 	template<class T2>
-	explicit BasePtr(T2 *p) : _refCount(new RefValue(1)), _deletion(new BasePtrDeletionImpl<T2>(p)), _pointer(p) {
+	explicit BasePtr(T2 *p) : _tracker(p ? (new BasePtrTrackerImpl<T2>(p)) : nullptr), _pointer(p) {
 	}
 
 	template<class T2, class DL>
-	BasePtr(T2 *p, DL d) : _refCount(new RefValue(1)), _deletion(new BasePtrDeletionDeleterImpl<T2, DL>(p, d)), _pointer(p) {
+	BasePtr(T2 *p, DL d) : _tracker(p ? new BasePtrTrackerDeletionImpl<T2, DL>(p, d) : nullptr), _pointer(p) {
 	}
 
-	BasePtr(const BasePtr &r) : _refCount(r._refCount), _deletion(r._deletion), _pointer(r._pointer) {
-		if (_refCount)
-			++(*_refCount);
+	BasePtr(const BasePtr &r) : _tracker(r._tracker), _pointer(r._pointer) {
 	}
+
 	template<class T2>
-	BasePtr(const BasePtr<T2> &r) : _refCount(r._refCount), _deletion(r._deletion), _pointer(r._pointer) {
-		if (_refCount) ++(*_refCount);
+	BasePtr(const BasePtr<T2> &r) : _tracker(r._tracker), _pointer(r._pointer) {
 	}
 
 	~BasePtr() {
-		decRef();
 	}
 
 	/**
@@ -119,114 +160,15 @@ public:
 		return _pointer != nullptr;
 	}
 
-	/**
-	 * Returns the number of references to the assigned pointer.
-	 * This should just be used for debugging purposes.
-	 */
-	RefValue refCount() const {
-		return _refCount ? *_refCount : 0;
-	}
-
-	/**
-	 * Returns whether the referenced object isn't valid
-	 */
-	bool expired() const {
-		return !_refCount;
-	}
-
-	/**
-	 * Checks if the object is the only object refering
-	 * to the assigned pointer. This should just be used for
-	 * debugging purposes.
-	 */
-	bool unique() const {
-		return refCount() == 1;
-	}
-
-	BasePtr &operator=(const BasePtr &r) {
-		reset(r);
-		return *this;
-	}
-
-	template<class T2>
-	BasePtr &operator=(const BasePtr<T2> &r) {
-		reset(r);
-		return *this;
-	}
-
-	/**
-	 * Resets the object to a NULL pointer.
-	 */
-	void reset() {
-		decRef();
-		_deletion = nullptr;
-		_refCount = nullptr;
-		_pointer = nullptr;
-	}
-
-	/**
-	 * Resets the object to the specified pointer
-	 */
-	void reset(const BasePtr &r) {
-		if (r._refCount)
-			++(*r._refCount);
-		decRef();
-
-		_refCount = r._refCount;
-		_deletion = r._deletion;
-		_pointer = r._pointer;
-	}
-
-	/**
-	 * Resets the object to the specified pointer
-	 */
+protected:
 	template<class T2>
-	void reset(const BasePtr<T2> &r) {
-		if (r._refCount)
-			++(*r._refCount);
-		decRef();
-
-		_refCount = r._refCount;
-		_deletion = r._deletion;
+	void assign(const BasePtr<T2> &r) {
+		_tracker = r._tracker;
 		_pointer = r._pointer;
 	}
 
-	/**
-	 * Resets the object to the specified pointer
-	 */
-	void reset(T *ptr) {
-		reset(BasePtr<T>(ptr));
-	}
-
-protected:
-	RefValue *_refCount;
-	BasePtrDeletionInternal *_deletion;
-	PointerType _pointer;
-protected:
-	/**
-	 * Decrements the reference count to the stored pointer, and deletes it if
-	 * there are no longer any references to it
-	 */
-	void decRef() {
-		if (_refCount) {
-			--(*_refCount);
-			if (!*_refCount) {
-				delete _refCount;
-				delete _deletion;
-				_deletion = nullptr;
-				_refCount = nullptr;
-				_pointer = nullptr;
-			}
-		}
-	}
-
-	/**
-	 * Increments the reference count to the stored pointer
-	 */
-	void incRef() {
-		if (_refCount)
-			++*_refCount;
-	}
+	BasePtrTrackerInternal *_tracker;
+	T *_pointer;
 };
 
 template<class T>
@@ -276,6 +218,7 @@ class WeakPtr;
 template<class T>
 class SharedPtr : public BasePtr<T> {
 public:
+	// Invariant: If _tracker is non-null, then the object is alive
 	typedef T *PointerType;
 	typedef T &ReferenceType;
 
@@ -285,6 +228,11 @@ public:
 	SharedPtr(std::nullptr_t) : BasePtr<T>() {
 	}
 
+	~SharedPtr() {
+		if (this->_tracker)
+			this->_tracker->decStrong();
+	}
+
 	template<class T2>
 	explicit SharedPtr(T2 *p) : BasePtr<T>(p) {
 	}
@@ -294,23 +242,44 @@ public:
 	}
 
 	SharedPtr(const SharedPtr<T> &r) : BasePtr<T>(r) {
+		if (this->_tracker)
+			this->_tracker->incStrong();
 	}
 
-	SharedPtr(const WeakPtr<T> &r) : BasePtr<T>(r) {
+	SharedPtr(const WeakPtr<T> &r) {
+		reset(r);
 	}
 
 	template<class T2>
 	SharedPtr(const SharedPtr<T2> &r) : BasePtr<T>(r) {
+		if (this->_tracker)
+			this->_tracker->incStrong();
 	}
 
 	SharedPtr &operator=(const SharedPtr &r) {
-		BasePtr<T>::operator=(r);
+		BasePtrTrackerInternal *oldTracker = this->_tracker;
+
+		this->assign(r);
+
+		if (this->_tracker)
+			this->_tracker->incStrong();
+		if (oldTracker)
+			oldTracker->decStrong();
+
 		return *this;
 	}
 
 	template<class T2>
 	SharedPtr &operator=(const SharedPtr<T2> &r) {
-		BasePtr<T>::operator=(r);
+		BasePtrTrackerInternal *oldTracker = this->_tracker;
+
+		this->assign(r);
+
+		if (this->_tracker)
+			this->_tracker->incStrong();
+		if (oldTracker)
+			oldTracker->decStrong();
+
 		return *this;
 	}
 
@@ -334,6 +303,75 @@ public:
 	bool operator!=(const SharedPtr<T2> &r) const {
 		return this->_pointer != r.get();
 	}
+
+	/**
+	 * Checks if the object is the only object refering
+	 * to the assigned pointer. This should just be used for
+	 * debugging purposes.
+	 */
+	bool unique() const {
+		return this->_tracker != nullptr && this->_tracker->getStrongCount() == 1;
+	}
+
+	/**
+	 * Resets the object to a NULL pointer.
+	 */
+	void reset() {
+		if (this->_tracker)
+			this->_tracker->decStrong();
+		this->_tracker = nullptr;
+		this->_pointer = nullptr;
+	}
+
+	/**
+	 * Resets the object to the specified pointer
+	 */
+	void reset(const BasePtr<T> &r) {
+		BasePtrTrackerInternal *oldTracker = this->_tracker;
+
+		this->assign(r);
+
+		if (this->_tracker && this->_tracker->isAlive()) {
+			this->_tracker->incStrong();
+		} else {
+			this->_tracker = nullptr;
+			this->_pointer = nullptr;
+		}
+
+		if (oldTracker)
+			oldTracker->decStrong();
+	}
+
+	/**
+	 * Resets the object to the specified pointer
+	 */
+	template<class T2>
+	void reset(const BasePtr<T2> &r) {
+		BasePtrTrackerInternal *oldTracker = this->_tracker;
+
+		this->assign(r);
+
+		if (this->_tracker && this->_tracker->isAlive()) {
+			this->_tracker->incStrong();
+		} else {
+			this->_tracker = nullptr;
+			this->_pointer = nullptr;
+		}
+
+		if (oldTracker)
+			oldTracker->decStrong();
+	}
+
+	/**
+	 * Resets the object to the specified pointer
+	 */
+	void reset(T *ptr) {
+		if (this->_tracker)
+			this->_tracker->decStrong();
+
+		this->_pointer = ptr;
+		this->_tracker = new BasePtrTrackerImpl<T>(ptr);
+	}
 };
 
 /**
@@ -349,15 +387,20 @@ public:
 	WeakPtr(std::nullptr_t) : BasePtr<T>() {
 	}
 
-	template<class T2>
-	explicit WeakPtr(T2 *p) : BasePtr<T>(p) {
+	WeakPtr(const BasePtr<T> &r) : BasePtr<T>(r) {
+		if (this->_tracker)
+			this->_tracker->incWeak();
 	}
 
-	WeakPtr(const BasePtr<T> &r) : BasePtr<T>(r) {
+	~WeakPtr() {
+		if (this->_tracker)
+			this->_tracker->decWeak();
 	}
 
 	template<class T2>
 	WeakPtr(const BasePtr<T2> &r) : BasePtr<T>(r) {
+		if (this->_tracker)
+			this->_tracker->incWeak();
 	}
 
 	/**
@@ -366,6 +409,71 @@ public:
 	SharedPtr<T> lock() const {
 		return SharedPtr<T>(*this);
 	}
+
+	/**
+	 * Implicit conversion operator to bool for convenience, to make
+	 * checks like "if (sharedPtr) ..." possible.
+	 */
+	bool operator_bool() const {
+		return this->_pointer != nullptr;
+	}
+
+	/**
+	 * Returns whether the referenced object isn't valid
+	 */
+	bool expired() const {
+		return this->_tracker == nullptr || this->_tracker->getStrongCount() == 0;
+	}
+
+	WeakPtr<T> &operator=(const BasePtr<T> &r) {
+		reset(r);
+		return *this;
+	}
+
+	template<class T2>
+	WeakPtr<T> &operator=(const BasePtr<T2> &r) {
+		reset(r);
+		return *this;
+	}
+
+	/**
+	 * Resets the object to a NULL pointer.
+	 */
+	void reset() {
+		if (this->_tracker)
+			this->_tracker->decWeak();
+		this->_tracker = nullptr;
+		this->_pointer = nullptr;
+	}
+
+	/**
+	 * Resets the object to the specified pointer
+	 */
+	void reset(const BasePtr<T> &r) {
+		BasePtrTrackerInternal *oldTracker = this->_tracker;
+
+		this->assign(r);
+
+		if (oldTracker)
+			oldTracker->incWeak();
+		if (this->_tracker)
+			this->_tracker->decWeak();
+	}
+
+	/**
+	 * Resets the object to the specified pointer
+	 */
+	template<class T2>
+	void reset(const BasePtr<T2> &r) {
+		BasePtrTrackerInternal *oldTracker = this->_tracker;
+
+		this->assign(r);
+
+		if (oldTracker)
+			oldTracker->incWeak();
+		if (this->_tracker)
+			this->_tracker->decWeak();
+	}
 };
 
 template <typename T>
diff --git a/test/common/ptr.h b/test/common/ptr.h
index 790d541b137..8200825dc13 100644
--- a/test/common/ptr.h
+++ b/test/common/ptr.h
@@ -156,6 +156,16 @@ class PtrTestSuite : public CxxTest::TestSuite {
 		Common::SharedPtr<A> a(b);
 		a = b;
 	}
+
+	void test_weak_ptr() {
+		Common::SharedPtr<B> b(new B);
+		Common::WeakPtr<A> a(b);
+		TS_ASSERT(a.lock() == b);
+		TS_ASSERT(!a.expired());
+		b.reset();
+		TS_ASSERT(a.expired());
+		TS_ASSERT(!a.lock());
+	}
 };
 
 int PtrTestSuite::InstanceCountingClass::count = 0;


Commit: bd23944fd1c66dfbac34cb4c0c46d69c1a8fbd72
    https://github.com/scummvm/scummvm/commit/bd23944fd1c66dfbac34cb4c0c46d69c1a8fbd72
Author: elasota (ejlasota at gmail.com)
Date: 2022-04-25T18:20:19-07:00

Commit Message:
COMMON: Fix SharedPtr natvis

Changed paths:
    devtools/create_project/scripts/scummvm.natvis


diff --git a/devtools/create_project/scripts/scummvm.natvis b/devtools/create_project/scripts/scummvm.natvis
index 8d839246cf4..73fab76710c 100644
--- a/devtools/create_project/scripts/scummvm.natvis
+++ b/devtools/create_project/scripts/scummvm.natvis
@@ -135,7 +135,7 @@
     <DisplayString Condition="_pointer != 0">{*_pointer}</DisplayString>
     <Expand>
       <Item Condition="_pointer != 0" Name="[ptr]">_pointer</Item>
-      <Item Condition="_refCount != 0" Name="[refCount]">*_refCount</Item>
+      <Item Condition="_tracker != 0" Name="[refCount]">_tracker->_strongRefCount</Item>
     </Expand>
   </Type>
 </AutoVisualizer>


Commit: be5a7cb8e704105ecda2780fb76634b0f707211f
    https://github.com/scummvm/scummvm/commit/be5a7cb8e704105ecda2780fb76634b0f707211f
Author: elasota (ejlasota at gmail.com)
Date: 2022-04-25T18:20:19-07:00

Commit Message:
COMMON: Fix up WeakPtr logic

Changed paths:
    common/ptr.h


diff --git a/common/ptr.h b/common/ptr.h
index 5e27b22d3da..fb6769e14f0 100644
--- a/common/ptr.h
+++ b/common/ptr.h
@@ -425,6 +425,17 @@ public:
 		return this->_tracker == nullptr || this->_tracker->getStrongCount() == 0;
 	}
 
+	WeakPtr<T> &operator=(const WeakPtr<T> &r) {
+		reset(r);
+		return *this;
+	}
+
+	template<class T2>
+	WeakPtr<T> &operator=(const WeakPtr<T2> &r) {
+		reset(r);
+		return *this;
+	}
+
 	WeakPtr<T> &operator=(const BasePtr<T> &r) {
 		reset(r);
 		return *this;
@@ -454,10 +465,10 @@ public:
 
 		this->assign(r);
 
-		if (oldTracker)
-			oldTracker->incWeak();
 		if (this->_tracker)
-			this->_tracker->decWeak();
+			this->_tracker->incWeak();
+		if (oldTracker)
+			oldTracker->decWeak();
 	}
 
 	/**
@@ -469,10 +480,10 @@ public:
 
 		this->assign(r);
 
-		if (oldTracker)
-			oldTracker->incWeak();
 		if (this->_tracker)
-			this->_tracker->decWeak();
+			this->_tracker->incWeak();
+		if (oldTracker)
+			oldTracker->decWeak();
 	}
 };
 


Commit: c8d018675260ec7ca1dbee9ef10b7e11937fa2d9
    https://github.com/scummvm/scummvm/commit/c8d018675260ec7ca1dbee9ef10b7e11937fa2d9
Author: elasota (ejlasota at gmail.com)
Date: 2022-04-25T18:20:19-07:00

Commit Message:
COMMON: Fix WeakPtr missing copy constructor

Changed paths:
    common/ptr.h


diff --git a/common/ptr.h b/common/ptr.h
index fb6769e14f0..c4720cc1fe0 100644
--- a/common/ptr.h
+++ b/common/ptr.h
@@ -387,6 +387,11 @@ public:
 	WeakPtr(std::nullptr_t) : BasePtr<T>() {
 	}
 
+	WeakPtr(const WeakPtr<T> &r) : BasePtr<T>(r) {
+		if (this->_tracker)
+			this->_tracker->incWeak();
+	}
+
 	WeakPtr(const BasePtr<T> &r) : BasePtr<T>(r) {
 		if (this->_tracker)
 			this->_tracker->incWeak();


Commit: 22b72dba52e60fe6261d3d009b1bcf3d0c77a004
    https://github.com/scummvm/scummvm/commit/22b72dba52e60fe6261d3d009b1bcf3d0c77a004
Author: elasota (ejlasota at gmail.com)
Date: 2022-04-25T18:20:19-07:00

Commit Message:
COMMON: Remove BasePtr base class from SharedPtr and WeakPtr.  Fix missing refCount method in SharedPtr.

Changed paths:
    common/ptr.h
    devtools/create_project/scripts/scummvm.natvis


diff --git a/common/ptr.h b/common/ptr.h
index c4720cc1fe0..6911fd983a9 100644
--- a/common/ptr.h
+++ b/common/ptr.h
@@ -112,65 +112,6 @@ private:
 	DL _deleter;
 };
 
-/**
- * A base class for both SharedPtr and WeakPtr.
- *
- * This base class encapsulates the logic for the reference counter
- * used by both.
- */
-template<class T>
-class BasePtr : public SafeBool<BasePtr<T> > {
-#if !defined(__GNUC__) || GCC_ATLEAST(3, 0)
-	template<class T2> friend class BasePtr;
-#endif
-public:
-	typedef T ValueType;
-	typedef T *PointerType;
-	typedef T &ReferenceType;
-
-	BasePtr() : _tracker(nullptr), _pointer(nullptr) {
-	}
-
-	explicit BasePtr(std::nullptr_t) : _tracker(nullptr), _pointer(nullptr) {
-	}
-
-	template<class T2>
-	explicit BasePtr(T2 *p) : _tracker(p ? (new BasePtrTrackerImpl<T2>(p)) : nullptr), _pointer(p) {
-	}
-
-	template<class T2, class DL>
-	BasePtr(T2 *p, DL d) : _tracker(p ? new BasePtrTrackerDeletionImpl<T2, DL>(p, d) : nullptr), _pointer(p) {
-	}
-
-	BasePtr(const BasePtr &r) : _tracker(r._tracker), _pointer(r._pointer) {
-	}
-
-	template<class T2>
-	BasePtr(const BasePtr<T2> &r) : _tracker(r._tracker), _pointer(r._pointer) {
-	}
-
-	~BasePtr() {
-	}
-
-	/**
-	 * Implicit conversion operator to bool for convenience, to make
-	 * checks like "if (sharedPtr) ..." possible.
-	 */
-	bool operator_bool() const {
-		return _pointer != nullptr;
-	}
-
-protected:
-	template<class T2>
-	void assign(const BasePtr<T2> &r) {
-		_tracker = r._tracker;
-		_pointer = r._pointer;
-	}
-
-	BasePtrTrackerInternal *_tracker;
-	T *_pointer;
-};
-
 template<class T>
 class WeakPtr;
 
@@ -216,75 +157,69 @@ class WeakPtr;
  * a plain pointer is only possible via SharedPtr::get.
  */
 template<class T>
-class SharedPtr : public BasePtr<T> {
+class SharedPtr : public SafeBool<SharedPtr<T> > {
+	template<class T2>
+	friend class WeakPtr;
+	template<class T2>
+	friend class SharedPtr;
 public:
 	// Invariant: If _tracker is non-null, then the object is alive
 	typedef T *PointerType;
 	typedef T &ReferenceType;
+	typedef BasePtrTrackerInternal::RefValue RefValue;
 
-	SharedPtr() : BasePtr<T>() {
+	SharedPtr() : _pointer(nullptr), _tracker(nullptr) {
 	}
 
-	SharedPtr(std::nullptr_t) : BasePtr<T>() {
+	SharedPtr(std::nullptr_t) : _pointer(nullptr), _tracker(nullptr) {
 	}
 
 	~SharedPtr() {
-		if (this->_tracker)
-			this->_tracker->decStrong();
+		if (_tracker)
+			_tracker->decStrong();
 	}
 
 	template<class T2>
-	explicit SharedPtr(T2 *p) : BasePtr<T>(p) {
+	explicit SharedPtr(T2 *p) : _pointer(p), _tracker(p ? (new BasePtrTrackerImpl<T2>(p)) : nullptr) {
 	}
 
 	template<class T2, class DL>
-	SharedPtr(T2 *p, DL d) : BasePtr<T>(p, d) {
+	SharedPtr(T2 *p, DL d) : _pointer(p), _tracker(p ? (new BasePtrTrackerDeletionImpl<T2, DL>(p, d)) : nullptr) {
 	}
 
-	SharedPtr(const SharedPtr<T> &r) : BasePtr<T>(r) {
-		if (this->_tracker)
-			this->_tracker->incStrong();
+	SharedPtr(const SharedPtr<T> &r) : _pointer(r._pointer), _tracker(r._tracker) {
+		if (_tracker)
+			_tracker->incStrong();
 	}
 
-	SharedPtr(const WeakPtr<T> &r) {
-		reset(r);
+	template<class T2>
+	SharedPtr(const SharedPtr<T2> &r) : _pointer(r._pointer), _tracker(r._tracker) {
+		if (_tracker)
+			_tracker->incStrong();
 	}
 
 	template<class T2>
-	SharedPtr(const SharedPtr<T2> &r) : BasePtr<T>(r) {
-		if (this->_tracker)
-			this->_tracker->incStrong();
+	explicit SharedPtr(const WeakPtr<T2> &r) : _pointer(nullptr), _tracker(nullptr) {
+		if (r._tracker && r._tracker->isAlive()) {
+			_pointer = r._pointer;
+			_tracker = r._tracker;
+			_tracker->incStrong();
+		}
 	}
 
 	SharedPtr &operator=(const SharedPtr &r) {
-		BasePtrTrackerInternal *oldTracker = this->_tracker;
-
-		this->assign(r);
-
-		if (this->_tracker)
-			this->_tracker->incStrong();
-		if (oldTracker)
-			oldTracker->decStrong();
-
+		reset(r);
 		return *this;
 	}
 
 	template<class T2>
 	SharedPtr &operator=(const SharedPtr<T2> &r) {
-		BasePtrTrackerInternal *oldTracker = this->_tracker;
-
-		this->assign(r);
-
-		if (this->_tracker)
-			this->_tracker->incStrong();
-		if (oldTracker)
-			oldTracker->decStrong();
-
+		reset(r);
 		return *this;
 	}
 
-	T &operator*() const { assert(this->_pointer); return *this->_pointer; }
-	T *operator->() const { assert(this->_pointer); return this->_pointer; }
+	T &operator*() const { assert(_pointer); return *_pointer; }
+	T *operator->() const { assert(_pointer); return _pointer; }
 
 	/**
 	 * Returns the plain pointer value. Be sure you know what you
@@ -292,16 +227,33 @@ public:
 	 *
 	 * @return the pointer the SharedPtr object manages
 	 */
-	PointerType get() const { return this->_pointer; }
+	PointerType get() const { return _pointer; }
 
 	template<class T2>
 	bool operator==(const SharedPtr<T2> &r) const {
-		return this->_pointer == r.get();
+		return _pointer == r.get();
 	}
 
 	template<class T2>
 	bool operator!=(const SharedPtr<T2> &r) const {
-		return this->_pointer != r.get();
+		return _pointer != r.get();
+	}
+
+	/**
+	 * Implicit conversion operator to bool for convenience, to make
+	 * checks like "if (sharedPtr) ..." possible.
+	 */
+	bool operator_bool() const {
+		return _pointer != nullptr;
+	}
+
+	/**
+	 * Returns the number of strong references to the object.
+	 */
+	int refCount() const {
+		if (_tracker == nullptr)
+			return 0;
+		return _tracker->getStrongCount();
 	}
 
 	/**
@@ -310,52 +262,49 @@ public:
 	 * debugging purposes.
 	 */
 	bool unique() const {
-		return this->_tracker != nullptr && this->_tracker->getStrongCount() == 1;
+		return refCount() == 1;
 	}
 
 	/**
 	 * Resets the object to a NULL pointer.
 	 */
 	void reset() {
-		if (this->_tracker)
-			this->_tracker->decStrong();
-		this->_tracker = nullptr;
-		this->_pointer = nullptr;
+		if (_tracker)
+			_tracker->decStrong();
+		_tracker = nullptr;
+		_pointer = nullptr;
 	}
 
 	/**
-	 * Resets the object to the specified pointer
+	 * Resets the object to the specified shared pointer
 	 */
-	void reset(const BasePtr<T> &r) {
-		BasePtrTrackerInternal *oldTracker = this->_tracker;
-
-		this->assign(r);
+	template<class T2>
+	void reset(const SharedPtr<T2> &r) {
+		BasePtrTrackerInternal *oldTracker = _tracker;
 
-		if (this->_tracker && this->_tracker->isAlive()) {
-			this->_tracker->incStrong();
-		} else {
-			this->_tracker = nullptr;
-			this->_pointer = nullptr;
-		}
+		_pointer = r._pointer;
+		_tracker = r._tracker;
 
+		if (_tracker)
+			_tracker->incStrong();
 		if (oldTracker)
 			oldTracker->decStrong();
 	}
 
 	/**
-	 * Resets the object to the specified pointer
+	 * Resets the object to the specified weak pointer
 	 */
 	template<class T2>
-	void reset(const BasePtr<T2> &r) {
-		BasePtrTrackerInternal *oldTracker = this->_tracker;
+	void reset(const WeakPtr<T2> &r) {
+		BasePtrTrackerInternal *oldTracker = _tracker;
 
-		this->assign(r);
-
-		if (this->_tracker && this->_tracker->isAlive()) {
-			this->_tracker->incStrong();
+		if (r._tracker && r._tracker->isAlive()) {
+			_tracker = r._tracker;
+			_pointer = r._pointer;
+			_tracker->incStrong();
 		} else {
-			this->_tracker = nullptr;
-			this->_pointer = nullptr;
+			_tracker = nullptr;
+			_pointer = nullptr;
 		}
 
 		if (oldTracker)
@@ -366,46 +315,57 @@ public:
 	 * Resets the object to the specified pointer
 	 */
 	void reset(T *ptr) {
-		if (this->_tracker)
-			this->_tracker->decStrong();
+		if (_tracker)
+			_tracker->decStrong();
 
-		this->_pointer = ptr;
-		this->_tracker = new BasePtrTrackerImpl<T>(ptr);
+		_pointer = ptr;
+		_tracker = new BasePtrTrackerImpl<T>(ptr);
 	}
+
+private:
+	T *_pointer;
+	BasePtrTrackerInternal *_tracker;
 };
 
+
+
 /**
  * Implements a smart pointer that holds a non-owning ("weak") refrence to
  * a pointer. It needs to be converted to a SharedPtr to access it.
  */
 template<class T>
-class WeakPtr : public BasePtr<T> {
+class WeakPtr : public SafeBool<WeakPtr<T> > {
+	template<class T2>
+	friend class WeakPtr;
+	template<class T2>
+	friend class SharedPtr;
 public:
-	WeakPtr() : BasePtr<T>() {
+	WeakPtr() : _pointer(nullptr), _tracker(nullptr) {
 	}
 
-	WeakPtr(std::nullptr_t) : BasePtr<T>() {
+	WeakPtr(std::nullptr_t) : _pointer(nullptr), _tracker(nullptr) {
 	}
 
-	WeakPtr(const WeakPtr<T> &r) : BasePtr<T>(r) {
-		if (this->_tracker)
-			this->_tracker->incWeak();
+	WeakPtr(const WeakPtr<T> &r) : _pointer(r._pointer), _tracker(r._tracker) {
+		if (_tracker)
+			_tracker->incWeak();
 	}
 
-	WeakPtr(const BasePtr<T> &r) : BasePtr<T>(r) {
-		if (this->_tracker)
-			this->_tracker->incWeak();
+	~WeakPtr() {
+		if (_tracker)
+			_tracker->decWeak();
 	}
 
-	~WeakPtr() {
-		if (this->_tracker)
-			this->_tracker->decWeak();
+	template<class T2>
+	WeakPtr(const WeakPtr<T2> &r) : _pointer(r._pointer), _tracker(r._tracker) {
+		if (_tracker)
+			_tracker->incWeak();
 	}
 
 	template<class T2>
-	WeakPtr(const BasePtr<T2> &r) : BasePtr<T>(r) {
-		if (this->_tracker)
-			this->_tracker->incWeak();
+	WeakPtr(const SharedPtr<T2> &r) : _pointer(r._pointer), _tracker(r._tracker) {
+		if (_tracker)
+			_tracker->incWeak();
 	}
 
 	/**
@@ -417,17 +377,27 @@ public:
 
 	/**
 	 * Implicit conversion operator to bool for convenience, to make
-	 * checks like "if (sharedPtr) ..." possible.
+	 * checks like "if (weakPtr) ..." possible.  This only checks if the
+	 * weak pointer contains a pointer, not that the pointer is live.
 	 */
 	bool operator_bool() const {
-		return this->_pointer != nullptr;
+		return _pointer != nullptr;
+	}
+
+	/**
+	 * Returns the number of strong references to the object.
+	 */
+	int refCount() const {
+		if (_tracker == nullptr)
+			return 0;
+		return _tracker->getStrongCount();
 	}
 
 	/**
 	 * Returns whether the referenced object isn't valid
 	 */
 	bool expired() const {
-		return this->_tracker == nullptr || this->_tracker->getStrongCount() == 0;
+		return _tracker == nullptr || _tracker->getStrongCount() == 0;
 	}
 
 	WeakPtr<T> &operator=(const WeakPtr<T> &r) {
@@ -441,13 +411,8 @@ public:
 		return *this;
 	}
 
-	WeakPtr<T> &operator=(const BasePtr<T> &r) {
-		reset(r);
-		return *this;
-	}
-
 	template<class T2>
-	WeakPtr<T> &operator=(const BasePtr<T2> &r) {
+	WeakPtr<T> &operator=(const SharedPtr<T2> &r) {
 		reset(r);
 		return *this;
 	}
@@ -456,40 +421,47 @@ public:
 	 * Resets the object to a NULL pointer.
 	 */
 	void reset() {
-		if (this->_tracker)
-			this->_tracker->decWeak();
-		this->_tracker = nullptr;
-		this->_pointer = nullptr;
+		if (_tracker)
+			_tracker->decWeak();
+		_tracker = nullptr;
+		_pointer = nullptr;
 	}
 
 	/**
-	 * Resets the object to the specified pointer
+	 * Resets the object to the specified shared pointer
 	 */
-	void reset(const BasePtr<T> &r) {
-		BasePtrTrackerInternal *oldTracker = this->_tracker;
+	template<class T2>
+	void reset(const SharedPtr<T2> &r) {
+		BasePtrTrackerInternal *oldTracker = _tracker;
 
-		this->assign(r);
+		_pointer = r._pointer;
+		_tracker = r._tracker;
 
-		if (this->_tracker)
-			this->_tracker->incWeak();
+		if (_tracker)
+			_tracker->incWeak();
 		if (oldTracker)
 			oldTracker->decWeak();
 	}
 
 	/**
-	 * Resets the object to the specified pointer
+	 * Resets the object to the specified weak pointer
 	 */
 	template<class T2>
-	void reset(const BasePtr<T2> &r) {
-		BasePtrTrackerInternal *oldTracker = this->_tracker;
+	void reset(const WeakPtr<T2> &r) {
+		BasePtrTrackerInternal *oldTracker = _tracker;
 
-		this->assign(r);
+		_pointer = r._pointer;
+		_tracker = r._tracker;
 
-		if (this->_tracker)
-			this->_tracker->incWeak();
+		if (_tracker)
+			_tracker->incWeak();
 		if (oldTracker)
 			oldTracker->decWeak();
 	}
+
+private:
+	T *_pointer;
+	BasePtrTrackerInternal *_tracker;
 };
 
 template <typename T>
diff --git a/devtools/create_project/scripts/scummvm.natvis b/devtools/create_project/scripts/scummvm.natvis
index 73fab76710c..63b94bff620 100644
--- a/devtools/create_project/scripts/scummvm.natvis
+++ b/devtools/create_project/scripts/scummvm.natvis
@@ -130,6 +130,15 @@
     </Expand>
   </Type>
 
+  <Type Name="Common::WeakPtr<*>">
+    <DisplayString Condition="_pointer == 0">nullptr</DisplayString>
+    <DisplayString Condition="_pointer != 0">{*_pointer}</DisplayString>
+    <Expand>
+      <Item Condition="_pointer != 0" Name="[ptr]">_pointer</Item>
+      <Item Condition="_tracker != 0" Name="[refCount]">_tracker->_strongRefCount</Item>
+    </Expand>
+  </Type>
+
   <Type Name="Common::SharedPtr<*>">
     <DisplayString Condition="_pointer == 0">nullptr</DisplayString>
     <DisplayString Condition="_pointer != 0">{*_pointer}</DisplayString>


Commit: 2432c674264fd9749ff33e42849d702c5f6e8d86
    https://github.com/scummvm/scummvm/commit/2432c674264fd9749ff33e42849d702c5f6e8d86
Author: elasota (ejlasota at gmail.com)
Date: 2022-04-25T18:20:19-07:00

Commit Message:
COMMON: Fix double destruction when using a deleter.

Changed paths:
    common/ptr.h


diff --git a/common/ptr.h b/common/ptr.h
index 6911fd983a9..2e0cb48f128 100644
--- a/common/ptr.h
+++ b/common/ptr.h
@@ -101,7 +101,7 @@ template<class T, class DL>
 class BasePtrTrackerDeletionImpl : public BasePtrTrackerInternal {
 public:
 	BasePtrTrackerDeletionImpl(T *ptr, DL d) : _ptr(ptr), _deleter(d) {}
-	~BasePtrTrackerDeletionImpl() { _deleter(_ptr); }
+	~BasePtrTrackerDeletionImpl() { }
 
 private:
 	void destructObject() override {


Commit: a1f2121bb87c536c35c43921b50979cfe9602070
    https://github.com/scummvm/scummvm/commit/a1f2121bb87c536c35c43921b50979cfe9602070
Author: elasota (ejlasota at gmail.com)
Date: 2022-04-25T18:20:19-07:00

Commit Message:
COMMON: Remove empty BasePtrTrackerDeletionImpl destructor

Changed paths:
    common/ptr.h


diff --git a/common/ptr.h b/common/ptr.h
index 2e0cb48f128..ff55241fc8c 100644
--- a/common/ptr.h
+++ b/common/ptr.h
@@ -101,7 +101,6 @@ template<class T, class DL>
 class BasePtrTrackerDeletionImpl : public BasePtrTrackerInternal {
 public:
 	BasePtrTrackerDeletionImpl(T *ptr, DL d) : _ptr(ptr), _deleter(d) {}
-	~BasePtrTrackerDeletionImpl() { }
 
 private:
 	void destructObject() override {


Commit: 5cb75fdaecdd061f93e45196caaab0b819b2827d
    https://github.com/scummvm/scummvm/commit/5cb75fdaecdd061f93e45196caaab0b819b2827d
Author: elasota (ejlasota at gmail.com)
Date: 2022-04-25T18:20:19-07:00

Commit Message:
COMMON: Remove implicit bool conversion from WeakPtr

Changed paths:
    common/ptr.h


diff --git a/common/ptr.h b/common/ptr.h
index ff55241fc8c..45353da3518 100644
--- a/common/ptr.h
+++ b/common/ptr.h
@@ -333,7 +333,7 @@ private:
  * a pointer. It needs to be converted to a SharedPtr to access it.
  */
 template<class T>
-class WeakPtr : public SafeBool<WeakPtr<T> > {
+class WeakPtr {
 	template<class T2>
 	friend class WeakPtr;
 	template<class T2>
@@ -374,15 +374,6 @@ public:
 		return SharedPtr<T>(*this);
 	}
 
-	/**
-	 * Implicit conversion operator to bool for convenience, to make
-	 * checks like "if (weakPtr) ..." possible.  This only checks if the
-	 * weak pointer contains a pointer, not that the pointer is live.
-	 */
-	bool operator_bool() const {
-		return _pointer != nullptr;
-	}
-
 	/**
 	 * Returns the number of strong references to the object.
 	 */




More information about the Scummvm-git-logs mailing list