Merge pull request #72954 from jckarter/borrowing-switch-expr-based-ownership-6.0

[6.0] Alternative noncopyable switch design based on expression kind.

Author: jckarter

lib/AST/Pattern.cpp

@@ -777,6 +777,9 @@ Pattern::getOwnership(
     void visitNamedPattern(NamedPattern *p) {
       switch (p->getDecl()->getIntroducer()) {
       case VarDecl::Introducer::Let:
+        // `let` defaults to the prevailing ownership of the switch.
+        break;
+      
       case VarDecl::Introducer::Var:
         // If the subpattern type is copyable, then we can bind the variable
         // by copying without requiring more than a borrow of the original.
@@ -786,7 +789,7 @@ Pattern::getOwnership(
         // TODO: An explicit `consuming` binding kind consumes regardless of
         // type.
 
-        // Noncopyable `let` and `var` consume the bound value to move it into
+        // Noncopyable `var` consumes the bound value to move it into
         // a new independent variable.
         increaseOwnership(ValueOwnership::Owned, p);
         break;
@@ -797,8 +800,8 @@ Pattern::getOwnership(
         break;
 
       case VarDecl::Introducer::Borrowing:
-        // `borrow` bindings borrow parts of the value in-place so they don't
-        // need stronger access to the subject value.
+        // `borrow` bindings borrow parts of the value in-place.
+        increaseOwnership(ValueOwnership::Shared, p);        
         break;
       }
     }

lib/SILGen/SILGenPattern.cpp

@@ -3218,12 +3218,123 @@ static void switchCaseStmtSuccessCallback(SILGenFunction &SGF,
   SGF.Cleanups.emitBranchAndCleanups(sharedDest, caseBlock, args);
 }
 
+// TODO: Integrate this with findStorageExprForMoveOnly, which does almost the
+// same check.
+static bool isBorrowableSubject(SILGenFunction &SGF,
+                                Expr *subjectExpr) {
+  // Look through forwarding expressions.
+  for (;;) {
+    subjectExpr = subjectExpr->getValueProvidingExpr();
+    
+    // Look through loads.
+    if (auto load = dyn_cast<LoadExpr>(subjectExpr)) {
+      subjectExpr = load->getSubExpr();
+      continue;
+    }
+    
+    // Look through optional force-projections.
+    // We can't look through optional evaluations here because wrapping the
+    // value up in an Optional at the end needs a copy/move to create the
+    // temporary optional.
+    if (auto force = dyn_cast<ForceValueExpr>(subjectExpr)) {
+      subjectExpr = force->getSubExpr();
+      continue;
+    }
+
+    // Look through parens.
+    if (auto paren = dyn_cast<ParenExpr>(subjectExpr)) {
+      subjectExpr = paren->getSubExpr();
+      continue;
+    }
+
+    // Look through `try` and `await`.
+    if (auto tryExpr = dyn_cast<TryExpr>(subjectExpr)) {
+      subjectExpr = tryExpr->getSubExpr();
+      continue;
+    }
+    if (auto awaitExpr = dyn_cast<AwaitExpr>(subjectExpr)) {
+      subjectExpr = awaitExpr->getSubExpr();
+      continue;
+    }
+    
+    break;
+  }
+  
+  // An explicit `borrow` expression requires us to do a borrowing access.
+  if (isa<BorrowExpr>(subjectExpr)) {
+    return true;
+  }
+  
+  AbstractStorageDecl *storage;
+  AccessSemantics access;
+  
+  // A subject is potentially borrowable if it's some kind of storage reference.
+  if (auto declRef = dyn_cast<DeclRefExpr>(subjectExpr)) {
+    storage = dyn_cast<AbstractStorageDecl>(declRef->getDecl());
+    access = declRef->getAccessSemantics();
+  } else if (auto memberRef = dyn_cast<MemberRefExpr>(subjectExpr)) {
+    storage = dyn_cast<AbstractStorageDecl>(memberRef->getMember().getDecl());
+    access = memberRef->getAccessSemantics();
+  } else if (auto subscript = dyn_cast<SubscriptExpr>(subjectExpr)) {
+    storage = dyn_cast<AbstractStorageDecl>(subscript->getMember().getDecl());
+    access = subscript->getAccessSemantics();
+  } else {
+    return false;
+  }
+  
+  // If the member being referenced isn't storage, there's no benefit to
+  // borrowing it.
+  if (!storage) {
+    return false;
+  }
+  
+  // Check the access strategy used to read the storage.
+  auto strategy = storage->getAccessStrategy(access,
+                                             AccessKind::Read,
+                                             SGF.SGM.SwiftModule,
+                                             SGF.F.getResilienceExpansion());
+                                             
+  switch (strategy.getKind()) {
+  case AccessStrategy::Kind::Storage:
+    // Accessing storage directly benefits from borrowing.
+    return true;
+  case AccessStrategy::Kind::DirectToAccessor:
+  case AccessStrategy::Kind::DispatchToAccessor:
+    // If we use an accessor, the kind of accessor affects whether we get
+    // a reference we can borrow or a temporary that will be consumed.
+    switch (strategy.getAccessor()) {
+    case AccessorKind::Get:
+    case AccessorKind::DistributedGet:
+      // Get returns an owned value.
+      return false;
+    case AccessorKind::Read:
+    case AccessorKind::Modify:
+    case AccessorKind::Address:
+    case AccessorKind::MutableAddress:
+      // Read, modify, and addressors yield a borrowable reference.
+      return true;
+    case AccessorKind::Init:
+    case AccessorKind::Set:
+    case AccessorKind::WillSet:
+    case AccessorKind::DidSet:
+      llvm_unreachable("should not be involved in a read");
+    }
+    llvm_unreachable("switch not covered?");
+    
+  case AccessStrategy::Kind::MaterializeToTemporary:
+  case AccessStrategy::Kind::DispatchToDistributedThunk:
+    return false;
+  }
+  llvm_unreachable("switch not covered?");
+}
+
 void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
   LLVM_DEBUG(llvm::dbgs() << "emitting switch stmt\n";
              S->dump(llvm::dbgs());
              llvm::dbgs() << '\n');
 
-  auto subjectTy = S->getSubjectExpr()->getType();
+  auto subjectExpr = S->getSubjectExpr();
+  auto subjectTy = subjectExpr->getType();
   auto subjectLoweredTy = getLoweredType(subjectTy);
   auto subjectLoweredAddress =
     silConv.useLoweredAddresses() && subjectLoweredTy.isAddressOnly(F);
@@ -3244,7 +3355,14 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
   if (getASTContext().LangOpts.hasFeature(Feature::BorrowingSwitch)) {
     if (subjectTy->isNoncopyable()) {
       // Determine the overall ownership behavior of the switch, based on the
-      // patterns' ownership behavior.
+      // subject expression and the patterns' ownership behavior.
+      
+      // If the subject expression is borrowable, then perform the switch as
+      // a borrow. (A `consume` expression would render the expression
+      // non-borrowable.) Otherwise, perform it as a consume.
+      ownership = isBorrowableSubject(*this, subjectExpr)
+        ? ValueOwnership::Shared
+        : ValueOwnership::Owned;
       for (auto caseLabel : S->getCases()) {
         for (auto item : caseLabel->getCaseLabelItems()) {
           ownership = std::max(ownership, item.getPattern()->getOwnership());

stdlib/public/core/Optional.swift

@@ -846,7 +846,7 @@ public func ?? <T: ~Copyable>(
   optional: consuming T?,
   defaultValue: @autoclosure () throws -> T?
 ) rethrows -> T? {
-  switch optional {
+  switch consume optional {
   case .some(let value):
     return value
   case .none:

test/SILGen/borrowing_switch_return_on_all_paths.swift

@@ -25,7 +25,7 @@ extension NoncopyableList.Link {
     func find(where predicate: (Element)->Bool) -> Maybe<Element> {
         switch self {
         case .empty: return .none
-        case .more(_borrowing box):
+        case .more(let box):
             if predicate(box.wrapped.element) { return .some(box.wrapped.element) }
             return box.wrapped.next.find(where: predicate)
         }

test/SILGen/borrowing_switch_subjects.swift

@@ -25,7 +25,7 @@ func borrowParam(x: borrowing Outer) {
     // CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [strict] [no_consume_or_assign]
     // CHECK: [[BORROW:%.*]] = begin_borrow [[MARK]]
     switch x {
-    case _borrowing y:
+    case let y:
         // CHECK: apply {{.*}}([[BORROW]])
         use(y)
     }
@@ -39,7 +39,7 @@ func borrowParam(x: borrowing Outer) {
     // CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [strict] [no_consume_or_assign] [[COPY_INNER]]
     // CHECK: [[BORROW:%.*]] = begin_borrow [[MARK]]
     switch x.storedInner {
-    case _borrowing y:
+    case let y:
         // CHECK: apply {{.*}}([[BORROW]])
         use(y)
     }
@@ -56,22 +56,24 @@ func borrowParam(x: borrowing Outer) {
     // CHECK: [[MARK2:%.*]] = mark_unresolved_non_copyable_value [strict] [no_consume_or_assign] [[COPY2]]
     // CHECK: [[BORROW2:%.*]] = begin_borrow [[MARK2]]
     switch x.readInner {
-    case _borrowing y:
+    case let y:
         // CHECK: apply {{.*}}([[BORROW2]])
         use(y)
     }
     // CHECK: end_apply [[TOKEN]]
     // CHECK: end_borrow [[BORROW_OUTER]]
 
+    // `temporary()` is an rvalue, so we 
     // CHECK: [[FN:%.*]] = function_ref @{{.*}}9temporary
     // CHECK: [[TMP:%.*]] = apply [[FN]]()
     // CHECK: [[BORROW_OUTER:%.*]] = begin_borrow [fixed] [[TMP]]
-    // CHECK: [[COPY:%.*]] = copy_value [[BORROW_OUTER]]
-    // CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [strict] [no_consume_or_assign] [[COPY]]
-    // CHECK: [[BORROW:%.*]] = begin_borrow [[MARK]]
+    // CHECK: end_borrow [[BORROW_OUTER]]
+    // CHECK: store [[TMP]] to [init] [[Y:%.*]] :
+    // CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[Y]]
     switch temporary() {
-    case _borrowing y:
-        // CHECK: apply {{.*}}([[BORROW]])
+    case let y:
+        // CHECK: [[LOAD_BORROW:%.*]] = load_borrow [[MARK]]
+        // CHECK: apply {{.*}}([[LOAD_BORROW]])
         use(y)
     }
 }