From 076667dc0064432a3d4e3cf696d6688ee01a17a9 Mon Sep 17 00:00:00 2001 From: Colin Snover Date: Fri, 17 Nov 2017 23:28:15 -0600 Subject: [PATCH] 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. --- common/ptr.h | 30 +++++++++--------- engines/savestate.cpp | 2 +- graphics/surface.h | 2 +- test/common/ptr.h | 72 ++++++++++++++++++++++++++++++++++++++----- 4 files changed, 82 insertions(+), 24 deletions(-) diff --git a/common/ptr.h b/common/ptr.h index 510fec14df2..f592beb0052 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 -class ScopedPtr : private NonCopyable, public SafeBool > { +template +struct DefaultDeleter { + inline void operator()(T *object) { + STATIC_ASSERT(sizeof(T) > 0, cannot_delete_incomplete_type); + delete object; + } +}; + +template > +class ScopedPtr : private NonCopyable, public SafeBool > { 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 -class DisposablePtr : private NonCopyable, public SafeBool > { +template > +class DisposablePtr : private NonCopyable, public SafeBool > { 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 20a37581dc8..92c1eaf837d 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(t, Graphics::SharedPtrSurfaceDeleter()); + _thumbnail = Common::SharedPtr(t, Graphics::SurfaceDeleter()); } void SaveStateDescriptor::setSaveDate(int year, int month, int day) { diff --git a/graphics/surface.h b/graphics/surface.h index 87c5f525035..19107b8bab0 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 ebd978959a6..0c871a761d9 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 a(new int(0)); + TS_ASSERT(!CustomDeleter::invoked); + } + + TS_ASSERT(CustomDeleter::invoked); + } + + void test_disposable_deleter() { + CustomDeleter::invoked = false; + + { + Common::DisposablePtr a1(new int, DisposeAfterUse::YES); + TS_ASSERT(!CustomDeleter::invoked); + } + + TS_ASSERT(CustomDeleter::invoked); + CustomDeleter::invoked = false; + + int *a = new int; + { + Common::DisposablePtr a2(a, DisposeAfterUse::NO); + } + + TS_ASSERT(!CustomDeleter::invoked); + delete a; + } + + void test_scoped_deref() { + A *raw = new A(); + raw->a = 123; + Common::ScopedPtr 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(raw, DisposeAfterUse::YES); + TS_ASSERT_EQUALS(&*a, &*raw); + TS_ASSERT_EQUALS(a->a, raw->a); + } + void test_assign() { Common::SharedPtr 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(new B); Common::SharedPtr a(b); @@ -104,3 +159,4 @@ class PtrTestSuite : public CxxTest::TestSuite { }; int PtrTestSuite::InstanceCountingClass::count = 0; +bool PtrTestSuite::CustomDeleter::invoked = false;