From 2089adc62a671e5c06f8dfae19c942506ef2872f Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Thu, 12 Nov 2020 11:47:59 +0000 Subject: [PATCH] Move most of Ref's functionality in RefBase for reuse. This will allow using the same logic for other kinds of smartpointers, like NSRef<> Bug: dawn:89 Change-Id: Idbe08208fdb38b236f52635bc913162e60baf0f0 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/32160 Reviewed-by: Stephen White Commit-Queue: Corentin Wallez --- src/common/BUILD.gn | 1 + src/common/CMakeLists.txt | 1 + src/common/RefBase.h | 189 ++++++++++++++++++++++++ src/common/RefCounted.h | 155 ++----------------- src/dawn_native/vulkan/TextureVk.cpp | 2 +- src/tests/unittests/RefCountedTests.cpp | 9 +- 6 files changed, 209 insertions(+), 148 deletions(-) create mode 100644 src/common/RefBase.h diff --git a/src/common/BUILD.gn b/src/common/BUILD.gn index 6e4d90ce0e..11bb461dae 100644 --- a/src/common/BUILD.gn +++ b/src/common/BUILD.gn @@ -169,6 +169,7 @@ if (is_win || is_linux || is_chromeos || is_mac || is_fuchsia || is_android) { "Math.h", "PlacementAllocated.h", "Platform.h", + "RefBase.h", "RefCounted.cpp", "RefCounted.h", "Result.cpp", diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index f19818642a..63a5c24df2 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -31,6 +31,7 @@ target_sources(dawn_common PRIVATE "Math.h" "PlacementAllocated.h" "Platform.h" + "RefBase.h" "RefCounted.cpp" "RefCounted.h" "Result.cpp" diff --git a/src/common/RefBase.h b/src/common/RefBase.h new file mode 100644 index 0000000000..1f4705466d --- /dev/null +++ b/src/common/RefBase.h @@ -0,0 +1,189 @@ +// Copyright 2020 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef COMMON_REFBASE_H_ +#define COMMON_REFBASE_H_ + +#include "common/Compiler.h" + +#include + +// A common class for various smart-pointers acting on referenceable/releasable pointer-like +// objects. Logic for each specialization can be customized using a Traits type that looks +// like the following: +// +// struct { +// static constexpr T kNullValue = ...; +// static void Reference(T value) { ... } +// static void Release(T value) { ... } +// }; +// +// RefBase supports +template +class RefBase { + private: + static constexpr T kNullValue = Traits::kNullValue; + + public: + // Default constructor and destructor. + RefBase() : mValue(kNullValue) { + } + + ~RefBase() { + Release(); + mValue = kNullValue; + } + + // Constructors from nullptr. + constexpr RefBase(std::nullptr_t) : RefBase() { + } + + RefBase& operator=(std::nullptr_t) { + Release(); + mValue = kNullValue; + return *this; + } + + // Constructors from a value T. + RefBase(T value) : mValue(value) { + Reference(); + } + + RefBase& operator=(const T& value) { + mValue = value; + Reference(); + return *this; + } + + // Constructors from a RefBase + RefBase(const RefBase& other) : mValue(other.mValue) { + Reference(); + } + + RefBase& operator=(const RefBase& other) { + if (&other != this) { + other.Reference(); + Release(); + mValue = other.mValue; + } + + return *this; + } + + RefBase(RefBase&& other) { + mValue = other.mValue; + other.mValue = kNullValue; + } + + RefBase& operator=(RefBase&& other) { + if (&other != this) { + Release(); + mValue = other.mValue; + other.mValue = kNullValue; + } + + return *this; + } + + // Constructors from a RefBase. Note that in the *-assignment operators this cannot be the + // same as `other` because overload resolution rules would have chosen the *-assignement + // operators defined with `other` == RefBase. + template ::type> + RefBase(const RefBase& other) : mValue(other.mValue) { + Reference(); + } + + template ::type> + RefBase& operator=(const RefBase& other) { + other.Reference(); + Release(); + mValue = other.mValue; + return *this; + } + + template ::type> + RefBase(RefBase&& other) { + mValue = other.Detach(); + } + + template ::type> + RefBase& operator=(RefBase&& other) { + Release(); + mValue = other.Detach(); + return *this; + } + + // Comparison operators. + bool operator==(const T& other) const { + return mValue == other; + } + + bool operator!=(const T& other) const { + return mValue != other; + } + + operator bool() { + return mValue != kNullValue; + } + + // Operator * and -> to act like a pointer. + const typename Traits::PointedType& operator*() const { + return *mValue; + } + typename Traits::PointedType& operator*() { + return *mValue; + } + + const T operator->() const { + return mValue; + } + T operator->() { + return mValue; + } + + // Smart pointer methods. + const T& Get() const { + return mValue; + } + T& Get() { + return mValue; + } + + T Detach() DAWN_NO_DISCARD { + T value = mValue; + mValue = kNullValue; + return value; + } + + private: + // Friend is needed so that instances of RefBase can call Reference and Release on + // RefBase. + template + friend class RefBase; + + void Reference() const { + if (mValue != kNullValue) { + Traits::Reference(mValue); + } + } + void Release() const { + if (mValue != kNullValue) { + Traits::Release(mValue); + } + } + + T mValue; +}; + +#endif // COMMON_REFBASE_H_ diff --git a/src/common/RefCounted.h b/src/common/RefCounted.h index b8e2ce8f82..5b36ca8e1a 100644 --- a/src/common/RefCounted.h +++ b/src/common/RefCounted.h @@ -15,6 +15,8 @@ #ifndef COMMON_REFCOUNTED_H_ #define COMMON_REFCOUNTED_H_ +#include "common/RefBase.h" + #include #include @@ -39,146 +41,21 @@ class RefCounted { }; template -class Ref { +struct RefCountedTraits { + using PointedType = T; + static constexpr T* kNullValue = nullptr; + static void Reference(T* value) { + value->Reference(); + } + static void Release(T* value) { + value->Release(); + } +}; + +template +class Ref : public RefBase> { public: - Ref() = default; - - constexpr Ref(std::nullptr_t) { - } - - Ref& operator=(std::nullptr_t) { - Release(); - mPointee = nullptr; - return *this; - } - - template - Ref(U* p) : mPointee(p) { - static_assert(std::is_convertible::value, ""); - Reference(); - } - - Ref(const Ref& other) : mPointee(other.mPointee) { - Reference(); - } - template - Ref(const Ref& other) : mPointee(other.mPointee) { - static_assert(std::is_convertible::value, ""); - Reference(); - } - - Ref& operator=(const Ref& other) { - if (&other == this) - return *this; - - other.Reference(); - Release(); - mPointee = other.mPointee; - - return *this; - } - - template - Ref& operator=(const Ref& other) { - static_assert(std::is_convertible::value, ""); - - other.Reference(); - Release(); - mPointee = other.mPointee; - - return *this; - } - - template - Ref(Ref&& other) { - static_assert(std::is_convertible::value, ""); - mPointee = other.mPointee; - other.mPointee = nullptr; - } - - Ref& operator=(Ref&& other) { - if (&other == this) - return *this; - - Release(); - mPointee = other.mPointee; - other.mPointee = nullptr; - - return *this; - } - template - Ref& operator=(Ref&& other) { - static_assert(std::is_convertible::value, ""); - - Release(); - mPointee = other.mPointee; - other.mPointee = nullptr; - - return *this; - } - - ~Ref() { - Release(); - mPointee = nullptr; - } - - bool operator==(const T* other) const { - return mPointee == other; - } - - bool operator!=(const T* other) const { - return mPointee != other; - } - - operator bool() { - return mPointee != nullptr; - } - - const T& operator*() const { - return *mPointee; - } - T& operator*() { - return *mPointee; - } - - const T* operator->() const { - return mPointee; - } - T* operator->() { - return mPointee; - } - - const T* Get() const { - return mPointee; - } - T* Get() { - return mPointee; - } - - T* Detach() { - T* pointee = mPointee; - mPointee = nullptr; - return pointee; - } - - private: - // Friend is needed so that instances of Ref can assign mPointee - // members of Ref. - template - friend class Ref; - - void Reference() const { - if (mPointee != nullptr) { - mPointee->Reference(); - } - } - void Release() const { - if (mPointee != nullptr) { - mPointee->Release(); - } - } - - T* mPointee = nullptr; + using RefBase>::RefBase; }; template diff --git a/src/dawn_native/vulkan/TextureVk.cpp b/src/dawn_native/vulkan/TextureVk.cpp index 97a20a4c88..1b4840de4e 100644 --- a/src/dawn_native/vulkan/TextureVk.cpp +++ b/src/dawn_native/vulkan/TextureVk.cpp @@ -483,7 +483,7 @@ namespace dawn_native { namespace vulkan { Ref texture = AcquireRef(new Texture(device, descriptor, TextureState::OwnedExternal)); texture->InitializeForSwapChain(nativeImage); - return std::move(texture); + return texture; } MaybeError Texture::InitializeAsInternalTexture(VkImageUsageFlags extraUsages) { diff --git a/src/tests/unittests/RefCountedTests.cpp b/src/tests/unittests/RefCountedTests.cpp index 7e5c959c52..3096d68485 100644 --- a/src/tests/unittests/RefCountedTests.cpp +++ b/src/tests/unittests/RefCountedTests.cpp @@ -43,14 +43,7 @@ class RCTest : public RefCounted { }; struct RCTestDerived : public RCTest { - RCTestDerived() : RCTest() { - } - - RCTestDerived(uint64_t payload) : RCTest(payload) { - } - - RCTestDerived(bool* deleted) : RCTest(deleted) { - } + using RCTest::RCTest; }; // Test that RCs start with one ref, and removing it destroys the object.