Merge pull request #79149 from jckarter/borrow-to-destructure-interior-pointer-order-of-operations

BorrowToDestructureTransform: Fix order of operations when placing borrows around InteriorPointerOperands.

Author: jckarter

include/swift/SIL/OwnershipUtils.h

@@ -893,9 +893,6 @@ class InteriorPointerOperandKind {
 /// object with guaranteed ownership. All transitive address uses of the
 /// interior pointer must be within the lifetime of the guaranteed lifetime. As
 /// such, these must be treated as implicit uses of the parent guaranteed value.
-///
-/// FIXME: This should probably also handle project_box once we support
-/// borrowing a box.
 struct InteriorPointerOperand {
   Operand *operand;
   InteriorPointerOperandKind kind;

lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureUtils.cpp

@@ -1190,7 +1190,7 @@ void Implementation::rewriteUses(InstructionDeleter *deleter) {
     LLVM_DEBUG(llvm::dbgs() << "    Computed available values for block bb"
                             << block->getDebugID() << '\n';
                availableValues.print(llvm::dbgs(), "        "));
-    // Then walk from the top to the bottom of the block rewriting as we go.
+    // Then walk from the beginning to the end of the block, rewriting as we go.
     for (auto ii = block->begin(), ie = block->end(); ii != ie;) {
       auto *inst = &*ii;
       ++ii;
@@ -1336,13 +1336,6 @@ void Implementation::rewriteUses(InstructionDeleter *deleter) {
                 borrowBuilder.createGuaranteedMoveOnlyWrapperToCopyableValue(
                     loc, value);
           }
-          // NOTE: oldInst may be nullptr if our operand is a SILArgument
-          // which can happen with switch_enum.
-          auto *oldInst = operand.get()->getDefiningInstruction();
-          operand.set(value);
-          if (oldInst && deleter)
-            deleter->forceTrackAsDead(oldInst);
-
           // If we have a terminator that is a trivial use (e.x.: we
           // struct_extract a trivial value). Just put the end_borrow before the
           // terminator.
@@ -1351,19 +1344,31 @@ void Implementation::rewriteUses(InstructionDeleter *deleter) {
                 operand.getOperandOwnership() == OperandOwnership::TrivialUse) {
               SILBuilderWithScope endBuilder(inst);
               endBuilder.createEndBorrow(getSafeLoc(inst), borrow);
-              continue;
+              goto update_operand;
             } else {
               // Otherwise, put the end_borrow.
               for (auto *succBlock : ti->getSuccessorBlocks()) {
                 auto *nextInst = &succBlock->front();
                 SILBuilderWithScope endBuilder(nextInst);
                 endBuilder.createEndBorrow(getSafeLoc(nextInst), borrow);
               }
-              continue;
+              goto update_operand;
             }
           }
 
           insertEndBorrowsForNonConsumingUse(&operand, borrow);
+update_operand:
+          // We update the operand after placing end_borrows, since we might
+          // need the original operand's lifetime to correctly delineate the
+          // new lifetime, such as if there is an InteriorPointerOperand.
+          
+          // NOTE: oldInst may be nullptr if our operand is a SILArgument
+          // which can happen with switch_enum.
+          auto *oldInst = operand.get()->getDefiningInstruction();
+          operand.set(value);
+          if (oldInst && deleter)
+            deleter->forceTrackAsDead(oldInst);
+
           continue;
         }
 

test/SILOptimizer/moveonly_borrow_to_destructure_class_projection.swift

@@ -0,0 +1,35 @@
+// rdar://133333278
+
+// RUN: %target-swift-frontend -emit-sil -verify %s
+
+public final class List<Element> where Element: ~Copyable {
+    public private(set) var count: Int
+            
+    @inlinable
+    public var capacity: Int {
+        return buffer.count
+    }
+    
+    @inlinable
+    public static var defaultCapacity: Int { 2 }
+    
+    @_alwaysEmitIntoClient
+    private var buffer: UnsafeMutableBufferPointer<Element>
+
+    public init(emptyWithCapacity initialCapacity: Int = defaultCapacity) {
+        precondition(initialCapacity >= 0)
+        self.count = 0
+        self.buffer = .allocate(capacity: initialCapacity)
+    }
+}
+
+private struct ListProcessor: ~Copyable {
+    fileprivate struct ListElement {
+    }
+    
+    fileprivate var tasks: List<ListElement>
+    fileprivate func process() {
+        for _ in 0..<tasks.count {
+        }
+    }
+}