DI: Place cleanups for partially-initialized non-Copyable values on the mark_unresolved_noncopyable marker.

This ensures that move checking sees the cleanups when determining a value's lifetime and doesn't
try to insert its own cleanup leading to a double destroy. Fixes #85063 | rdar://163194098.

Author: jckarter

lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp

@@ -39,17 +39,22 @@ static bool isVariableOrResult(MarkUninitializedInst *MUI) {
 static void gatherDestroysOfContainer(const DIMemoryObjectInfo &memoryInfo,
                                       DIElementUseInfo &useInfo) {
   auto *uninitMemory = memoryInfo.getUninitializedValue();
+  auto origUninitMemory = uninitMemory;
+
+  if (auto mui = dyn_cast<MarkUnresolvedNonCopyableValueInst>(uninitMemory)) {
+    origUninitMemory = cast<SingleValueInstruction>(mui->getOperand());
+  }
 
   // The uninitMemory must be used on an alloc_box, alloc_stack, or global_addr.
   // If we have an alloc_stack or a global_addr, there is nothing further to do.
-  if (isa<AllocStackInst>(uninitMemory->getOperand(0)) ||
-      isa<GlobalAddrInst>(uninitMemory->getOperand(0)) ||
-      isa<SILArgument>(uninitMemory->getOperand(0)) ||
+  if (isa<AllocStackInst>(origUninitMemory->getOperand(0)) ||
+      isa<GlobalAddrInst>(origUninitMemory->getOperand(0)) ||
+      isa<SILArgument>(origUninitMemory->getOperand(0)) ||
       // FIXME: We only support pointer to address here to not break LLDB. It is
       // important that long term we get rid of this since this is a situation
       // where LLDB is breaking SILGen/DI invariants by not creating a new
       // independent stack location for the pointer to address.
-      isa<PointerToAddressInst>(uninitMemory->getOperand(0))) {
+      isa<PointerToAddressInst>(origUninitMemory->getOperand(0))) {
     return;
   }
 
@@ -58,8 +63,8 @@ static void gatherDestroysOfContainer(const DIMemoryObjectInfo &memoryInfo,
   //
   // TODO: This should really be tracked separately from other destroys so that
   // we distinguish the lifetime of the container from the value itself.
-  assert(isa<ProjectBoxInst>(uninitMemory));
-  auto value = uninitMemory->getOperand(0);
+  assert(isa<ProjectBoxInst>(origUninitMemory));
+  auto value = origUninitMemory->getOperand(0);
   if (auto *bbi = dyn_cast<BeginBorrowInst>(value)) {
     value = bbi->getOperand();
   }
@@ -181,17 +186,23 @@ SILInstruction *DIMemoryObjectInfo::getFunctionEntryPoint() const {
 
 static SingleValueInstruction *
 getUninitializedValue(MarkUninitializedInst *MemoryInst) {
-  SILValue inst = MemoryInst;
-  if (auto *bbi = MemoryInst->getSingleUserOfType<BeginBorrowInst>()) {
-    inst = bbi;
+  SingleValueInstruction *inst = MemoryInst;
+  SILValue projectBoxUser = inst;
+  
+  // Check for a project_box instruction (possibly via a borrow).
+  if (auto *bbi = projectBoxUser->getSingleUserOfType<BeginBorrowInst>()) {
+    projectBoxUser = bbi;
+  }
+  if (auto pbi = projectBoxUser->getSingleUserOfType<ProjectBoxInst>()) {
+    inst = pbi;
   }
 
-  if (SingleValueInstruction *svi =
-          inst->getSingleUserOfType<ProjectBoxInst>()) {
-    return svi;
+  // Access move-only values through their marker.
+  if (auto mui = inst->getSingleUserOfType<MarkUnresolvedNonCopyableValueInst>()) {
+    inst = mui;
   }
 
-  return MemoryInst;
+  return inst;
 }
 
 SingleValueInstruction *DIMemoryObjectInfo::getUninitializedValue() const {

test/Interpreter/moveonly_throw_partial_init.swift

@@ -0,0 +1,40 @@
+// RUN: %target-run-simple-swift
+// REQUIRES: executable_test
+
+struct Butt: Error {}
+
+struct Inside: ~Copyable {
+	init() throws { throw Butt() }
+}
+
+class C {
+	deinit { print("destroying") }
+}
+
+struct Outside: ~Copyable {
+    let instance: Inside
+	var c: C
+
+    init() throws {
+		c = C()
+        let instance = try Inside()
+        self.instance = instance
+    }
+}
+
+// CHECK: begin
+// CHECK-NEXT: destroying
+// CHECK-NEXT: caught
+// CHECK-NEXT: end
+
+func test() {
+	print("begin")
+	do {
+		_ = try Outside()
+	} catch {
+		print("caught")
+	}
+	print("end")
+}
+test()
+

test/SILOptimizer/definite_init_moveonly.sil

@@ -0,0 +1,40 @@
+// RUN: %target-sil-opt -module-name definite_init_moveonly -enable-sil-verify-all -definite-init %s | %FileCheck %s
+
+import Swift
+
+class C {
+	init()
+}
+
+struct S: ~Copyable {
+	var c1: C
+	var c2: C
+}
+
+// Ensure that, when DI introduces cleanups for partially initialized move-
+// only values, the cleanups are applied under the `mark_unresolved`
+// instruction, so that the move checker properly accounts for the cleanups.
+// #85063 | rdar://163194098
+
+// CHECK-LABEL: sil [ossa] @$test
+sil [ossa] @$test : $@convention(thin) (@owned C) -> () {
+entry(%c : @owned $C):
+	%s = alloc_stack [lexical] [var_decl] $S, var, name "butts", type $S
+	%s_marked = mark_uninitialized [rootself] %s
+	// CHECK: [[S_MOVEONLY:%.*]] = mark_unresolved_non_copyable_value
+	%s_moveonly = mark_unresolved_non_copyable_value [consumable_and_assignable] %s_marked
+	%s_access = begin_access [modify] [static] %s_moveonly
+	%c1 = struct_element_addr %s_access, #S.c1
+	assign %c to %c1
+	end_access %s_access
+	// CHECK: br [[NEXT:bb[0-9]+]]
+	br next
+
+	// CHECK: [[NEXT]]:
+	// CHECK: [[FIELD_TO_DESTROY:%.*]] = struct_element_addr [[S_MOVEONLY]], #S.c1
+	// CHECK: destroy_addr [[FIELD_TO_DESTROY]]
+next:
+	destroy_addr %s_moveonly
+	dealloc_stack %s
+	return undef : $()
+}

test/SILOptimizer/moveonly_partial_consumption_inlinable.swift

@@ -0,0 +1,73 @@
+// RUN: %target-swift-frontend -enable-library-evolution -emit-sil -verify %s
+
+public struct Consumable: ~Copyable {}
+
+public func consume(_: consuming Consumable) {}
+
+public struct NotFrozen: ~Copyable {
+	public var field: Consumable
+
+	public consuming func notInlinable() {
+		consume(field)
+	}
+
+	@inlinable
+	public consuming func inlinable() {
+		// expected-error @+1 {{consumed but not reinitialized}}
+		consume(field) // expected-note{{}}
+	}
+
+	@_alwaysEmitIntoClient
+	public consuming func aeic() {
+		// expected-error @+1 {{consumed but not reinitialized}}
+		consume(field) // expected-note{{}}
+	}
+
+	@_transparent
+	public consuming func transparent() {
+		// expected-error @+1 {{consumed but not reinitialized}}
+		consume(field) // expected-note{{}}
+	}
+}
+
+@frozen
+public struct Frozen: ~Copyable {
+	public var field: Consumable
+
+	public consuming func notInlinable() {
+		consume(field)
+	}
+
+	@inlinable
+	public consuming func inlinable() {
+		consume(field)
+	}
+}
+
+@usableFromInline
+internal struct NotFrozenUFI: ~Copyable {
+	public var field: Consumable
+
+	public consuming func notInlinable() {
+		consume(field)
+	}
+
+	@inlinable
+	public consuming func inlinable() {
+		// expected-error @+1 {{consumed but not reinitialized}}
+		consume(field) // expected-note{{}}
+	}
+
+	@_alwaysEmitIntoClient
+	public consuming func aeic() {
+		// expected-error @+1 {{consumed but not reinitialized}}
+		consume(field) // expected-note{{}}
+	}
+
+	@_transparent
+	public consuming func transparent() {
+		// expected-error @+1 {{consumed but not reinitialized}}
+		consume(field) // expected-note{{}}
+	}
+}
+