Consume switch subjects under an opaque sub-access.

This prevents the move-only checker from trying to analyze the bindings
as partial consumptions, which ought to be unnecessary since SILGen
will always fully consume the subject as part of forming the bindings.

Author: jckarter

lib/SILGen/SILGenPattern.cpp

@@ -1289,8 +1289,8 @@ void PatternMatchEmission::bindIrrefutablePatterns(const ClauseRow &row,
 
 void PatternMatchEmission::bindIrrefutableBorrows(const ClauseRow &row,
                                                   ArgArray args,
-                                                   bool forIrrefutableRow,
-                                                   bool hasMultipleItems) {
+                                                  bool forIrrefutableRow,
+                                                  bool hasMultipleItems) {
   assert(row.columns() == args.size());
   for (unsigned i = 0, e = args.size(); i != e; ++i) {
     if (!row[i]) // We use null patterns to mean artificial AnyPatterns
@@ -1446,7 +1446,7 @@ void PatternMatchEmission::bindBorrow(Pattern *pattern, VarDecl *var,
     auto access = SGF.B.createBeginAccess(pattern, bindValue.getValue(),
                                           SILAccessKind::Read,
                                           SILAccessEnforcement::Static,
-                                          false, false);
+                                          /*no nested conflict*/ true, false);
     SGF.Cleanups.pushCleanup<EndAccessCleanup>(access);
     bindValue = ManagedValue::forBorrowedAddressRValue(access);
   }
@@ -2797,11 +2797,23 @@ void PatternMatchEmission::emitDestructiveCaseBlocks() {
     // Create a scope to break down the subject value.
     Scope caseScope(SGF, pattern);
 
+    // If the subject value is in memory, enter a deinit access for the memory.
+    // This saves the move-only-checker from trying to analyze the payload
+    // decomposition as a potential partial consume. We always fully consume
+    // the subject on this path.
+    auto origSubject = NoncopyableConsumableValue.forward(SGF);
+    if (origSubject->getType().isAddress()) {
+      origSubject = SGF.B.createBeginAccess(pattern, origSubject,
+                                            SILAccessKind::Deinit,
+                                            SILAccessEnforcement::Static,
+                                            /*no nested conflict*/ true, false);
+      SGF.Cleanups.pushCleanup<EndAccessCleanup>(origSubject);
+    }
+    
     // Clone the original subject's cleanup state so that it will be reliably
     // consumed in this scope, while leaving the original for other case
     // blocks to re-consume.
-    ManagedValue subject = SGF.emitManagedRValueWithCleanup(
-                                     NoncopyableConsumableValue.forward(SGF));
+    ManagedValue subject = SGF.emitManagedRValueWithCleanup(origSubject);
 
     // TODO: handle fallthroughs and multiple cases bindings
     // In those cases we'd need to forward bindings through the shared case
@@ -3428,7 +3440,8 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
               // match is not allowed to modify it.
               auto access = B.createBeginAccess(S, subjectMV.getValue(),
                 SILAccessKind::Read,
-                SILAccessEnforcement::Static, false, false);
+                SILAccessEnforcement::Static,
+                /*no nested conflict*/ true, false);
               Cleanups.pushCleanup<EndAccessCleanup>(access);
               subjectMV = ManagedValue::forBorrowedAddressRValue(access);
             }

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

@@ -1978,17 +1978,13 @@ struct GatherUsesVisitor : public TransitiveAddressWalker<GatherUsesVisitor> {
 } // end anonymous namespace
 
 bool GatherUsesVisitor::visitTransitiveUseAsEndPointUse(Operand *op) {
-  // If an access is checked by its own strict mark_unresolved_noncopyable
-  // instruction, then treat the access as an opaque borrowing use from the
-  // outside.
+  // If an access is static and marked as "no nested conflict", we use that
+  // in switch codegen to mark an opaque sub-access that move-only checking
+  // should not look through.
   if (auto ba = dyn_cast<BeginAccessInst>(op->getUser())) {
-    for (auto accessUse : ba->getUses()) {
-      if (auto mark
-         = dyn_cast<MarkUnresolvedNonCopyableValueInst>(accessUse->getUser())) {
-        if (mark->isStrict()) {
-          return true;
-        }
-      }
+    if (ba->getEnforcement() == SILAccessEnforcement::Static
+        && ba->hasNoNestedConflict()) {
+      return true;
     }
   }
   return false;
@@ -2548,20 +2544,29 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
     return true;
   }
 
-  // Treat an opaque read access as a borrow liveness use for the duration
-  // of the access.
   if (auto *access = dyn_cast<BeginAccessInst>(op->getUser())) {
-    assert(access->getAccessKind() == SILAccessKind::Read);
-    LLVM_DEBUG(llvm::dbgs() << "begin_access use\n");
-    
-    auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
-    if (!leafRange) {
-      LLVM_DEBUG(llvm::dbgs() << "Failed to form leaf type range!\n");
-      return false;
-    }
+    switch (access->getAccessKind()) {
+    // Treat an opaque read access as a borrow liveness use for the duration
+    // of the access.
+    case SILAccessKind::Read: {
+      LLVM_DEBUG(llvm::dbgs() << "begin_access [read]\n");
+      
+      auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
+      if (!leafRange) {
+        LLVM_DEBUG(llvm::dbgs() << "Failed to form leaf type range!\n");
+        return false;
+      }
 
-    useState.recordLivenessUse(user, *leafRange);
-    return true;
+      useState.recordLivenessUse(user, *leafRange);
+      return true;
+    }
+    // Treat a deinit access as a consume of the entire value.
+    case SILAccessKind::Deinit:
+      llvm_unreachable("should have been handled by `memInstMustConsume`");
+    case SILAccessKind::Init:
+    case SILAccessKind::Modify:
+      llvm_unreachable("should look through these kinds of accesses currently");
+    }
   }
 
   // If we don't fit into any of those categories, just track as a liveness

lib/SILOptimizer/Mandatory/MoveOnlyUtils.cpp

@@ -313,6 +313,8 @@ bool noncopyable::memInstMustConsume(Operand *memOper) {
   case SILInstructionKind::LoadInst:
     return cast<LoadInst>(memInst)->getOwnershipQualifier() ==
            LoadOwnershipQualifier::Take;
+  case SILInstructionKind::BeginAccessInst:
+    return cast<BeginAccessInst>(memInst)->getAccessKind() == SILAccessKind::Deinit;
   }
 }
 

test/SILOptimizer/moveonly_borrowing_switch.swift

@@ -236,8 +236,8 @@ func testOuterAO(consuming bas: consuming AOBas) { // expected-error{{'bas' used
         break
     }
 
-    switch bas {
-    case _borrowing x: // expected-warning{{}} expected-note{{used here}}
+    switch bas { // expected-note{{used here}}
+    case _borrowing x: // expected-warning{{}}
         break
     }
 }

test/SILOptimizer/moveonly_consuming_switch.swift

@@ -0,0 +1,80 @@
+// RUN: %target-swift-frontend -emit-sil -enable-experimental-feature NoncopyableGenerics -enable-experimental-feature BorrowingSwitch -enable-experimental-feature MoveOnlyPartialConsumption -verify %s
+struct MyPointer<Wrapped: ~Copyable>: Copyable {
+    var v: UnsafeMutablePointer<Int>
+
+    static func allocate(capacity: Int) -> Self {
+        fatalError()
+    }
+
+    func initialize(to: consuming Wrapped) {
+    }
+    func deinitialize(count: Int) {
+    }
+    func deallocate() {
+    }
+    func move() -> Wrapped {
+        fatalError()
+    }
+}
+
+struct Box<Wrapped: ~Copyable>: ~Copyable {
+    private let _pointer: MyPointer<Wrapped>
+    
+    init(_ element: consuming Wrapped) {
+        _pointer = .allocate(capacity: 1)
+        print("allocating",_pointer)
+        _pointer.initialize(to: element)
+    }
+        
+    deinit {
+        print("deallocating",_pointer)
+        _pointer.deinitialize(count: 1)
+        _pointer.deallocate()
+    }
+    
+    consuming func move() -> Wrapped {
+        let wrapped = _pointer.move()
+        print("deallocating", _pointer)
+        _pointer.deallocate()
+        discard self
+        return wrapped
+    }
+}
+
+
+
+
+
+
+
+
+
+enum List<Element: ~Copyable>: ~Copyable {
+    case empty
+    case node(Element, Box<List>)
+}
+
+extension List {
+    init() { self = .empty }
+    
+    mutating func push(_ element: consuming Element) {
+        self = .node(element, Box(self))
+    }
+        
+    mutating func pop() -> Element {
+        switch self {
+        case .node(let element, let box):
+            self = box.move()
+            return element
+        case .empty:
+            fatalError()
+        }
+    }
+    
+    var isEmpty: Bool {
+        switch self {
+        case .empty: true
+        case .node: false
+        }
+    }
+}

test/SILOptimizer/moveonly_consuming_switch_2.swift

@@ -0,0 +1,27 @@
+// RUN: %target-swift-frontend -emit-sil -enable-experimental-feature BorrowingSwitch -enable-experimental-feature MoveOnlyPartialConsumption -verify %s
+
+struct Box: ~Copyable {
+    let ptr: UnsafeMutablePointer<Int>
+    deinit { print("butt") }
+}
+
+enum List<Element>: ~Copyable {
+    case end
+    case more(Element, Box)
+}
+
+extension List {
+    init() {
+        self = .end
+    }
+
+    mutating func pop() -> Element {
+        switch consume self {
+        case .more(let element, let box): // expected-warning{{}}
+            self = .end
+            return element
+        case .end: fatalError()
+        }
+    }
+}
+