Allow Ref<Derived> -> Ref<Base> assignment, and move operations

Previous to this change, you were unable to assign or move Ref<Derived>
to a Ref<Base>.

This change addresses the problem by introducing <typename U> versions
of assignment, copy and move methods. nullptr_t specific ones were
also added to disambiguate things for the compiler.

Bug:dawn:390
Change-Id: Ib5d44231e26db35de33d63c67b36b5bf411a3540
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/20121
Commit-Queue: Rafael Cintron <rafael.cintron@microsoft.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

src/common/RefCounted.h

@@ -41,16 +41,32 @@ class RefCounted {
 template <typename T>
 class Ref {
   public:
-    Ref() {
+    Ref() = default;
+
+    constexpr Ref(std::nullptr_t) {
     }
 
-    Ref(T* p) : mPointee(p) {
+    Ref& operator=(std::nullptr_t) {
+        Release();
+        mPointee = nullptr;
+        return *this;
+    }
+
+    template <typename U>
+    Ref(U* p) : mPointee(p) {
+        static_assert(std::is_convertible<U*, T*>::value, "");
         Reference();
     }
 
     Ref(const Ref<T>& other) : mPointee(other.mPointee) {
         Reference();
     }
+    template <typename U>
+    Ref(const Ref<U>& other) : mPointee(other.mPointee) {
+        static_assert(std::is_convertible<U*, T*>::value, "");
+        Reference();
+    }
+
     Ref<T>& operator=(const Ref<T>& other) {
         if (&other == this)
             return *this;
@@ -62,10 +78,24 @@ class Ref {
         return *this;
     }
 
-    Ref(Ref<T>&& other) {
+    template <typename U>
+    Ref<T>& operator=(const Ref<U>& other) {
+        static_assert(std::is_convertible<U*, T*>::value, "");
+
+        other.Reference();
+        Release();
+        mPointee = other.mPointee;
+
+        return *this;
+    }
+
+    template <typename U>
+    Ref(Ref<U>&& other) {
+        static_assert(std::is_convertible<U*, T*>::value, "");
         mPointee = other.mPointee;
         other.mPointee = nullptr;
     }
+
     Ref<T>& operator=(Ref<T>&& other) {
         if (&other == this)
             return *this;
@@ -76,6 +106,16 @@ class Ref {
 
         return *this;
     }
+    template <typename U>
+    Ref<T>& operator=(Ref<U>&& other) {
+        static_assert(std::is_convertible<U*, T*>::value, "");
+
+        Release();
+        mPointee = other.mPointee;
+        other.mPointee = nullptr;
+
+        return *this;
+    }
 
     ~Ref() {
         Release();
@@ -114,6 +154,11 @@ class Ref {
     }
 
   private:
+    // Friend is needed so that instances of Ref<U> can assign mPointee
+    // members of Ref<T>.
+    template <typename U>
+    friend class Ref;
+
     void Reference() const {
         if (mPointee != nullptr) {
             mPointee->Reference();
@@ -125,7 +170,6 @@ class Ref {
         }
     }
 
-    // static_assert(std::is_base_of<RefCounted, T>::value, "");
     T* mPointee = nullptr;
 };
 

src/tests/unittests/RefCountedTests.cpp

@@ -17,19 +17,20 @@
 
 #include "common/RefCounted.h"
 
-struct RCTest : public RefCounted {
+class RCTest : public RefCounted {
+  public:
     RCTest() : RefCounted() {
     }
 
     RCTest(uint64_t payload) : RefCounted(payload) {
     }
 
-    RCTest(bool* deleted) : deleted(deleted) {
+    RCTest(bool* deleted) : mDeleted(deleted) {
     }
 
     ~RCTest() override {
-        if (deleted != nullptr) {
+        if (mDeleted != nullptr) {
-            *deleted = true;
+            *mDeleted = true;
         }
     }
 
@@ -37,7 +38,19 @@ struct RCTest : public RefCounted {
         return this;
     }
 
-    bool* deleted = nullptr;
+  private:
+    bool* mDeleted = nullptr;
+};
+
+struct RCTestDerived : public RCTest {
+    RCTestDerived() : RCTest() {
+    }
+
+    RCTestDerived(uint64_t payload) : RCTest(payload) {
+    }
+
+    RCTestDerived(bool* deleted) : RCTest(deleted) {
+    }
 };
 
 // Test that RCs start with one ref, and removing it destroys the object.
@@ -142,14 +155,21 @@ TEST(Ref, CopyConstructor) {
     RCTest* original = new RCTest(&deleted);
 
     Ref<RCTest> source(original);
+    EXPECT_EQ(original->GetRefCountForTesting(), 2u);
+
     Ref<RCTest> destination(source);
+    EXPECT_EQ(original->GetRefCountForTesting(), 3u);
+
     original->Release();
+    EXPECT_EQ(original->GetRefCountForTesting(), 2u);
 
     EXPECT_EQ(source.Get(), original);
     EXPECT_EQ(destination.Get(), original);
 
     source = nullptr;
     EXPECT_FALSE(deleted);
+    EXPECT_EQ(original->GetRefCountForTesting(), 1u);
+
     destination = nullptr;
     EXPECT_TRUE(deleted);
 }
@@ -182,8 +202,13 @@ TEST(Ref, MoveConstructor) {
     RCTest* original = new RCTest(&deleted);
 
     Ref<RCTest> source(original);
+    EXPECT_EQ(original->GetRefCountForTesting(), 2u);
+
     Ref<RCTest> destination(std::move(source));
+    EXPECT_EQ(original->GetRefCountForTesting(), 2u);
+
     original->Release();
+    EXPECT_EQ(original->GetRefCountForTesting(), 1u);
 
     EXPECT_EQ(source.Get(), nullptr);
     EXPECT_EQ(destination.Get(), original);
@@ -199,10 +224,14 @@ TEST(Ref, MoveAssignment) {
     RCTest* original = new RCTest(&deleted);
 
     Ref<RCTest> source(original);
+    EXPECT_EQ(original->GetRefCountForTesting(), 2u);
+
     original->Release();
+    EXPECT_EQ(original->GetRefCountForTesting(), 1u);
 
     Ref<RCTest> destination;
     destination = std::move(source);
+    EXPECT_EQ(original->GetRefCountForTesting(), 1u);
 
     EXPECT_EQ(source.Get(), nullptr);
     EXPECT_EQ(destination.Get(), original);
@@ -212,6 +241,31 @@ TEST(Ref, MoveAssignment) {
     EXPECT_TRUE(deleted);
 }
 
+// Test move assigment where the destination and source
+// point to the same underlying object.
+TEST(Ref, MoveAssignmentSameObject) {
+    bool deleted = false;
+    RCTest* original = new RCTest(&deleted);
+
+    Ref<RCTest> source(original);
+    EXPECT_EQ(original->GetRefCountForTesting(), 2u);
+
+    original->Release();
+    EXPECT_EQ(original->GetRefCountForTesting(), 1u);
+
+    Ref<RCTest>& referenceToSource = source;
+    EXPECT_EQ(original->GetRefCountForTesting(), 1u);
+
+    referenceToSource = std::move(source);
+
+    EXPECT_EQ(source.Get(), original);
+    EXPECT_EQ(original->GetRefCountForTesting(), 1u);
+    EXPECT_FALSE(deleted);
+
+    source = nullptr;
+    EXPECT_TRUE(deleted);
+}
+
 // Test the payload initial value is set correctly
 TEST(Ref, InitialPayloadValue) {
     RCTest* testDefaultConstructor = new RCTest();
@@ -255,4 +309,105 @@ TEST(Ref, Detach) {
 
     detached->Release();
     EXPECT_TRUE(deleted);
+}
+
+// Test constructor passed a derived pointer
+TEST(Ref, DerivedPointerConstructor) {
+    bool deleted = false;
+    {
+        Ref<RCTest> test(new RCTestDerived(&deleted));
+        test->Release();
+    }
+    EXPECT_TRUE(deleted);
+}
+
+// Test copy constructor of derived class
+TEST(Ref, DerivedCopyConstructor) {
+    bool deleted = false;
+    Ref<RCTestDerived> testDerived(new RCTestDerived(&deleted));
+    testDerived->Release();
+
+    {
+        Ref<RCTest> testBase(testDerived);
+        EXPECT_EQ(testBase->GetRefCountForTesting(), 2u);
+        EXPECT_EQ(testDerived->GetRefCountForTesting(), 2u);
+    }
+
+    EXPECT_EQ(testDerived->GetRefCountForTesting(), 1u);
+}
+
+// Test Ref constructed with nullptr
+TEST(Ref, ConstructedWithNullptr) {
+    Ref<RCTest> test(nullptr);
+    EXPECT_EQ(test.Get(), nullptr);
+}
+
+// Test Ref's copy assignment with derived class
+TEST(Ref, CopyAssignmentDerived) {
+    bool deleted = false;
+
+    RCTestDerived* original = new RCTestDerived(&deleted);
+    Ref<RCTestDerived> source(original);
+    original->Release();
+    EXPECT_EQ(original->GetRefCountForTesting(), 1u);
+
+    Ref<RCTest> destination;
+    destination = source;
+    EXPECT_EQ(original->GetRefCountForTesting(), 2u);
+
+    EXPECT_EQ(source.Get(), original);
+    EXPECT_EQ(destination.Get(), original);
+
+    source = nullptr;
+    EXPECT_EQ(original->GetRefCountForTesting(), 1u);
+    EXPECT_FALSE(deleted);
+
+    destination = nullptr;
+    EXPECT_TRUE(deleted);
+}
+
+// Test Ref's move constructor with derived class
+TEST(Ref, MoveConstructorDerived) {
+    bool deleted = false;
+    RCTestDerived* original = new RCTestDerived(&deleted);
+
+    Ref<RCTestDerived> source(original);
+    EXPECT_EQ(original->GetRefCountForTesting(), 2u);
+
+    Ref<RCTest> destination(std::move(source));
+    EXPECT_EQ(original->GetRefCountForTesting(), 2u);
+
+    original->Release();
+    EXPECT_EQ(original->GetRefCountForTesting(), 1u);
+
+    EXPECT_EQ(source.Get(), nullptr);
+    EXPECT_EQ(destination.Get(), original);
+    EXPECT_FALSE(deleted);
+
+    destination = nullptr;
+    EXPECT_TRUE(deleted);
+}
+
+// Test Ref's move assignment with derived class
+TEST(Ref, MoveAssignmentDerived) {
+    bool deleted = false;
+    RCTestDerived* original = new RCTestDerived(&deleted);
+
+    Ref<RCTestDerived> source(original);
+    EXPECT_EQ(original->GetRefCountForTesting(), 2u);
+
+    original->Release();
+    EXPECT_EQ(original->GetRefCountForTesting(), 1u);
+
+    Ref<RCTest> destination;
+    destination = std::move(source);
+
+    EXPECT_EQ(original->GetRefCountForTesting(), 1u);
+
+    EXPECT_EQ(source.Get(), nullptr);
+    EXPECT_EQ(destination.Get(), original);
+    EXPECT_FALSE(deleted);
+
+    destination = nullptr;
+    EXPECT_TRUE(deleted);
 }