COMMON: Add limited support for custom deleters to ScopedPtr

Custom deleters of ScopedPtr are not currently fully conforming to
C++11's support for custom deleters in std::unique_ptr for the
sake of simplicity of implementation. Unlike in the standard
library, plain functions and lvalue references are not supported,
nor may custom deleters be passed to the constructor at runtime.
This can be improved in the future, if necessary, by doing what
standard library implementations usually do and creating a Pair
class that uses the Empty Base Optimization idiom to avoid extra
storage overhead of the deleter instance when it is not needed, as
in typical standard library implementations, plus some additional
type traits to support the necessary metaprogramming for the
different type overloads.

4 files changed, 82 insertions, 24 deletions

diff --git a/common/ptr.h b/common/ptr.h
index 510fec14df..f592beb005 100644
--- a/common/ptr.h
+++ b/common/ptr.h
@@ -40,12 +40,7 @@ class SharedPtrDeletionImpl : public SharedPtrDeletionInternal {
 public:
 	SharedPtrDeletionImpl(T *ptr) : _ptr(ptr) {}
 	~SharedPtrDeletionImpl() {
-		// Checks if the supplied type is not just a plain
-		// forward definition, taken from boost::checked_delete
-		// This makes the user really aware what he tries to do
-		// when using this with an incomplete type.
-		typedef char completeCheck[sizeof(T) ? 1 : -1];
-		(void)sizeof(completeCheck);
+		STATIC_ASSERT(sizeof(T) > 0, SharedPtr_cannot_delete_incomplete_type);
 		delete _ptr;
 	}
 private:
@@ -223,8 +218,16 @@ private:
 	PointerType _pointer;
 };
 
