Merge pull request #66092 from gottesmm/pr-6e8d5b3eecd4d51d09bf0be496ecf5a5db426ec3

[move-only] Ban destructuring of noncopyable address only types like we do for loadable types.

Author: gottesmm

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

@@ -1867,6 +1867,18 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
     if (!leafRange)
       return false;
 
+    // Now check if we have a destructure through deinit. If we do, emit an
+    // error.
+    unsigned numDiagnostics =
+        moveChecker.diagnosticEmitter.getDiagnosticCount();
+    checkForDestructureThroughDeinit(markedValue, op, *leafRange,
+                                     diagnosticEmitter);
+    if (numDiagnostics != moveChecker.diagnosticEmitter.getDiagnosticCount()) {
+      LLVM_DEBUG(llvm::dbgs()
+                 << "Emitting destructure through deinit error!\n");
+      return true;
+    }
+
     LLVM_DEBUG(llvm::dbgs() << "Pure consuming use: " << *user);
     useState.takeInsts.insert({user, *leafRange});
     return true;

test/SILOptimizer/moveonly_addresschecker_destructure_through_deinit_diagnostics.swift

@@ -14,44 +14,50 @@ class MoveOnlyKlass {
   var value: Int = 0
 }
 
-@_moveOnly
-struct KlassPair {
+struct KlassPair : ~Copyable {
   var lhs: Klass
   var rhs: MoveOnlyKlass
 }
 
-@_moveOnly
-struct AggStruct {
+struct AggStruct : ~Copyable {
   var pair: KlassPair
 }
 
-@_moveOnly
-struct KlassPair2 {
+struct KlassPair2 : ~Copyable {
   var lhs: MoveOnlyKlass
   var rhs: MoveOnlyKlass
 }
 
-@_moveOnly
-struct AggStruct2 {
+struct AggStruct2 : ~Copyable {
   var lhs: MoveOnlyKlass
   var pair: KlassPair2
   var rhs: MoveOnlyKlass
 }
 
-@_moveOnly
-struct SingleIntContainingStruct {
+struct SingleIntContainingStruct : ~Copyable {
     var value: Int = 0
 }
 
+struct MoveOnlyPair : ~Copyable {
+  var first = SingleIntContainingStruct()
+  var second = SingleIntContainingStruct()
+}
+
+protocol P {
+  static var value: Self { get }
+}
+
+func consume<T : P>(_ x: consuming T) {}
+func consume(_ x: consuming SingleIntContainingStruct) {}
 func consume(_ x: consuming MoveOnlyKlass) {}
+func consume(_ x: consuming MoveOnlyPair) {}
 func consume(_ x: consuming Klass) {}
 
 ////////////////////
-// Test Top Level //
+// MARK: Loadable //
 ////////////////////
 
-@_moveOnly
-struct DeinitStruct {
+struct DeinitStruct : ~Copyable {
   var first: Klass
   var second: (Klass, Klass)
   var third: KlassPair
@@ -98,12 +104,12 @@ func testConsumeNonCopyable4(_ x: consuming DeinitStruct) {
     consume(x.fifth) // expected-note {{consuming use here}}
 }
 
-/////////////////
-// Test Fields //
-/////////////////
 
-@_moveOnly
-struct StructContainDeinitStruct {
+///////////////////////////
+// MARK: Loadable Fields //
+///////////////////////////
+
+struct StructContainDeinitStruct : ~Copyable {
   var first: DeinitStruct
   var second: (DeinitStruct, DeinitStruct)
   var third: Klass
@@ -153,3 +159,85 @@ func testStructContainStructContainDeinitStructConsumeNonCopyable4(_ x: consumin
     // expected-error @-1 {{Cannot partially consume 'x' since it contains field 'x.first' whose type 'DeinitStruct' has a user defined deinit}}
     consume(x.first.fifth) // expected-note {{consuming use here}}
 }
+
+////////////////////////
+// MARK: Address Only //
+////////////////////////
+
+struct AddressOnlyDeinitStruct<T : P> : ~Copyable {
+  var copyable: T = T.value
+  var moveOnly = SingleIntContainingStruct()
+  var moveOnlyPair = MoveOnlyPair()
+
+  deinit {}
+  // expected-note @-1 {{deinit declared here}}
+  // expected-note @-2 {{deinit declared here}}
+  // expected-note @-3 {{deinit declared here}}
+  // expected-note @-4 {{deinit declared here}}
+  // expected-note @-5 {{deinit declared here}}
+}
+
+func consume<T : P>(_ x: consuming AddressOnlyDeinitStruct<T>) {}
+
+func testAddressOnlyCanConsumeEntireType<T : P>(_ x: consuming AddressOnlyDeinitStruct<T>) {
+  // This is ok since we are consuming a copyable value.
+  consume(x.copyable)
+  // This is ok since we are consuming the entire value.
+  consume(x)
+}
+
+func testAddressOnlyCannotPartialConsume<T : P>(_ x: consuming AddressOnlyDeinitStruct<T>) {
+  // expected-error @-1 {{Cannot partially consume 'x' since it has a user defined deinit}}
+  // expected-error @-2 {{Cannot partially consume 'x' since it has a user defined deinit}}
+  consume(x.moveOnly) // expected-note {{consuming use here}}
+  consume(x.moveOnlyPair.first) // expected-note {{consuming use here}}
+  consume(x.copyable)
+}
+
+struct AddressOnlyContainingDeinitStruct<T : P> : ~Copyable {
+  var a = AddressOnlyDeinitStruct<T>()
+}
+
+func testAddressOnlyCannotPartialConsumeEvenIfSingleElt<T : P>(_ x: consuming AddressOnlyContainingDeinitStruct<T>) {
+  // expected-error @-1 {{Cannot partially consume 'x' since it contains field 'x.a' whose type 'AddressOnlyDeinitStruct' has a user defined deinit}}
+
+  // We do not error here since we can partially consume x, but not x.a
+  consume(x.a)
+  consume(x.a.moveOnlyPair) // expected-note {{consuming use here}}
+}
+
+struct AddressOnlyContainingDeinitSingleField<T : P> : ~Copyable {
+  var moveOnly = SingleIntContainingStruct()
+  deinit {}
+  // expected-note @-1 {{deinit declared here}}
+}
+
+struct AddressOnlyContainingDeinitStruct3<T : P> : ~Copyable {
+  var a = AddressOnlyContainingDeinitSingleField<T>()
+}
+
+func consume<T : P>(_ x: consuming AddressOnlyContainingDeinitSingleField<T>) {}
+
+func testAddressOnlyCannotPartialConsumeEvenIfSingleElt<T : P>(_ x: consuming AddressOnlyContainingDeinitStruct3<T>) {
+  // expected-error @-1 {{Cannot partially consume 'x' since it contains field 'x.a' whose type 'AddressOnlyContainingDeinitSingleField' has a user defined deinit}}
+
+  // We do not error here since we can partially consume x, but not x.a
+  consume(x.a)
+  consume(x.a.moveOnly) // expected-note {{consuming use here}}
+}
+
+
+struct AddressOnlyContainingDeinitStructPair<T : P> : ~Copyable {
+  var first = AddressOnlyDeinitStruct<T>()
+  var second = AddressOnlyDeinitStruct<T>()
+}
+
+// Make sure that if the deinit is in an intermediate element of the path that
+// we still handle it appropriately.
+func testAddressOnlyDeinitInMiddlePath<T : P>(_ x: consuming AddressOnlyContainingDeinitStructPair<T>) {
+  // expected-error @-1 {{Cannot partially consume 'x' since it contains field 'x.first' whose type 'AddressOnlyDeinitStruct' has a user defined deinit}}
+  // expected-error @-2 {{Cannot partially consume 'x' since it contains field 'x.first' whose type 'AddressOnlyDeinitStruct' has a user defined deinit}}
+  consume(x.first.moveOnly) // expected-note {{consuming use here}}
+  consume(x.first.moveOnlyPair.first) // expected-note {{consuming use here}}
+  consume(x.first.copyable)
+}