Merge pull request #71431 from jckarter/borrowing-switch-6

SILGen: Improve handling of copyable subpatterns in borrowing switches.

Author: jckarter

lib/AST/Pattern.cpp

@@ -775,13 +775,19 @@ Pattern::getOwnership(
     }
 
     void visitNamedPattern(NamedPattern *p) {
-      // `var` and `let` bindings consume the matched value.
-      // TODO: borrowing/mutating/consuming parameters
       switch (p->getDecl()->getIntroducer()) {
       case VarDecl::Introducer::Let:
       case VarDecl::Introducer::Var:
-        // `let` and `var` consume the bound value to move it into a new
-        // independent variable.
+        // If the subpattern type is copyable, then we can bind the variable
+        // by copying without requiring more than a borrow of the original.
+        if (!p->hasType() || !p->getType()->isNoncopyable()) {
+          break;
+        }
+        // TODO: An explicit `consuming` binding kind consumes regardless of
+        // type.
+      
+        // Noncopyable `let` and `var` consume the bound value to move it into
+        // a new independent variable.
         increaseOwnership(ValueOwnership::Owned, p);
         break;
 
@@ -805,9 +811,9 @@ Pattern::getOwnership(
     }
 
     void visitIsPattern(IsPattern *p) {
-      // Casting currently always consumes.
-      // TODO: Sometimes maybe it doesn't need to be.
-      increaseOwnership(ValueOwnership::Owned, p);
+      // Casting has to either be possible by borrowing or copying the subject,
+      // or can't be supported in a pattern match.
+      /* no change */
     }
 
     void visitEnumElementPattern(EnumElementPattern *p) {
@@ -821,11 +827,9 @@ Pattern::getOwnership(
     }
 
     void visitExprPattern(ExprPattern *p) {
-      // We can't get the ownership reliably if the pattern hasn't been resolved.
-      if (!p->isResolved()) {
-        return;
-      }
-      increaseOwnership(p->getMatchOperandOwnership(), p);
+      // A `~=` operator has to be able to either borrow or copy the operand,
+      // or can't be used.
+      /* no change */
     }
   };
 

lib/SILGen/SILGenPattern.cpp

@@ -531,7 +531,8 @@ class PatternMatchEmission {
                                bool forIrrefutableRow, bool hasMultipleItems);
 
   // Bind noncopyable variable bindings as borrows.
-  void bindIrrefutableBorrows(const ClauseRow &row, ArgArray args);
+  void bindIrrefutableBorrows(const ClauseRow &row, ArgArray args,
+                               bool forIrrefutableRow, bool hasMultipleItems);
 
   // End the borrow of the subject and derived values during a move-only match.
   void unbindAndEndBorrows(const ClauseRow &row, ArgArray args);
@@ -1187,7 +1188,8 @@ void PatternMatchEmission::emitWildcardDispatch(ClauseMatrix &clauses,
     // If the final pattern match is only borrowing as well,
     // we can bind the variables immediately here too.
     if (*ownership <= ValueOwnership::Shared) {
-      bindIrrefutableBorrows(clauses[row], args);
+      bindIrrefutableBorrows(clauses[row], args,
+                             !hasGuard, hasMultipleItems);
     }
 
     if (hasGuard) {
@@ -1286,7 +1288,9 @@ void PatternMatchEmission::bindIrrefutablePatterns(const ClauseRow &row,
 }
 
 void PatternMatchEmission::bindIrrefutableBorrows(const ClauseRow &row,
-                                                  ArgArray args) {
+                                                  ArgArray args,
+                                                   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
@@ -1301,7 +1305,16 @@ void PatternMatchEmission::bindIrrefutableBorrows(const ClauseRow &row,
       break;
     case PatternKind::Named: {
       NamedPattern *named = cast<NamedPattern>(pattern);
-      bindBorrow(pattern, named->getDecl(), args[i]);
+      // If the subpattern matches a copyable type, and the match isn't
+      // explicitly `borrowing`, then we can bind it as a normal copyable
+      // value.
+      if (named->getDecl()->getIntroducer() != VarDecl::Introducer::Borrowing
+          && !named->getType()->isNoncopyable()) {
+        bindVariable(pattern, named->getDecl(), args[i], forIrrefutableRow,
+                     hasMultipleItems);
+      } else {
+        bindBorrow(pattern, named->getDecl(), args[i]);
+      }
       break;
     }
     default:
@@ -1391,14 +1404,25 @@ void PatternMatchEmission::bindBorrow(Pattern *pattern, VarDecl *var,
   assert(value.getFinalConsumption() == CastConsumptionKind::BorrowAlways);
 
   auto bindValue = value.asBorrowedOperand2(SGF, pattern).getFinalManagedValue();
-  if (bindValue.getType().isMoveOnly()) {
-    if (bindValue.getType().isObject()) {
-      // Create a notional copy for the borrow checker to use.
-      bindValue = bindValue.copy(SGF, pattern);
+
+  // Borrow bindings of copyable type should still be no-implicit-copy.
+  if (!bindValue.getType().isMoveOnly()) {
+    if (bindValue.getType().isAddress()) {
+      bindValue = ManagedValue::forBorrowedAddressRValue(
+        SGF.B.createCopyableToMoveOnlyWrapperAddr(pattern, bindValue.getValue()));
+    } else {
+      bindValue =
+        SGF.B.createGuaranteedCopyableToMoveOnlyWrapperValue(pattern, bindValue);
     }
-    bindValue = SGF.B.createMarkUnresolvedNonCopyableValueInst(pattern, bindValue,
-                MarkUnresolvedNonCopyableValueInst::CheckKind::NoConsumeOrAssign);
   }
+
+  if (bindValue.getType().isObject()) {
+    // Create a notional copy for the borrow checker to use.
+    bindValue = bindValue.copy(SGF, pattern);
+  }
+  bindValue = SGF.B.createMarkUnresolvedNonCopyableValueInst(pattern, bindValue,
+              MarkUnresolvedNonCopyableValueInst::CheckKind::NoConsumeOrAssign);
+
   SGF.VarLocs[var] = SILGenFunction::VarLoc::get(bindValue.getValue());
 }
 
@@ -1420,7 +1444,9 @@ void PatternMatchEmission::emitGuardBranch(SILLocation loc, Expr *guard,
     // subject yet.
     if (auto ownership = getNoncopyableOwnership()) {
       if (*ownership > ValueOwnership::Shared) {
-        bindIrrefutableBorrows(row, args);
+        bindIrrefutableBorrows(row, args,
+                               /*irrefutable*/ false,
+                               /*multiple items*/ false);
       }
     }
     testBool = SGF.emitRValueAsSingleValue(guard).getUnmanagedValue();

test/SILOptimizer/moveonly_borrowing_switch_copyable_subpattern.swift

@@ -0,0 +1,55 @@
+// RUN: %target-swift-frontend -emit-sil -verify -enable-experimental-feature BorrowingSwitch -disable-experimental-parser-round-trip %s
+
+struct Payload: ~Copyable {
+    var x: Int
+    var y: String
+}
+
+enum Foo: ~Copyable {
+    case nonCopyablePayload(Payload)
+    case copyablePayload(String)
+}
+
+func hungryCondition(_: consuming String) -> Bool { fatalError() }
+func condition(_: borrowing String) -> Bool { fatalError() }
+
+func eat(_: consuming String) {}
+func nibble(_: borrowing String) {}
+
+func test(borrowing foo: borrowing Foo) {
+    switch foo {
+    case .nonCopyablePayload(_borrowing x): // expected-warning{{}}
+        break
+
+    // OK to form a normal `let` binding when the payload is copyable.
+    // Then it's OK to consume copies of it in the condition clause
+    // and in the body.
+    case .copyablePayload(let x) where hungryCondition(x):
+        eat(x)
+        nibble(x)
+    case .copyablePayload(let x) where condition(x):
+        eat(x)
+        nibble(x)
+
+    // `_borrowing` match variables impose the no-implicit-copy constraint
+    // like `borrowing` parameters do.
+    case .copyablePayload(_borrowing x) // expected-error{{'x' is borrowed and cannot be consumed}}
+      where hungryCondition(x): // expected-note{{consumed here}}
+        eat(x) // expected-note{{consumed here}}
+        nibble(x)
+
+    case .copyablePayload(_borrowing x) // expected-error{{'x' is borrowed and cannot be consumed}}
+      where condition(x):
+        eat(x) // expected-note{{consumed here}}
+        nibble(x)
+
+    // Explicit copies are OK.
+    case .copyablePayload(_borrowing x)
+      where hungryCondition(copy x):
+        eat(copy x)
+        nibble(x)
+
+    case .copyablePayload(_borrowing x):
+        nibble(x)
+    }
+}

test/Sema/switch-ownership.swift

@@ -31,7 +31,7 @@ func test(value: A) {
     case C():
         break
     // CHECK: (case_stmt
-    // CHECK:   (case_label_item ownership=consuming
+    // CHECK:   (case_label_item ownership=borrowing
     // CHECK:     (pattern_expr type="A" ownership=consuming
     case D():
         break
@@ -46,7 +46,7 @@ func test(value: A) {
     case F():
         break
     // CHECK: (case_stmt
-    // CHECK:   (case_label_item ownership=consuming
+    // CHECK:   (case_label_item ownership=borrowing
     // CHECK:     (pattern_expr type="A" ownership=consuming
     case G():
         break