-template<typename T>
-class ScopedPtr : private NonCopyable, public SafeBool<ScopedPtr<T> > {
+template <typename T>
+struct DefaultDeleter {
+	inline void operator()(T *object) {
+		STATIC_ASSERT(sizeof(T) > 0, cannot_delete_incomplete_type);
+		delete object;
+	}
+};
+
+template<typename T, class D = DefaultDeleter<T> >
+class ScopedPtr : private NonCopyable, public SafeBool<ScopedPtr<T, D> > {
 public:
 	typedef T ValueType;
 	typedef T *PointerType;
@@ -242,14 +245,14 @@ public:
 	bool operator_bool() const { return _pointer != nullptr; }
 
 	~ScopedPtr() {
-		delete _pointer;
+		D()(_pointer);
 	}
 
 	/**
 	 * Resets the pointer with the new value. Old object will be destroyed
 	 */
 	void reset(PointerType o = 0) {
-		delete _pointer;
+		D()(_pointer);
 		_pointer = o;
 	}
 
@@ -276,9 +279,8 @@ private:
 	PointerType _pointer;
 };
 
-
-template<typename T>
-class DisposablePtr : private NonCopyable, public SafeBool<DisposablePtr<T> > {
+template<typename T, class D = DefaultDeleter<T> >
+class DisposablePtr : private NonCopyable, public SafeBool<DisposablePtr<T, D> > {
 public:
 	typedef T  ValueType;
 	typedef T *PointerType;
@@ -287,7 +289,7 @@ public:
 	explicit DisposablePtr(PointerType o, DisposeAfterUse::Flag dispose) : _pointer(o), _dispose(dispose) {}
 
 	~DisposablePtr() {
-		if (_dispose) delete _pointer;
+		if (_dispose) D()(_pointer);
 	}
 
 	ReferenceType operator*() const { return *_pointer; }
diff --git a/engines/savestate.cpp b/engines/savestate.cpp
index 20a37581dc..92c1eaf837 100644
--- a/engines/savestate.cpp
+++ b/engines/savestate.cpp
@@ -39,7 +39,7 @@ void SaveStateDescriptor::setThumbnail(Graphics::Surface *t) {
 	if (_thumbnail.get() == t)
 		return;
 
-	_thumbnail = Common::SharedPtr<Graphics::Surface>(t, Graphics::SharedPtrSurfaceDeleter());
+	_thumbnail = Common::SharedPtr<Graphics::Surface>(t, Graphics::SurfaceDeleter());
 }
 
 void SaveStateDescriptor::setSaveDate(int year, int month, int day) {
diff --git a/graphics/surface.h b/graphics/surface.h
index 87c5f52503..19107b8bab 100644
--- a/graphics/surface.h
+++ b/graphics/surface.h
@@ -334,7 +334,7 @@ public:
  *
  * This deleter assures Surface::free is called on deletion.
  */
-struct SharedPtrSurfaceDeleter {
+struct SurfaceDeleter {
 	void operator()(Surface *ptr) {
 		if (ptr) {
 			ptr->free();
diff --git a/test/common/ptr.h b/test/common/ptr.h
index ebd978959a..0c871a761d 100644
--- a/test/common/ptr.h
+++ b/test/common/ptr.h
@@ -5,6 +5,14 @@
 class PtrTestSuite : public CxxTest::TestSuite {
 	public:
 
+	struct A {
+		int a;
+	};
+
+	struct B : public A {
+		int b;
+	};
+
 	// A simple class which keeps track of all its instances
 	class InstanceCountingClass {
 	public:
@@ -26,6 +34,61 @@ class PtrTestSuite : public CxxTest::TestSuite {
 		TS_ASSERT_EQUALS(InstanceCountingClass::count, 0);
 	}
 
+	struct CustomDeleter {
+		static bool invoked;
+		void operator()(int *object) {
+			invoked = true;
+			delete object;
+		}
+	};
+
+	void test_scoped_deleter() {
+		CustomDeleter::invoked = false;
+
+		{
+			Common::ScopedPtr<int, CustomDeleter> a(new int(0));
+			TS_ASSERT(!CustomDeleter::invoked);
+		}
+
+		TS_ASSERT(CustomDeleter::invoked);
+	}
+
+	void test_disposable_deleter() {
+		CustomDeleter::invoked = false;
+
+		{
+			Common::DisposablePtr<int, CustomDeleter> a1(new int, DisposeAfterUse::YES);
+			TS_ASSERT(!CustomDeleter::invoked);
+		}
+
+		TS_ASSERT(CustomDeleter::invoked);
+		CustomDeleter::invoked = false;
+
+		int *a = new int;
+		{
+			Common::DisposablePtr<int, CustomDeleter> a2(a, DisposeAfterUse::NO);
+		}
+
+		TS_ASSERT(!CustomDeleter::invoked);
+		delete a;
+	}
+
+	void test_scoped_deref() {
+		A *raw = new A();
+		raw->a = 123;
+		Common::ScopedPtr<A> a(raw);
+		TS_ASSERT_EQUALS(&*a, &*raw);
+		TS_ASSERT_EQUALS(a->a, raw->a);
+	}
+
+	void test_disposable_deref() {
+		A *raw = new A();
+		raw->a = 123;
+		Common::DisposablePtr<A> a(raw, DisposeAfterUse::YES);
+		TS_ASSERT_EQUALS(&*a, &*raw);
+		TS_ASSERT_EQUALS(a->a, raw->a);
+	}
+
 	void test_assign() {
 		Common::SharedPtr<int> p1(new int(1));
 		TS_ASSERT(p1.unique());
@@ -88,14 +151,6 @@ class PtrTestSuite : public CxxTest::TestSuite {
 		TS_ASSERT(!p1);
 	}
 
-	struct A {
-		int a;
-	};
-
-	struct B : public A {
-		int b;
-	};
-
 	void test_cast() {
 		Common::SharedPtr<B> b(new B);
 		Common::SharedPtr<A> a(b);
@@ -104,3 +159,4 @@ class PtrTestSuite : public CxxTest::TestSuite {
 };
 
 int PtrTestSuite::InstanceCountingClass::count = 0;
+bool PtrTestSuite::CustomDeleter::invoked = false;