Merge pull request #64397 from hyp/eng/destroy-before-assigncopytake

hyp · web-flow · commit de56accb96da · 2023-03-17T16:14:06.000-07:00
[interop] C++ destination record should be destroyed before being cop…
diff --git a/lib/IRGen/GenStruct.cpp b/lib/IRGen/GenStruct.cpp
@@ -772,6 +772,7 @@ namespace {
     void assignWithCopy(IRGenFunction &IGF, Address destAddr, Address srcAddr,
                         SILType T, bool isOutlined) const override {
       if (auto copyConstructor = findCopyConstructor()) {
+        destroy(IGF, destAddr, T, isOutlined);
         emitCopyWithCopyConstructor(IGF, T, copyConstructor,
                                     srcAddr.getAddress(),
                                     destAddr.getAddress());
@@ -800,6 +801,7 @@ namespace {
     void assignWithTake(IRGenFunction &IGF, Address dest, Address src, SILType T,
                         bool isOutlined) const override {
       if (auto copyConstructor = findCopyConstructor()) {
+        destroy(IGF, dest, T, isOutlined);
         emitCopyWithCopyConstructor(IGF, T, copyConstructor,
                                     src.getAddress(),
                                     dest.getAddress());
diff --git a/test/Interop/Cxx/class/Inputs/copy-move-assignment.h b/test/Interop/Cxx/class/Inputs/copy-move-assignment.h
@@ -0,0 +1,100 @@
+#pragma once
+
+struct InstanceBalanceCounter {
+    static inline int &theCounterValue() {
+        static int counterValue = 0;
+        return counterValue;
+    }
+    static inline int getCounterValue() {
+        return theCounterValue();
+    }
+    
+    __attribute__((optnone))
+    InstanceBalanceCounter() {
+        ++theCounterValue();
+    }
+    __attribute__((optnone))
+    InstanceBalanceCounter(const InstanceBalanceCounter &) {
+        ++theCounterValue();
+    }
+    __attribute__((optnone))
+    InstanceBalanceCounter(InstanceBalanceCounter &&) {
+        ++theCounterValue();
+    }
+    __attribute__((optnone))
+    ~InstanceBalanceCounter() {
+        --theCounterValue();
+    }
+};
+
+__attribute__((optnone))
+inline void someFunc() {}
+
+struct NonTrivialCopyAssign {
+    __attribute__((optnone))
+    NonTrivialCopyAssign(): copyAssignCounter(0) {}
+    __attribute__((optnone))
+    ~NonTrivialCopyAssign() {
+        someFunc();
+    }
+
+    __attribute__((optnone))
+    NonTrivialCopyAssign &operator =(const NonTrivialCopyAssign &) {
+        ++copyAssignCounter;
+        return *this;
+    }
+
+    int copyAssignCounter;
+    InstanceBalanceCounter instanceBalancer;
+};
+
+struct NonTrivialMoveAssign {
+    __attribute__((optnone))
+    NonTrivialMoveAssign(): moveAssignCounter(0) {}
+    __attribute__((optnone))
+    NonTrivialMoveAssign(const NonTrivialMoveAssign &) = default;
+    __attribute__((optnone))
+    ~NonTrivialMoveAssign() {
+        someFunc();
+    }
+
+    __attribute__((optnone))
+    NonTrivialMoveAssign &operator =(NonTrivialMoveAssign &&) {
+        ++moveAssignCounter;
+        return *this;
+    }
+
+    int moveAssignCounter;
+    InstanceBalanceCounter instanceBalancer;
+};
+
+struct NonTrivialCopyAndCopyMoveAssign {
+    __attribute__((optnone))
+    NonTrivialCopyAndCopyMoveAssign(): assignCounter(0) {}
+    __attribute__((optnone))
+    NonTrivialCopyAndCopyMoveAssign(const NonTrivialCopyAndCopyMoveAssign &other) : assignCounter(other.assignCounter), instanceBalancer(other.instanceBalancer) {
+        someFunc();
+    }
+    __attribute__((optnone))
+    NonTrivialCopyAndCopyMoveAssign( NonTrivialCopyAndCopyMoveAssign &&other) : assignCounter(other.assignCounter), instanceBalancer(other.instanceBalancer) {
+        someFunc();
+    }
+    __attribute__((optnone))
+    ~NonTrivialCopyAndCopyMoveAssign() {
+        someFunc();
+    }
+
+    __attribute__((optnone))
+    NonTrivialCopyAndCopyMoveAssign &operator =(const NonTrivialCopyAndCopyMoveAssign &) {
+        ++assignCounter;
+        return *this;
+    }
+    __attribute__((optnone))
+    NonTrivialCopyAndCopyMoveAssign &operator =(NonTrivialCopyAndCopyMoveAssign &&) {
+        ++assignCounter;
+        return *this;
+    }
+
+    int assignCounter;
+    InstanceBalanceCounter instanceBalancer;
+};
diff --git a/test/Interop/Cxx/class/Inputs/module.modulemap b/test/Interop/Cxx/class/Inputs/module.modulemap
@@ -135,4 +135,9 @@ module ForwardDeclaredInNamespace {
 module ConformsTo {
   header "conforms-to.h"
   requires cplusplus
-}
+}
+
+module CopyMoveAssignment {
+  header "copy-move-assignment.h"
+  requires cplusplus
+}
diff --git a/test/Interop/Cxx/class/copy-move-assignment-executable.swift b/test/Interop/Cxx/class/copy-move-assignment-executable.swift
@@ -0,0 +1,58 @@
+// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -enable-experimental-cxx-interop)
+//
+// Re-test with optimizations:
+// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -enable-experimental-cxx-interop -O)
+//
+// REQUIRES: executable_test
+
+import StdlibUnittest
+import CopyMoveAssignment
+
+var CxxCopyMoveAssignTestSuite = TestSuite("CxxCopyMoveAssignTestSuite")
+
+@inline(never)
+func takeValue<T>(_ x: T) {
+    let _ = x
+}
+
+CxxCopyMoveAssignTestSuite.test("NonTrivialCopyAssign") {
+    do {
+        var instance = NonTrivialCopyAssign()
+        expectEqual(0, instance.copyAssignCounter)
+        let instance2 = NonTrivialCopyAssign()
+        instance = instance2
+        // `operator=` isn't called.
+        expectEqual(0, instance.copyAssignCounter)
+        takeValue(instance2)
+    }
+    // The number of construcors and destructors called for `NonTrivialCopyAssign` must be balanced.
+    expectEqual(0, InstanceBalanceCounter.getCounterValue())
+}
+
+CxxCopyMoveAssignTestSuite.test("NonTrivialMoveAssign") {
+    do {
+        var instance = NonTrivialMoveAssign()
+        expectEqual(0, instance.moveAssignCounter)
+        instance = NonTrivialMoveAssign()
+        // `operator=` isn't called.
+        expectEqual(0, instance.moveAssignCounter)
+    }
+    // The number of construcors and destructors called for `NonTrivialCopyAssign` must be balanced.
+    expectEqual(0, InstanceBalanceCounter.getCounterValue())
+}
+
+CxxCopyMoveAssignTestSuite.test("NonTrivialCopyAndCopyMoveAssign") {
+    do {
+        var instance = NonTrivialCopyAndCopyMoveAssign()
+        expectEqual(0, instance.assignCounter)
+        let instance2 = NonTrivialCopyAndCopyMoveAssign()
+        instance = instance2
+        // `operator=` isn't called.
+        expectEqual(0, instance.assignCounter)
+        takeValue(instance2)
+    }
+    // The number of construcors and destructors called for `NonTrivialCopyAndCopyMoveAssign` must be balanced.
+    expectEqual(0, InstanceBalanceCounter.getCounterValue())
+}
+
+runAllTests()
diff --git a/test/Interop/Cxx/class/copy-move-assignment-irgen.swift b/test/Interop/Cxx/class/copy-move-assignment-irgen.swift
@@ -0,0 +1,42 @@
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -I %S/Inputs -enable-experimental-cxx-interop -emit-ir %s -Xcc -fignore-exceptions -O | %FileCheck %s
+
+import CopyMoveAssignment
+
+@inline(never)
+func takeValue<T>(_ x: T) {
+    let _ = x
+}
+
+@inline(never)
+public func copyAssign() {
+    var instance = NonTrivialCopyAndCopyMoveAssign()
+    let instance2 = NonTrivialCopyAndCopyMoveAssign()
+    instance = instance2
+    takeValue(instance2)
+    takeValue(instance)
+}
+
+// CHECK-LABEL: define {{.*}}copyAssign
+// CHECK: call {{void|\%struct.NonTrivialCopyAndCopyMoveAssign\*}} @{{_ZN31NonTrivialCopyAndCopyMoveAssignC1Ev|_ZN31NonTrivialCopyAndCopyMoveAssignC2Ev|"\?\?0NonTrivialCopyAndCopyMoveAssign@@QEAA@XZ"}}(%struct.NonTrivialCopyAndCopyMoveAssign* %[[COPY_INSTANCE:.*]])
+// CHECK: call {{void|\%struct.NonTrivialCopyAndCopyMoveAssign\*}} @{{_ZN31NonTrivialCopyAndCopyMoveAssignC1Ev|_ZN31NonTrivialCopyAndCopyMoveAssignC2Ev|"\?\?0NonTrivialCopyAndCopyMoveAssign@@QEAA@XZ"}}(%struct.NonTrivialCopyAndCopyMoveAssign* %[[COPY_INSTANCE2:.*]])
+// CHECK: call {{void|\%struct.NonTrivialCopyAndCopyMoveAssign\*}} @{{_ZN31NonTrivialCopyAndCopyMoveAssignD1Ev|_ZN31NonTrivialCopyAndCopyMoveAssignD2Ev|"\?\?1NonTrivialCopyAndCopyMoveAssign@@QEAA@XZ"}}(%struct.NonTrivialCopyAndCopyMoveAssign* %[[COPY_INSTANCE]])
+// CHECK: call {{void|\%struct.NonTrivialCopyAndCopyMoveAssign\*}} @{{_ZN31NonTrivialCopyAndCopyMoveAssignC1ERKS_|_ZN31NonTrivialCopyAndCopyMoveAssignC2ERKS_|"\?\?0NonTrivialCopyAndCopyMoveAssign@@QEAA@AEBU0@@Z"}}(%struct.NonTrivialCopyAndCopyMoveAssign* %[[COPY_INSTANCE]], %struct.NonTrivialCopyAndCopyMoveAssign* %[[COPY_INSTANCE2]])
+// CHECK: call {{.*}} @{{_ZN31NonTrivialCopyAndCopyMoveAssignD1Ev|_ZN31NonTrivialCopyAndCopyMoveAssignD2Ev|"\?\?1NonTrivialCopyAndCopyMoveAssign@@QEAA@XZ"}}(%struct.NonTrivialCopyAndCopyMoveAssign* %[[COPY_INSTANCE2]])
+// CHECK: call {{.*}} @{{_ZN31NonTrivialCopyAndCopyMoveAssignD1Ev|_ZN31NonTrivialCopyAndCopyMoveAssignD2Ev|"\?\?1NonTrivialCopyAndCopyMoveAssign@@QEAA@XZ"}}(%struct.NonTrivialCopyAndCopyMoveAssign* %[[COPY_INSTANCE]])
+
+// CHECK-LABEL: define {{.*}}takeAssign
+// CHECK: call {{void|\%struct.NonTrivialCopyAndCopyMoveAssign\*}} @{{_ZN31NonTrivialCopyAndCopyMoveAssignC1Ev|_ZN31NonTrivialCopyAndCopyMoveAssignC2Ev|"\?\?0NonTrivialCopyAndCopyMoveAssign@@QEAA@XZ"}}(%struct.NonTrivialCopyAndCopyMoveAssign* %[[MOVE_INSTANCE:.*]])
+// CHECK: call {{.*}} @{{_ZN31NonTrivialCopyAndCopyMoveAssignD1Ev|_ZN31NonTrivialCopyAndCopyMoveAssignD2Ev|"\?\?1NonTrivialCopyAndCopyMoveAssign@@QEAA@XZ"}}(%struct.NonTrivialCopyAndCopyMoveAssign* %[[MOVE_INSTANCE]])
+// CHECK: call {{.*}} @{{_ZN31NonTrivialCopyAndCopyMoveAssignD1Ev|_ZN31NonTrivialCopyAndCopyMoveAssignD2Ev|"\?\?1NonTrivialCopyAndCopyMoveAssign@@QEAA@XZ"}}(%struct.NonTrivialCopyAndCopyMoveAssign* %[[MOVE_INSTANCE]])
+
+// CHECK-LABEL: }
+
+@inline(never)
+public func takeAssign() {
+    var instance = NonTrivialCopyAndCopyMoveAssign()
+    instance = NonTrivialCopyAndCopyMoveAssign()
+    takeValue(instance)
+}
+
+copyAssign()
+takeAssign()
diff --git a/test/Interop/Cxx/value-witness-table/witness-lifetime-operations-irgen.swift b/test/Interop/Cxx/value-witness-table/witness-lifetime-operations-irgen.swift
@@ -28,6 +28,7 @@ let h = Holder(holding: NonTrivial())
 // $sSo10NonTrivialVwca ---> assignWithCopy value witness for __C.NonTrivia
 // CHECK-LABEL: define linkonce_odr hidden %swift.opaque* @"$sSo10NonTrivialVwca"
 // CHECK-NOT: call
+// CHECK: call void @{{_ZN10NonTrivialD(1|2)Ev|"\?\?1NonTrivial@@QEAA@XZ"}}(%struct.NonTrivial* %{{[0-9]+}})
 // CHECK: call {{void|\%struct\.NonTrivial\*}} @{{_ZN10NonTrivialC(1|2)ERKS_|"\?\?0NonTrivial@@QEAA@AEBU0@@Z"}}(%struct.NonTrivial* %{{[0-9]+}}, %struct.NonTrivial* %{{[0-9]+}})
 // CHECK-NOT: call
 // CHECK: ret %swift.opaque*
@@ -44,8 +45,9 @@ let h = Holder(holding: NonTrivial())
 // $sSo10NonTrivialVwta ---> assignWithTake value witness for __C.NonTrivial
 // CHECK-LABEL: define linkonce_odr hidden %swift.opaque* @"$sSo10NonTrivialVwta"
 // CHECK-NOT: call
+// CHECK: call void @{{_ZN10NonTrivialD(1|2)Ev|"\?\?1NonTrivial@@QEAA@XZ"}}(%struct.NonTrivial* %{{[0-9]+}})
 // CHECK: call {{void|\%struct\.NonTrivial\*}} @{{_ZN10NonTrivialC(1|2)ERKS_|"\?\?0NonTrivial@@QEAA@AEBU0@@Z"}}(%struct.NonTrivial* %{{[0-9]+}}, %struct.NonTrivial* %{{[0-9]+}})
 // CHECK-NOT: call
 // CHECK: call void @{{_ZN10NonTrivialD(1|2)Ev|"\?\?1NonTrivial@@QEAA@XZ"}}(%struct.NonTrivial* %{{[0-9]+}})
 // CHECK-NOT: call
-// CHECK: ret %swift.opaque*
+// CHECK: ret %swift.opaque*
