From 036dc5542c450f21740faf97d8941cd7e0b89113 Mon Sep 17 00:00:00 2001 From: Darragh Coy Date: Wed, 26 Mar 2014 15:30:15 -0700 Subject: [PATCH 01/10] Add cocos2d::RefPtr This class which acts a smart pointer type class for cocos2d::Ref objects. Can be used to help automate memory management and remove manual retain/release calls from code, resulting in less code and less chance of errors. Also useful to implement the RAII idiom in certain scenarios, ensuring that an object gets released upon exiting a scope. --- .../project.pbxproj.REMOVED.git-id | 2 +- .../project.pbxproj.REMOVED.git-id | 2 +- cocos/base/CCRefPtr.cpp | 51 ++++ cocos/base/CCRefPtr.h | 238 +++++++++++++++ .../cpp-tests/Classes/UnitTest/RefPtrTest.cpp | 287 ++++++++++++++++++ tests/cpp-tests/Classes/UnitTest/RefPtrTest.h | 17 ++ tests/cpp-tests/Classes/UnitTest/UnitTest.cpp | 4 +- 7 files changed, 598 insertions(+), 3 deletions(-) create mode 100644 cocos/base/CCRefPtr.cpp create mode 100644 cocos/base/CCRefPtr.h create mode 100644 tests/cpp-tests/Classes/UnitTest/RefPtrTest.cpp create mode 100644 tests/cpp-tests/Classes/UnitTest/RefPtrTest.h diff --git a/build/cocos2d_libs.xcodeproj/project.pbxproj.REMOVED.git-id b/build/cocos2d_libs.xcodeproj/project.pbxproj.REMOVED.git-id index 6cc12b110a..f8f068ce4c 100644 --- a/build/cocos2d_libs.xcodeproj/project.pbxproj.REMOVED.git-id +++ b/build/cocos2d_libs.xcodeproj/project.pbxproj.REMOVED.git-id @@ -1 +1 @@ -bdcba92fe68d80bd996d1c61e15ea07397895549 \ No newline at end of file +f722a4d89963f2d2a58b754e12d1baa2bf0c5ece \ No newline at end of file diff --git a/build/cocos2d_tests.xcodeproj/project.pbxproj.REMOVED.git-id b/build/cocos2d_tests.xcodeproj/project.pbxproj.REMOVED.git-id index f9786b571b..157a7b047a 100644 --- a/build/cocos2d_tests.xcodeproj/project.pbxproj.REMOVED.git-id +++ b/build/cocos2d_tests.xcodeproj/project.pbxproj.REMOVED.git-id @@ -1 +1 @@ -7aef47d712ae9388fcf70cac6c96bcd596f7fc82 \ No newline at end of file +d2fface37dcb7869c88017873fbf673fdfc35895 \ No newline at end of file diff --git a/cocos/base/CCRefPtr.cpp b/cocos/base/CCRefPtr.cpp new file mode 100644 index 0000000000..0cea720a83 --- /dev/null +++ b/cocos/base/CCRefPtr.cpp @@ -0,0 +1,51 @@ +/**************************************************************************** + Copyright (c) 2014 PlayFirst Inc. + Copyright (c) 2014 Chukong Technologies Inc. + + http://www.cocos2d-x.org + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + THE SOFTWARE. + ****************************************************************************/ + +#include "CCRefPtr.h" +#include "CCRef.h" + +NS_CC_BEGIN + +void RefPtrSupportFunctions::safeRetainRef(const void * refPtr) +{ + if (refPtr) + { + Ref * refPtrCast = reinterpret_cast(const_cast(refPtr)); + CC_ASSERT(dynamic_cast(refPtrCast)); // Check for invalid type pointed to in RefPtr. Object pointed to must be a cocos2d::Ref! + refPtrCast->retain(); + } +} + +void RefPtrSupportFunctions::safeReleaseRef(const void * refPtr) +{ + if (refPtr) + { + Ref * refPtrCast = reinterpret_cast(const_cast(refPtr)); + CC_ASSERT(dynamic_cast(refPtrCast)); // Check for invalid type pointed to in RefPtr. Object pointed to must be a cocos2d::Ref! + refPtrCast->release(); + } +} + +NS_CC_END diff --git a/cocos/base/CCRefPtr.h b/cocos/base/CCRefPtr.h new file mode 100644 index 0000000000..4ffba53bbf --- /dev/null +++ b/cocos/base/CCRefPtr.h @@ -0,0 +1,238 @@ +/**************************************************************************** + Copyright (c) 2014 PlayFirst Inc. + Copyright (c) 2014 Chukong Technologies Inc. + + http://www.cocos2d-x.org + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + THE SOFTWARE. + ****************************************************************************/ + +#ifndef __CC_REF_PTR_H__ +#define __CC_REF_PTR_H__ + +#include "CCPlatformMacros.h" +#include "ccConfig.h" +#include + +NS_CC_BEGIN + +class Ref; + +/** + * Utility/support functions for RefPtr. + */ +namespace RefPtrSupportFunctions +{ + void safeRetainRef(const void * refPtr); + + void safeReleaseRef(const void * refPtr); +} + +/** + * Wrapper class which maintains a strong reference to a cocos2dx cocos2d::Ref* type object. + * Similar in concept to a boost smart pointer. + * + * Enables the use of the RAII idiom with Cocos2dx objects and helps automate some of the more + * mundane tasks of pointer initialization and cleanup. + * + * The class itself is modelled on C++ 11 std::shared_ptr, and trys to keep some of the methods + * and functionality consistent with std::shared_ptr. + */ +template class RefPtr +{ +public: + + RefPtr() = default; + + inline RefPtr(T * ptr) + : + _ptr(ptr) + { + RefPtrSupportFunctions::safeRetainRef(_ptr); + } + + inline RefPtr(const RefPtr & other) + : + _ptr(other._ptr) + { + RefPtrSupportFunctions::safeRetainRef(_ptr); + } + + inline ~RefPtr() + { + RefPtrSupportFunctions::safeReleaseRef(_ptr); + _ptr = nullptr; + } + + inline RefPtr & operator = (const RefPtr & other) + { + if (other._ptr != _ptr) + { + RefPtrSupportFunctions::safeRetainRef(other._ptr); + RefPtrSupportFunctions::safeReleaseRef(_ptr); + _ptr = other._ptr; + } + + return *this; + } + + inline RefPtr & operator = (T * other) + { + if (other != _ptr) + { + RefPtrSupportFunctions::safeRetainRef(other); + RefPtrSupportFunctions::safeReleaseRef(_ptr); + _ptr = other; + } + + return *this; + } + + inline operator T * () const { return _ptr; } + + inline T & operator * () const + { + CCASSERT(_ptr, "Attempt to dereference a null pointer!"); + return *_ptr; + } + + inline T * operator->() const + { + CCASSERT(_ptr, "Attempt to dereference a null pointer!"); + return _ptr; + } + + inline bool operator == (const RefPtr & other) const { return _ptr == other._ptr; } + + inline bool operator == (const T * other) const { return _ptr == other; } + + inline bool operator == (typename std::remove_const::type * other) const { return _ptr == other; } + + inline bool operator == (const std::nullptr_t other) const { return _ptr == other; } + + + inline bool operator != (const RefPtr & other) const { return _ptr != other._ptr; } + + inline bool operator != (const T * other) const { return _ptr != other; } + + inline bool operator != (typename std::remove_const::type * other) const { return _ptr != other; } + + inline bool operator != (const std::nullptr_t other) const { return _ptr != other; } + + + inline bool operator > (const RefPtr & other) const { return _ptr > other._ptr; } + + inline bool operator > (const T * other) const { return _ptr > other; } + + inline bool operator > (typename std::remove_const::type * other) const { return _ptr > other; } + + inline bool operator > (const std::nullptr_t other) const { return _ptr > other; } + + + inline bool operator < (const RefPtr & other) const { return _ptr < other._ptr; } + + inline bool operator < (const T * other) const { return _ptr < other; } + + inline bool operator < (typename std::remove_const::type * other) const { return _ptr < other; } + + inline bool operator < (const std::nullptr_t other) const { return _ptr < other; } + + + inline bool operator >= (const RefPtr & other) const { return _ptr >= other._ptr; } + + inline bool operator >= (const T * other) const { return _ptr >= other; } + + inline bool operator >= (typename std::remove_const::type * other) const { return _ptr >= other; } + + inline bool operator >= (const std::nullptr_t other) const { return _ptr >= other; } + + + inline bool operator <= (const RefPtr & other) const { return _ptr <= other._ptr; } + + inline bool operator <= (const T * other) const { return _ptr <= other; } + + inline bool operator <= (typename std::remove_const::type * other) const { return _ptr <= other; } + + inline bool operator <= (const std::nullptr_t other) const { return _ptr <= other; } + + + inline operator bool() const { return _ptr != nullptr; } + + inline T * get() const { return _ptr; } + + inline void reset() + { + RefPtrSupportFunctions::safeReleaseRef(_ptr); + _ptr = nullptr; + } + + inline void swap(RefPtr & other) + { + if (&other != this) + { + T * tmp = _ptr; + _ptr = other._ptr; + other._ptr = tmp; + } + } + + /** + * This function assigns to this RefPtr but does not increase the reference count of the object pointed to. + * Useful for assigning an object created through the 'new' operator to a RefPtr. Basically used in scenarios + * where the RefPtr has the initial ownership of the object. + * + * E.G: + * RefPtr image; + * image.weakAssign(new cocos2d::Image()); + * + * Instead of: + * RefPtr image; + * image = new cocos2d::Image(); + * image->release(); // Required because new'd object already has a reference count of '1'. + */ + inline void weakAssign(const RefPtr & other) + { + RefPtrSupportFunctions::safeReleaseRef(_ptr); + _ptr = other._ptr; + } + +private: + + T * _ptr = nullptr; +}; + +/** + * Cast between RefPtr types statically. + */ +template RefPtr static_pointer_cast(const RefPtr & r) +{ + return RefPtr(static_cast(r.get())); +} + +/** + * Cast between RefPtr types dynamically. + */ +template RefPtr dynamic_pointer_cast(const RefPtr & r) +{ + return RefPtr(dynamic_cast(r.get())); +} + +NS_CC_END + +#endif // __CC_REF_PTR_H__ diff --git a/tests/cpp-tests/Classes/UnitTest/RefPtrTest.cpp b/tests/cpp-tests/Classes/UnitTest/RefPtrTest.cpp new file mode 100644 index 0000000000..a791e6b41b --- /dev/null +++ b/tests/cpp-tests/Classes/UnitTest/RefPtrTest.cpp @@ -0,0 +1,287 @@ +#include "RefPtrTest.h" +#include "CCRefPtr.h" + +void RefPtrTest::onEnter() +{ + UnitTestDemo::onEnter(); + + // TEST(constructors) + { + // Default constructor + RefPtr ref1; + CC_ASSERT(nullptr == ref1.get()); + + // Parameter constructor + RefPtr<__String> ref2(cocos2d::String::create("Hello")); + CC_ASSERT(strcmp("Hello", ref2->getCString()) == 0); + CC_ASSERT(2 == ref2->getReferenceCount()); + + // Parameter constructor with nullptr + RefPtr<__String> ref3(nullptr); + CC_ASSERT((__String*) nullptr == ref3.get()); + + // Copy constructor + RefPtr<__String> ref4(ref2); + CC_ASSERT(strcmp("Hello", ref4->getCString()) == 0); + CC_ASSERT(3 == ref2->getReferenceCount()); + CC_ASSERT(3 == ref4->getReferenceCount()); + + // Copy constructor with nullptr reference + RefPtr ref5(ref1); + CC_ASSERT((Ref*) nullptr == ref5.get()); + } + + // TEST(destructor) + { + __String * hello = __String::create("Hello"); + CC_ASSERT(1 == hello->getReferenceCount()); + + { + RefPtr ref(hello); + CC_ASSERT(2 == hello->getReferenceCount()); + } + + CC_ASSERT(1 == hello->getReferenceCount()); + } + + // TEST(assignmentOperator) + { + // Test basic assignment with pointer + RefPtr<__String> ref1; + CC_ASSERT((Ref*) nullptr == ref1.get()); + + ref1 = __String::create("World"); + CC_ASSERT(strcmp("World", ref1->getCString()) == 0); + CC_ASSERT(2 == ref1->getReferenceCount()); + + // Assigment back to nullptr + __String * world = ref1; + CC_ASSERT(2 == world->getReferenceCount()); + + ref1 = nullptr; + CC_ASSERT(1 == world->getReferenceCount()); + + // Assignment swapping + ref1 = __String::create("Hello"); + CC_ASSERT(strcmp("Hello", ref1->getCString()) == 0); + CC_ASSERT(2 == ref1->getReferenceCount()); + + __String * hello = (__String*) ref1; + CC_ASSERT(2 == hello->getReferenceCount()); + + ref1 = world; + CC_ASSERT(1 == hello->getReferenceCount()); + CC_ASSERT(2 == world->getReferenceCount()); + + // Test assignment with another reference + RefPtr<__String> ref2 = __String::create("blah"); + __String * blah = ref2; + CC_ASSERT(2 == blah->getReferenceCount()); + + ref2 = ref1; + CC_ASSERT(1 == blah->getReferenceCount()); + CC_ASSERT(3 == world->getReferenceCount()); + } + + // TEST(castBackToPointerOperator) + { + RefPtr<__String> ref1 = __String::create("Hello"); + __String * helloPtr = ref1; + Ref * objectPtr = ref1; + + CC_ASSERT(helloPtr == objectPtr); + } + + // TEST(dereferenceOperators) + { + RefPtr<__String> ref1 = __String::create("Hello"); + CC_ASSERT(strcmp("Hello", ref1->getCString()) == 0); + CC_ASSERT(strcmp("Hello", (*ref1).getCString()) == 0); + } + + // TEST(convertToBooleanOperator) + { + RefPtr<__String> ref1 = __String::create("Hello"); + CC_ASSERT(true == (bool) ref1); + + ref1 = nullptr; + CC_ASSERT(false == (bool) ref1); + } + + // TEST(get) + { + RefPtr<__String> ref1 = __String::create("Hello"); + CC_ASSERT(strcmp("Hello", ref1.get()->getCString()) == 0); + + ref1.reset(); + CC_ASSERT((__String*) nullptr == ref1.get()); + } + + // TEST(reset) + { + RefPtr<__String> ref1; + CC_ASSERT((__String*) nullptr == ref1.get()); + + ref1.reset(); + CC_ASSERT((__String*) nullptr == ref1.get()); + + ref1 = __String::create("Hello"); + CC_ASSERT(strcmp("Hello", ref1.get()->getCString()) == 0); + + __String * hello = ref1.get(); + CC_ASSERT(2 == hello->getReferenceCount()); + + ref1.reset(); + CC_ASSERT((__String*) nullptr == ref1.get()); + CC_ASSERT(1 == hello->getReferenceCount()); + } + + // TEST(swap) + { + RefPtr<__String> ref1; + RefPtr<__String> ref2; + + CC_ASSERT((__String*) nullptr == ref1.get()); + CC_ASSERT((__String*) nullptr == ref2.get()); + + ref1.swap(ref2); + + CC_ASSERT((__String*) nullptr == ref1.get()); + CC_ASSERT((__String*) nullptr == ref2.get()); + + __String * hello = __String::create("Hello"); + CC_ASSERT(1 == hello->getReferenceCount()); + + ref1 = hello; + CC_ASSERT(2 == hello->getReferenceCount()); + + ref1.swap(ref2); + CC_ASSERT(2 == hello->getReferenceCount()); + CC_ASSERT((__String*) nullptr == ref1.get()); + CC_ASSERT(strcmp("Hello", ref2->getCString()) == 0); + + ref1.swap(ref2); + CC_ASSERT(2 == hello->getReferenceCount()); + CC_ASSERT(strcmp("Hello", ref1->getCString()) == 0); + CC_ASSERT((__String*) nullptr == ref2.get()); + + __String * world = __String::create("World"); + CC_ASSERT(1 == world->getReferenceCount()); + + ref2 = world; + CC_ASSERT(2 == world->getReferenceCount()); + + ref2.swap(ref1); + CC_ASSERT(strcmp("World", ref1->getCString()) == 0); + CC_ASSERT(strcmp("Hello", ref2->getCString()) == 0); + + CC_ASSERT(2 == hello->getReferenceCount()); + CC_ASSERT(2 == world->getReferenceCount()); + + ref1.swap(ref2); + + CC_ASSERT(strcmp("Hello", ref1->getCString()) == 0); + CC_ASSERT(strcmp("World", ref2->getCString()) == 0); + + CC_ASSERT(2 == hello->getReferenceCount()); + CC_ASSERT(2 == world->getReferenceCount()); + } + + // TEST(staticPointerCast) + { + RefPtr<__String> ref1 = __String::create("Hello"); + CC_ASSERT(2 == ref1->getReferenceCount()); + + RefPtr ref2 = static_pointer_cast(ref1); + CC_ASSERT(strcmp("Hello", ((__String*) ref2.get())->getCString()) == 0); + CC_ASSERT(3 == ref1->getReferenceCount()); + } + + // TEST(dynamicPointerCast) + { + RefPtr<__String> ref1 = cocos2d::String::create("Hello"); + CC_ASSERT(2 == ref1->getReferenceCount()); + + RefPtr ref2 = dynamic_pointer_cast(ref1); + CC_ASSERT(strcmp("Hello", ((__String*) ref2.get())->getCString()) == 0); + CC_ASSERT(3 == ref1->getReferenceCount()); + + RefPtr<__String> ref3 = dynamic_pointer_cast<__String>(ref2); + CC_ASSERT(strcmp("Hello", ref3->getCString()) == 0); + CC_ASSERT(4 == ref1->getReferenceCount()); + + RefPtr ref4 = dynamic_pointer_cast(ref2); + CC_ASSERT((Ref*) nullptr == ref4.get()); + } + + // TEST(weakAssign) + { + RefPtr<__String> ref1; + + __String * string1 = new __String("Hello"); + ref1.weakAssign(string1); + CC_ASSERT(1 == string1->getReferenceCount()); + + RefPtr<__String> ref2 = ref1; + CC_ASSERT(2 == string1->getReferenceCount()); + + __String * string2 = new __String("World"); + ref1.weakAssign(string2); + + CC_ASSERT(1 == string1->getReferenceCount()); + CC_ASSERT(1 == ref2->getReferenceCount()); + + __String * string3 = new __String("Blah"); + RefPtr<__String> ref3; + ref3.weakAssign(string3); + CC_ASSERT(1 == string3->getReferenceCount()); + + string3->retain(); + CC_ASSERT(2 == string3->getReferenceCount()); + + ref3.weakAssign(nullptr); + CC_ASSERT(1 == string3->getReferenceCount()); + CC_SAFE_RELEASE_NULL(string3); + } + + // TEST(comparisonOperators) + { + RefPtr ref1; + RefPtr ref2; + + CC_ASSERT(true == (ref1 == ref2)); + CC_ASSERT(false == (ref1 != ref2)); + CC_ASSERT(false == (ref1 < ref2)); + CC_ASSERT(false == (ref1 > ref2)); + CC_ASSERT(true == (ref1 <= ref2)); + CC_ASSERT(true == (ref1 >= ref2)); + + CC_ASSERT(true == (ref1 == nullptr)); + CC_ASSERT(false == (ref1 != nullptr)); + CC_ASSERT(false == (ref1 < nullptr)); + CC_ASSERT(false == (ref1 > nullptr)); + CC_ASSERT(true == (ref1 <= nullptr)); + CC_ASSERT(true == (ref1 >= nullptr)); + + CC_ASSERT(false == (ref1 == __String::create("Hello"))); + CC_ASSERT(true == (ref1 != __String::create("Hello"))); + CC_ASSERT(true == (ref1 < __String::create("Hello"))); + CC_ASSERT(false == (ref1 > __String::create("Hello"))); + CC_ASSERT(true == (ref1 <= __String::create("Hello"))); + CC_ASSERT(false == (ref1 >= __String::create("Hello"))); + + ref1 = __String::create("Hello"); + + CC_ASSERT(false == (ref1 == ref2)); + CC_ASSERT(true == (ref1 != ref2)); + CC_ASSERT(false == (ref1 < ref2)); + CC_ASSERT(true == (ref1 > ref2)); + CC_ASSERT(false == (ref1 <= ref2)); + CC_ASSERT(true == (ref1 >= ref2)); + } +} + +std::string RefPtrTest::subtitle() const +{ + return "RefPtrTest, should not crash!"; +} diff --git a/tests/cpp-tests/Classes/UnitTest/RefPtrTest.h b/tests/cpp-tests/Classes/UnitTest/RefPtrTest.h new file mode 100644 index 0000000000..e8004436ba --- /dev/null +++ b/tests/cpp-tests/Classes/UnitTest/RefPtrTest.h @@ -0,0 +1,17 @@ +#ifndef __REF_PTR_TEST_H__ +#define __REF_PTR_TEST_H__ + +#include "UnitTest.h" + +class RefPtrTest : public UnitTestDemo +{ +public: + + CREATE_FUNC(RefPtrTest); + + virtual void onEnter() override; + + virtual std::string subtitle() const override; +}; + +#endif /* __REF_PTR_TEST_H__ */ diff --git a/tests/cpp-tests/Classes/UnitTest/UnitTest.cpp b/tests/cpp-tests/Classes/UnitTest/UnitTest.cpp index 57392bc388..ae7677c1b5 100644 --- a/tests/cpp-tests/Classes/UnitTest/UnitTest.cpp +++ b/tests/cpp-tests/Classes/UnitTest/UnitTest.cpp @@ -1,11 +1,13 @@ #include "UnitTest.h" +#include "RefPtrTest.h" // For ' < o > ' multiply test scene. static std::function createFunctions[] = { CL(TemplateVectorTest), CL(TemplateMapTest), - CL(ValueTest) + CL(ValueTest), + CL(RefPtrTest) }; static int sceneIdx = -1; From 586383a05ed6aa7a33f358b45c17acd0e7255215 Mon Sep 17 00:00:00 2001 From: Darragh Coy Date: Wed, 26 Mar 2014 16:30:04 -0700 Subject: [PATCH 02/10] Fix Android build for RefPtr addition. --- cocos/2d/Android.mk | 1 + tests/cpp-tests/Android.mk | 1 + 2 files changed, 2 insertions(+) diff --git a/cocos/2d/Android.mk b/cocos/2d/Android.mk index cf07e20906..32f619cf84 100644 --- a/cocos/2d/Android.mk +++ b/cocos/2d/Android.mk @@ -137,6 +137,7 @@ renderer/CCRenderMaterial.cpp \ ../base/CCGeometry.cpp \ ../base/CCNS.cpp \ ../base/CCRef.cpp \ +../base/CCRefPtr.cpp \ ../base/CCSet.cpp \ ../base/CCString.cpp \ ../base/CCValue.cpp \ diff --git a/tests/cpp-tests/Android.mk b/tests/cpp-tests/Android.mk index c2d2c02978..7fd692f58f 100644 --- a/tests/cpp-tests/Android.mk +++ b/tests/cpp-tests/Android.mk @@ -171,6 +171,7 @@ Classes/TouchesTest/Ball.cpp \ Classes/TouchesTest/Paddle.cpp \ Classes/TouchesTest/TouchesTest.cpp \ Classes/TransitionsTest/TransitionsTest.cpp \ +Classes/UnitTest/RefPtrTest.cpp \ Classes/UnitTest/UnitTest.cpp \ Classes/UserDefaultTest/UserDefaultTest.cpp \ Classes/ZwoptexTest/ZwoptexTest.cpp From f147ef723a8a459b3a05ab51eae7460dc922a16c Mon Sep 17 00:00:00 2001 From: Darragh Coy Date: Mon, 7 Apr 2014 16:04:00 -0700 Subject: [PATCH 03/10] Merge the alternate RefPtr implementation Merge in the version at: https://github.com/DarraghCoy/cocos2d-x/blob/add_refptr_class_alt_impl/cocos/base/CCRefPtr.h This version sacrifices the ability to use forward references in some places in return for added type safety and ensures we can never assign a non cocos2d::Ref derived object to the pointer. --- .../project.pbxproj.REMOVED.git-id | 2 +- cocos/base/CCRefPtr.cpp | 51 -------- cocos/base/CCRefPtr.h | 118 +++++++++++++----- 3 files changed, 85 insertions(+), 86 deletions(-) delete mode 100644 cocos/base/CCRefPtr.cpp diff --git a/build/cocos2d_libs.xcodeproj/project.pbxproj.REMOVED.git-id b/build/cocos2d_libs.xcodeproj/project.pbxproj.REMOVED.git-id index f8f068ce4c..517324a4e4 100644 --- a/build/cocos2d_libs.xcodeproj/project.pbxproj.REMOVED.git-id +++ b/build/cocos2d_libs.xcodeproj/project.pbxproj.REMOVED.git-id @@ -1 +1 @@ -f722a4d89963f2d2a58b754e12d1baa2bf0c5ece \ No newline at end of file +a6890abb8134492d61129c585fbd28941e825a6f \ No newline at end of file diff --git a/cocos/base/CCRefPtr.cpp b/cocos/base/CCRefPtr.cpp deleted file mode 100644 index 0cea720a83..0000000000 --- a/cocos/base/CCRefPtr.cpp +++ /dev/null @@ -1,51 +0,0 @@ -/**************************************************************************** - Copyright (c) 2014 PlayFirst Inc. - Copyright (c) 2014 Chukong Technologies Inc. - - http://www.cocos2d-x.org - - Permission is hereby granted, free of charge, to any person obtaining a copy - of this software and associated documentation files (the "Software"), to deal - in the Software without restriction, including without limitation the rights - to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - copies of the Software, and to permit persons to whom the Software is - furnished to do so, subject to the following conditions: - - The above copyright notice and this permission notice shall be included in - all copies or substantial portions of the Software. - - THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - THE SOFTWARE. - ****************************************************************************/ - -#include "CCRefPtr.h" -#include "CCRef.h" - -NS_CC_BEGIN - -void RefPtrSupportFunctions::safeRetainRef(const void * refPtr) -{ - if (refPtr) - { - Ref * refPtrCast = reinterpret_cast(const_cast(refPtr)); - CC_ASSERT(dynamic_cast(refPtrCast)); // Check for invalid type pointed to in RefPtr. Object pointed to must be a cocos2d::Ref! - refPtrCast->retain(); - } -} - -void RefPtrSupportFunctions::safeReleaseRef(const void * refPtr) -{ - if (refPtr) - { - Ref * refPtrCast = reinterpret_cast(const_cast(refPtr)); - CC_ASSERT(dynamic_cast(refPtrCast)); // Check for invalid type pointed to in RefPtr. Object pointed to must be a cocos2d::Ref! - refPtrCast->release(); - } -} - -NS_CC_END diff --git a/cocos/base/CCRefPtr.h b/cocos/base/CCRefPtr.h index 4ffba53bbf..e8906aadf3 100644 --- a/cocos/base/CCRefPtr.h +++ b/cocos/base/CCRefPtr.h @@ -26,23 +26,48 @@ #ifndef __CC_REF_PTR_H__ #define __CC_REF_PTR_H__ -#include "CCPlatformMacros.h" -#include "ccConfig.h" +#include "CCRef.h" #include NS_CC_BEGIN -class Ref; - /** - * Utility/support functions for RefPtr. + * Utility/support macros. Defined to enable RefPtr to contain types like 'const T' because we do not + * regard retain()/release() as affecting mutability of state. */ -namespace RefPtrSupportFunctions -{ - void safeRetainRef(const void * refPtr); - - void safeReleaseRef(const void * refPtr); -} +#define CC_REF_PTR_SAFE_RETAIN(ptr)\ + \ + do\ + {\ + if (ptr)\ + {\ + const_cast(static_cast(ptr))->retain();\ + }\ + \ + } while (0); + +#define CC_REF_PTR_SAFE_RELEASE(ptr)\ + \ + do\ + {\ + if (ptr)\ + {\ + const_cast(static_cast(ptr))->release();\ + }\ + \ + } while (0); + +#define CC_REF_PTR_SAFE_RELEASE_NULL(ptr)\ + \ + do\ + {\ + if (ptr)\ + {\ + const_cast(static_cast(ptr))->release();\ + ptr = nullptr;\ + }\ + \ + } while (0); /** * Wrapper class which maintains a strong reference to a cocos2dx cocos2d::Ref* type object. @@ -57,67 +82,89 @@ namespace RefPtrSupportFunctions template class RefPtr { public: + RefPtr() + : _ptr(nullptr) + { + } - RefPtr() = default; - inline RefPtr(T * ptr) : - _ptr(ptr) + _ptr(const_cast::type*>(ptr)) // Const cast allows RefPtr to reference objects marked const too. { - RefPtrSupportFunctions::safeRetainRef(_ptr); + CC_REF_PTR_SAFE_RETAIN(_ptr); + } + + inline RefPtr(std::nullptr_t ptr) + : + _ptr(nullptr) + { + } inline RefPtr(const RefPtr & other) : _ptr(other._ptr) { - RefPtrSupportFunctions::safeRetainRef(_ptr); + CC_REF_PTR_SAFE_RETAIN(_ptr); } inline ~RefPtr() { - RefPtrSupportFunctions::safeReleaseRef(_ptr); - _ptr = nullptr; + CC_REF_PTR_SAFE_RELEASE_NULL(_ptr); } inline RefPtr & operator = (const RefPtr & other) { if (other._ptr != _ptr) { - RefPtrSupportFunctions::safeRetainRef(other._ptr); - RefPtrSupportFunctions::safeReleaseRef(_ptr); + CC_REF_PTR_SAFE_RETAIN(other._ptr); + CC_REF_PTR_SAFE_RELEASE(_ptr); _ptr = other._ptr; } return *this; } + inline RefPtr & operator = (std::nullptr_t other) + { + CC_REF_PTR_SAFE_RELEASE_NULL(_ptr); + return *this; + } + inline RefPtr & operator = (T * other) { if (other != _ptr) { - RefPtrSupportFunctions::safeRetainRef(other); - RefPtrSupportFunctions::safeReleaseRef(_ptr); - _ptr = other; + CC_REF_PTR_SAFE_RETAIN(other); + CC_REF_PTR_SAFE_RELEASE(_ptr); + _ptr = const_cast::type*>(other); // Const cast allows RefPtr to reference objects marked const too. } return *this; } - inline operator T * () const { return _ptr; } + // Note: using reinterpret_cast<> instead of static_cast<> here because it doesn't require type info. + // Since we verify the correct type cast at compile time on construction/assign we don't need to know the type info + // here. Not needing the type info here enables us to use these operations in inline functions in header files when + // the type pointed to by this class is only forward referenced. + + inline operator T * () const { return reinterpret_cast(_ptr); } inline T & operator * () const { CCASSERT(_ptr, "Attempt to dereference a null pointer!"); - return *_ptr; + return reinterpret_cast(*_ptr); } inline T * operator->() const { CCASSERT(_ptr, "Attempt to dereference a null pointer!"); - return _ptr; + return reinterpret_cast(_ptr); } + inline T * get() const { return reinterpret_cast(_ptr); } + + inline bool operator == (const RefPtr & other) const { return _ptr == other._ptr; } inline bool operator == (const T * other) const { return _ptr == other; } @@ -174,19 +221,16 @@ public: inline operator bool() const { return _ptr != nullptr; } - inline T * get() const { return _ptr; } - inline void reset() { - RefPtrSupportFunctions::safeReleaseRef(_ptr); - _ptr = nullptr; + CC_REF_PTR_SAFE_RELEASE_NULL(_ptr); } inline void swap(RefPtr & other) { if (&other != this) { - T * tmp = _ptr; + Ref * tmp = _ptr; _ptr = other._ptr; other._ptr = tmp; } @@ -208,13 +252,12 @@ public: */ inline void weakAssign(const RefPtr & other) { - RefPtrSupportFunctions::safeReleaseRef(_ptr); + CC_REF_PTR_SAFE_RELEASE(_ptr); _ptr = other._ptr; } private: - - T * _ptr = nullptr; + Ref * _ptr; }; /** @@ -233,6 +276,13 @@ template RefPtr dynamic_pointer_cast(const RefPtr & r) return RefPtr(dynamic_cast(r.get())); } +/** + * Done with these macros. + */ +#undef CC_REF_PTR_SAFE_RETAIN +#undef CC_REF_PTR_SAFE_RELEASE +#undef CC_REF_PTR_SAFE_RELEASE_NULL + NS_CC_END #endif // __CC_REF_PTR_H__ From a34b952e8db681421271b5dec27ff9ec60dddd05 Mon Sep 17 00:00:00 2001 From: Darragh Coy Date: Tue, 8 Apr 2014 10:35:55 -0700 Subject: [PATCH 04/10] Revert "Fix Android build for RefPtr addition." This reverts commit 586383a05ed6aa7a33f358b45c17acd0e7255215. --- cocos/2d/Android.mk | 1 - tests/cpp-tests/Android.mk | 1 - 2 files changed, 2 deletions(-) diff --git a/cocos/2d/Android.mk b/cocos/2d/Android.mk index 32f619cf84..cf07e20906 100644 --- a/cocos/2d/Android.mk +++ b/cocos/2d/Android.mk @@ -137,7 +137,6 @@ renderer/CCRenderMaterial.cpp \ ../base/CCGeometry.cpp \ ../base/CCNS.cpp \ ../base/CCRef.cpp \ -../base/CCRefPtr.cpp \ ../base/CCSet.cpp \ ../base/CCString.cpp \ ../base/CCValue.cpp \ diff --git a/tests/cpp-tests/Android.mk b/tests/cpp-tests/Android.mk index 7fd692f58f..c2d2c02978 100644 --- a/tests/cpp-tests/Android.mk +++ b/tests/cpp-tests/Android.mk @@ -171,7 +171,6 @@ Classes/TouchesTest/Ball.cpp \ Classes/TouchesTest/Paddle.cpp \ Classes/TouchesTest/TouchesTest.cpp \ Classes/TransitionsTest/TransitionsTest.cpp \ -Classes/UnitTest/RefPtrTest.cpp \ Classes/UnitTest/UnitTest.cpp \ Classes/UserDefaultTest/UserDefaultTest.cpp \ Classes/ZwoptexTest/ZwoptexTest.cpp From 440a0adcde4996e7b46c4a5ec9817f86d5c4dbc0 Mon Sep 17 00:00:00 2001 From: Darragh Coy Date: Tue, 8 Apr 2014 10:40:32 -0700 Subject: [PATCH 05/10] Whoops... Shouldn't remove the test cpp. --- tests/cpp-tests/Android.mk | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/cpp-tests/Android.mk b/tests/cpp-tests/Android.mk index c2d2c02978..7fd692f58f 100644 --- a/tests/cpp-tests/Android.mk +++ b/tests/cpp-tests/Android.mk @@ -171,6 +171,7 @@ Classes/TouchesTest/Ball.cpp \ Classes/TouchesTest/Paddle.cpp \ Classes/TouchesTest/TouchesTest.cpp \ Classes/TransitionsTest/TransitionsTest.cpp \ +Classes/UnitTest/RefPtrTest.cpp \ Classes/UnitTest/UnitTest.cpp \ Classes/UserDefaultTest/UserDefaultTest.cpp \ Classes/ZwoptexTest/ZwoptexTest.cpp From 14b210a08272f4de695136ee7fc9aaca9d99d0e9 Mon Sep 17 00:00:00 2001 From: Darragh Coy Date: Tue, 8 Apr 2014 11:34:27 -0700 Subject: [PATCH 06/10] RefPtr - add move semantics Add support for move semantics to the RefPtr class. Implement a move constructor and a move assignment operator. --- cocos/base/CCRefPtr.h | 33 +++++++++++++---- .../cpp-tests/Classes/UnitTest/RefPtrTest.cpp | 35 +++++++++++++++++++ 2 files changed, 62 insertions(+), 6 deletions(-) diff --git a/cocos/base/CCRefPtr.h b/cocos/base/CCRefPtr.h index e8906aadf3..4a965f869f 100644 --- a/cocos/base/CCRefPtr.h +++ b/cocos/base/CCRefPtr.h @@ -82,10 +82,19 @@ NS_CC_BEGIN template class RefPtr { public: - RefPtr() - : _ptr(nullptr) + + inline RefPtr() + : + _ptr(nullptr) { + } + + inline RefPtr(RefPtr && other) + { + _ptr = other._ptr; + other._ptr = nullptr; + } inline RefPtr(T * ptr) : @@ -113,7 +122,7 @@ public: CC_REF_PTR_SAFE_RELEASE_NULL(_ptr); } - inline RefPtr & operator = (const RefPtr & other) + inline RefPtr & operator = (const RefPtr & other) { if (other._ptr != _ptr) { @@ -125,13 +134,19 @@ public: return *this; } - inline RefPtr & operator = (std::nullptr_t other) + inline RefPtr & operator = (RefPtr && other) { - CC_REF_PTR_SAFE_RELEASE_NULL(_ptr); + if (&other != this) + { + CC_REF_PTR_SAFE_RELEASE(_ptr); + _ptr = other._ptr; + other._ptr = nullptr; + } + return *this; } - inline RefPtr & operator = (T * other) + inline RefPtr & operator = (T * other) { if (other != _ptr) { @@ -143,6 +158,12 @@ public: return *this; } + inline RefPtr & operator = (std::nullptr_t other) + { + CC_REF_PTR_SAFE_RELEASE_NULL(_ptr); + return *this; + } + // Note: using reinterpret_cast<> instead of static_cast<> here because it doesn't require type info. // Since we verify the correct type cast at compile time on construction/assign we don't need to know the type info // here. Not needing the type info here enables us to use these operations in inline functions in header files when diff --git a/tests/cpp-tests/Classes/UnitTest/RefPtrTest.cpp b/tests/cpp-tests/Classes/UnitTest/RefPtrTest.cpp index a791e6b41b..18df8c1535 100644 --- a/tests/cpp-tests/Classes/UnitTest/RefPtrTest.cpp +++ b/tests/cpp-tests/Classes/UnitTest/RefPtrTest.cpp @@ -279,6 +279,41 @@ void RefPtrTest::onEnter() CC_ASSERT(false == (ref1 <= ref2)); CC_ASSERT(true == (ref1 >= ref2)); } + + // TEST(moveConstructor) + { + auto someFunc = []() -> RefPtr<__String> + { + return __String::create("Hello world!"); + }; + + // Note: std::move will turn the rvalue into an rvalue reference and thus cause the move constructor to be invoked. + // Have to use this because the compiler will try and optimize how we handle the return value otherwise and skip the move constructor. + RefPtr<__String> theString(std::move(someFunc())); + CC_ASSERT(theString->getReferenceCount() == 2); + CC_ASSERT(theString->compare("Hello world!") == 0); + + __String * theStringPtr = theString; + theString.reset(); + CC_ASSERT(theStringPtr->getReferenceCount() == 1); + CC_ASSERT(theStringPtr->compare("Hello world!") == 0); + } + + // TEST(moveAssignmentOperator) + { + auto someFunc = []() -> RefPtr<__String> + { + return __String::create("Hello world!"); + }; + + RefPtr<__String> theString(someFunc()); + CC_ASSERT(theString->getReferenceCount() == 2); + CC_ASSERT(theString->compare("Hello world!") == 0); + + theString = someFunc(); // No need to use std::move here, compiler should figure out that move semantics are appropriate for this statement. + CC_ASSERT(theString->getReferenceCount() == 2); + CC_ASSERT(theString->compare("Hello world!") == 0); + } } std::string RefPtrTest::subtitle() const From cbf85259cd92cb49ee6e7606931fcf2a28f5f505 Mon Sep 17 00:00:00 2001 From: Darragh Coy Date: Tue, 8 Apr 2014 11:44:54 -0700 Subject: [PATCH 07/10] Convert tabs to spaces --- cocos/base/CCRefPtr.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cocos/base/CCRefPtr.h b/cocos/base/CCRefPtr.h index 4a965f869f..8860f4a898 100644 --- a/cocos/base/CCRefPtr.h +++ b/cocos/base/CCRefPtr.h @@ -84,11 +84,11 @@ template class RefPtr public: inline RefPtr() - : + : _ptr(nullptr) - { + { - } + } inline RefPtr(RefPtr && other) { From 93724b3806b5cd1ee72584cf68fc76682b108785 Mon Sep 17 00:00:00 2001 From: Darragh Coy Date: Tue, 8 Apr 2014 12:34:22 -0700 Subject: [PATCH 08/10] Add in missing template parameter in function arg. --- cocos/base/CCRefPtr.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cocos/base/CCRefPtr.h b/cocos/base/CCRefPtr.h index 8860f4a898..2a082fb173 100644 --- a/cocos/base/CCRefPtr.h +++ b/cocos/base/CCRefPtr.h @@ -90,7 +90,7 @@ public: } - inline RefPtr(RefPtr && other) + inline RefPtr(RefPtr && other) { _ptr = other._ptr; other._ptr = nullptr; From d64603a8560219bd1d697dc96fa70e1d2b3ceb42 Mon Sep 17 00:00:00 2001 From: Darragh Coy Date: Tue, 8 Apr 2014 22:29:40 -0700 Subject: [PATCH 09/10] Attempt to fix the linux build. --- tests/cpp-tests/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/cpp-tests/CMakeLists.txt b/tests/cpp-tests/CMakeLists.txt index 7a36508532..22995708d1 100644 --- a/tests/cpp-tests/CMakeLists.txt +++ b/tests/cpp-tests/CMakeLists.txt @@ -172,6 +172,7 @@ set(SAMPLE_SRC Classes/DataVisitorTest/DataVisitorTest.cpp Classes/ConfigurationTest/ConfigurationTest.cpp Classes/ConsoleTest/ConsoleTest.cpp + Classes/UnitTest/RefPtrTest.cpp Classes/UnitTest/UnitTest.cpp Classes/controller.cpp Classes/testBasic.cpp From 50be1ca3adfe18dc5ed903d7a15b9eca4a010ca2 Mon Sep 17 00:00:00 2001 From: Darragh Coy Date: Tue, 8 Apr 2014 22:47:08 -0700 Subject: [PATCH 10/10] Fix win32 build --- tests/cpp-tests/proj.win32/cpp-tests.vcxproj | 4 +++- tests/cpp-tests/proj.win32/cpp-tests.vcxproj.filters | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/cpp-tests/proj.win32/cpp-tests.vcxproj b/tests/cpp-tests/proj.win32/cpp-tests.vcxproj index 172ef28da3..3ee0a04438 100644 --- a/tests/cpp-tests/proj.win32/cpp-tests.vcxproj +++ b/tests/cpp-tests/proj.win32/cpp-tests.vcxproj @@ -223,6 +223,7 @@ xcopy /Y /Q "$(ProjectDir)..\..\..\external\websockets\prebuilt\win32\*.*" "$(Ou + @@ -401,6 +402,7 @@ xcopy /Y /Q "$(ProjectDir)..\..\..\external\websockets\prebuilt\win32\*.*" "$(Ou + @@ -583,4 +585,4 @@ xcopy /Y /Q "$(ProjectDir)..\..\..\external\websockets\prebuilt\win32\*.*" "$(Ou - + \ No newline at end of file diff --git a/tests/cpp-tests/proj.win32/cpp-tests.vcxproj.filters b/tests/cpp-tests/proj.win32/cpp-tests.vcxproj.filters index a6317dd4a6..0cf88b687e 100644 --- a/tests/cpp-tests/proj.win32/cpp-tests.vcxproj.filters +++ b/tests/cpp-tests/proj.win32/cpp-tests.vcxproj.filters @@ -831,6 +831,9 @@ Classes\ShaderTest + + Classes\UnitTest + @@ -1535,5 +1538,8 @@ Classes\ShaderTest + + Classes\UnitTest + \ No newline at end of file