Add copy constructors to the C++ Dawn interface

This removes the need for Clone() so it is removed and also adds tests
for the new constructors.

BUG=dawn:11

Change-Id: Ia45c765c2d30e40b0e036427793a62327b2008fc
Reviewed-on: https://dawn-review.googlesource.com/c/1901
Reviewed-by: Stephen White <senorblanco@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

examples/ComputeBoids.cpp

@@ -232,9 +232,9 @@ void initSim() {
     dawn::PipelineLayout pl = utils::MakeBasicPipelineLayout(device, &bgl);
 
     dawn::ComputePipelineDescriptor csDesc;
-    csDesc.module = module.Clone();
+    csDesc.module = module;
     csDesc.entryPoint = "main";
-    csDesc.layout = pl.Clone();
+    csDesc.layout = pl;
     updatePipeline = device.CreateComputePipeline(&csDesc);
 
     dawn::BufferView updateParamsView = updateParams.CreateBufferViewBuilder()

generator/templates/apicpp.h

@@ -70,19 +70,29 @@ namespace dawn {
                 if (mHandle) Derived::DawnRelease(mHandle);
             }
 
-            ObjectBase(ObjectBase const& other) = delete;
-            Derived& operator=(ObjectBase const& other) = delete;
+            ObjectBase(ObjectBase const& other)
+                : ObjectBase(other.Get()) {
+            }
+            Derived& operator=(ObjectBase const& other) {
+                if (&other != this) {
+                    if (mHandle) Derived::DawnRelease(mHandle);
+                    mHandle = other.mHandle;
+                    if (mHandle) Derived::DawnReference(mHandle);
+                }
+
+                return static_cast<Derived&>(*this);
+            }
 
             ObjectBase(ObjectBase&& other) {
                 mHandle = other.mHandle;
                 other.mHandle = 0;
             }
             Derived& operator=(ObjectBase&& other) {
-                if (&other == this) return static_cast<Derived&>(*this);
-
-                if (mHandle) Derived::DawnRelease(mHandle);
-                mHandle = other.mHandle;
-                other.mHandle = 0;
+                if (&other != this) {
+                    if (mHandle) Derived::DawnRelease(mHandle);
+                    mHandle = other.mHandle;
+                    other.mHandle = 0;
+                }
 
                 return static_cast<Derived&>(*this);
             }
@@ -98,9 +108,6 @@ namespace dawn {
                 mHandle = 0;
                 return result;
             }
-            Derived Clone() const {
-                return Derived(mHandle);
-            }
             static Derived Acquire(CType handle) {
                 Derived result;
                 result.mHandle = handle;

src/tests/DawnTest.cpp

@@ -258,15 +258,13 @@ std::ostringstream& DawnTest::AddBufferExpectation(const char* file,
                                                    uint32_t offset,
                                                    uint32_t size,
                                                    detail::Expectation* expectation) {
-    dawn::Buffer source = buffer.Clone();
-
     auto readback = ReserveReadback(size);
 
     // We need to enqueue the copy immediately because by the time we resolve the expectation,
     // the buffer might have been modified.
     dawn::CommandBuffer commands =
         device.CreateCommandBufferBuilder()
-            .CopyBufferToBuffer(source, offset, readback.buffer, readback.offset, size)
+            .CopyBufferToBuffer(buffer, offset, readback.buffer, readback.offset, size)
             .GetResult();
 
     queue.Submit(1, &commands);
@@ -296,7 +294,6 @@ std::ostringstream& DawnTest::AddTextureExpectation(const char* file,
                                                     uint32_t level,
                                                     uint32_t pixelSize,
                                                     detail::Expectation* expectation) {
-    dawn::Texture source = texture.Clone();
     uint32_t rowPitch = Align(width * pixelSize, kTextureRowPitchAlignment);
     uint32_t size = rowPitch * (height - 1) + width * pixelSize;
 
@@ -306,7 +303,7 @@ std::ostringstream& DawnTest::AddTextureExpectation(const char* file,
     // the texture might have been modified.
     dawn::CommandBuffer commands =
         device.CreateCommandBufferBuilder()
-            .CopyTextureToBuffer(source, x, y, 0, width, height, 1, level, 0, readback.buffer,
+            .CopyTextureToBuffer(texture, x, y, 0, width, height, 1, level, 0, readback.buffer,
                                  readback.offset, rowPitch)
             .GetResult();
 
@@ -359,7 +356,7 @@ DawnTest::ReadbackReservation DawnTest::ReserveReadback(uint32_t readbackSize) {
     slot.buffer = device.CreateBuffer(&descriptor);
 
     ReadbackReservation reservation;
-    reservation.buffer = slot.buffer.Clone();
+    reservation.buffer = slot.buffer;
     reservation.slot = mReadbackSlots.size();
     reservation.offset = 0;
 

src/tests/end2end/BindGroupTests.cpp

@@ -52,9 +52,9 @@ TEST_P(BindGroupTests, ReusedBindGroupSingleSubmit) {
     dawn::ShaderModule module =
         utils::CreateShaderModule(device, dawn::ShaderStage::Compute, shader);
     dawn::ComputePipelineDescriptor cpDesc;
-    cpDesc.module = module.Clone();
+    cpDesc.module = module;
     cpDesc.entryPoint = "main";
-    cpDesc.layout = pl.Clone();
+    cpDesc.layout = pl;
     dawn::ComputePipeline cp = device.CreateComputePipeline(&cpDesc);
 
     dawn::BufferDescriptor bufferDesc;

src/tests/end2end/ComputeCopyStorageBufferTests.cpp

@@ -39,9 +39,9 @@ void ComputeCopyStorageBufferTests::BasicTest(const char* shader) {
     auto pl = utils::MakeBasicPipelineLayout(device, &bgl);
 
     dawn::ComputePipelineDescriptor csDesc;
-    csDesc.module = module.Clone();
+    csDesc.module = module;
     csDesc.entryPoint = "main";
-    csDesc.layout = pl.Clone();
+    csDesc.layout = pl;
     dawn::ComputePipeline pipeline = device.CreateComputePipeline(&csDesc);
 
     // Set up src storage buffer

src/tests/end2end/PushConstantTests.cpp

@@ -146,9 +146,9 @@ class PushConstantTest: public DawnTest {
             );
 
             dawn::ComputePipelineDescriptor descriptor;
-            descriptor.module = module.Clone();
+            descriptor.module = module;
             descriptor.entryPoint = "main";
-            descriptor.layout = pl.Clone();
+            descriptor.layout = pl;
             return device.CreateComputePipeline(&descriptor);
         }
 

src/tests/unittests/ObjectBaseTests.cpp

@@ -22,7 +22,7 @@ class Object : public dawn::ObjectBase<Object, int*> {
         using ObjectBase::operator=;
 
         static void DawnReference(int* handle) {
-            ASSERT_LT(0, *handle);
+            ASSERT_LE(0, *handle);
             *handle += 1;
         }
         static void DawnRelease(int* handle) {
@@ -52,16 +52,14 @@ TEST(ObjectBase, AcquireConstruction) {
     ASSERT_EQ(0, refcount);
 }
 
-// Test that cloning takes a new ref. Also test .Get().
-TEST(ObjectBase, Clone) {
+// Test .Get().
+TEST(ObjectBase, Get) {
     int refcount = 1;
     {
         Object obj1(&refcount);
-        Object obj2 = obj1.Clone();
 
-        ASSERT_EQ(3, refcount);
+        ASSERT_EQ(2, refcount);
         ASSERT_EQ(&refcount, obj1.Get());
-        ASSERT_EQ(&refcount, obj2.Get());
     }
     ASSERT_EQ(1, refcount);
 }
@@ -91,6 +89,51 @@ TEST(ObjectBase, OperatorBool) {
     }
 }
 
+// Test the copy constructor of C++ objects
+TEST(ObjectBase, CopyConstructor) {
+    int refcount = 1;
+
+    Object source(&refcount);
+    Object destination(source);
+
+    ASSERT_EQ(source.Get(), &refcount);
+    ASSERT_EQ(destination.Get(), &refcount);
+    ASSERT_EQ(3, refcount);
+
+    destination = Object();
+    ASSERT_EQ(refcount, 2);
+}
+
+// Test the copy assignment of C++ objects
+TEST(ObjectBase, CopyAssignment) {
+    int refcount = 1;
+    Object source(&refcount);
+
+    Object destination;
+    destination = source;
+
+    ASSERT_EQ(source.Get(), &refcount);
+    ASSERT_EQ(destination.Get(), &refcount);
+    ASSERT_EQ(3, refcount);
+
+    destination = Object();
+    ASSERT_EQ(refcount, 2);
+}
+
+// Test the copy assignment of C++ objects onto themselves
+TEST(ObjectBase, CopyAssignmentSelf) {
+    int refcount = 1;
+
+    Object obj(&refcount);
+
+    // Fool the compiler to avoid a -Wself-assign-overload
+    Object* objPtr = &obj;
+    obj = *objPtr;
+
+    ASSERT_EQ(obj.Get(), &refcount);
+    ASSERT_EQ(refcount, 2);
+}
+
 // Test the move constructor of C++ objects
 TEST(ObjectBase, MoveConstructor) {
     int refcount = 1;
@@ -121,3 +164,16 @@ TEST(ObjectBase, MoveAssignment) {
     ASSERT_EQ(refcount, 1);
 }
 
+// Test the move assignment of C++ objects onto themselves
+TEST(ObjectBase, MoveAssignmentSelf) {
+    int refcount = 1;
+
+    Object obj(&refcount);
+
+    // Fool the compiler to avoid a -Wself-move
+    Object* objPtr = &obj;
+    obj = std::move(*objPtr);
+
+    ASSERT_EQ(obj.Get(), &refcount);
+    ASSERT_EQ(refcount, 2);
+}