Merge branch 'main' into async-sequence-fixes

Author: DougGregor

docs/ABI/CallConvSummary.rst

@@ -140,9 +140,9 @@ Register usage
 | ``x8``   |         | Indirect result         |          |          |          |
 |          |         | location register       |          |          |          |
 +----------+---------+-------------------------+----------+----------+----------+
-| ``x16``  | ``ip0`` | Scratch registers (used |          |          |          |
-+----------+---------+ by dyld, can be used    |          |          |          |
-| ``x17``  | ``ip1`` | freely otherwise)       |          |          |          |
+| ``x16``, | ``ip0``,| Scratch registers (used |          |          |          |
+| ``x17``  | ``ip1`` | by dyld, can be used    |          |          |          |
+|          |         | freely otherwise)       |          |          |          |
 +----------+---------+-------------------------+----------+----------+----------+
 | ``x18``  |         | RESERVED **DO NOT USE** |          |          |          |
 +----------+---------+-------------------------+----------+----------+----------+

include/swift/ABI/Executor.h

@@ -155,8 +155,12 @@ class SerialExecutorRef {
     return reinterpret_cast<DefaultActor*>(Identity);
   }
 
+  bool hasSerialExecutorWitnessTable() const {
+    return !isGeneric() && !isDefaultActor();
+  }
+
   const SerialExecutorWitnessTable *getSerialExecutorWitnessTable() const {
-    assert(!isGeneric() && !isDefaultActor());
+    assert(hasSerialExecutorWitnessTable());
     auto table = Implementation & WitnessTableMask;
     return reinterpret_cast<const SerialExecutorWitnessTable*>(table);
   }

include/swift/AST/DiagnosticsSema.def

@@ -4669,7 +4669,7 @@ ERROR(value_of_module_type,none,
       "expected module member name after module name", ())
 
 ERROR(value_of_metatype_type,none,
-      "expected member name or constructor call after type name%select{|; this "
+      "expected member name or initializer call after type name%select{|; this "
       "will be an error in Swift 6}0", (bool))
 
 NOTE(add_parens_to_type,none,
@@ -6516,9 +6516,9 @@ ERROR(dynamic_replacement_replaced_not_objc_dynamic, none,
 ERROR(dynamic_replacement_replacement_not_objc_dynamic, none,
       "%0 is marked @objc dynamic", (DeclName))
 ERROR(dynamic_replacement_replaced_constructor_is_convenience, none,
-      "replaced constructor %0 is marked as convenience", (DeclNameRef))
+      "replaced initializer %0 is marked as convenience", (DeclNameRef))
 ERROR(dynamic_replacement_replaced_constructor_is_not_convenience, none,
-      "replaced constructor %0 is not marked as convenience", (DeclNameRef))
+      "replaced initializer %0 is not marked as convenience", (DeclNameRef))
 
 //------------------------------------------------------------------------------
 // MARK: @_typeEraser()

include/swift/Runtime/Bincompat.h

@@ -2,7 +2,7 @@
 //
 // This source file is part of the Swift.org open source project
 //
-// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
+// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception
 //
 // See https://swift.org/LICENSE.txt for license information
@@ -47,6 +47,38 @@ bool useLegacySwiftValueUnboxingInCasting();
 /// if present.
 bool useLegacySwiftObjCHashing();
 
+/// Legacy semantics allowed for the `swift_task_reportUnexpectedExecutor` to
+/// only log a warning. This changes in future releases and this function
+/// will fatal error always.
+///
+/// Similarly, the internal runtime function
+/// `swift_task_isCurrentExecutor(expected)` was previously allowed to return
+/// `false`. In future releases it will call into `checkIsolated`, and CRASH
+/// when previously it would have returned false.
+///
+/// Because some applications were running with "isolation warnings" and
+/// those call into the `isCurrentExecutor` API and expected warnings to be
+/// logged, but they ignored those warnings we cannot make them crashing,
+/// and must check if the app was built against a new.
+///
+/// Old behavior:
+/// - `swift_task_isCurrentExecutorImpl` cannot crash and does NOT invoke
+///     `SerialExecutor.checkIsolated`
+/// - `swift_task_isCurrentExecutorImpl` does not invoke `checkIsolated`
+/// - logging a warning on concurrency violation is allowed
+///
+/// New behavior:
+/// - always fatal error in `swift_task_reportUnexpectedExecutor`
+/// - `swift_task_isCurrentExecutorImpl` will crash when it would have returned
+///     false
+/// - `swift_task_isCurrentExecutorImpl` does invoke `checkIsolated` when other
+///     checks failed
+///
+/// This can be overridden by using `SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=1`
+/// or `SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=crash|nocrash`
+SWIFT_RUNTIME_STDLIB_SPI
+bool swift_bincompat_useLegacyNonCrashingExecutorChecks();
+
 } // namespace bincompat
 
 } // namespace runtime

include/swift/Runtime/Concurrency.h

@@ -715,6 +715,11 @@ void swift_task_enqueue(Job *job, SerialExecutorRef executor);
 SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
 void swift_task_enqueueGlobal(Job *job);
 
+/// Invoke an executor's `checkIsolated` or otherwise equivalent API,
+/// that will crash if the current executor is NOT the passed executor.
+SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
+void swift_task_checkIsolated(SerialExecutorRef executor);
+
 /// A count in nanoseconds.
 using JobDelay = unsigned long long;
 
@@ -729,6 +734,9 @@ void swift_task_enqueueGlobalWithDeadline(long long sec, long long nsec,
 SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
 void swift_task_enqueueMainExecutor(Job *job);
 
+/// WARNING: This method is expected to CRASH when caller is not on the
+/// expected executor.
+///
 /// Return true if the caller is running in a Task on the passed Executor.
 SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
 bool swift_task_isOnExecutor(
@@ -781,6 +789,12 @@ SWIFT_CC(swift) void (*swift_task_enqueueGlobalWithDeadline_hook)(
     int clock, Job *job,
     swift_task_enqueueGlobalWithDeadline_original original);
 
+typedef SWIFT_CC(swift) void (*swift_task_checkIsolated_original)(SerialExecutorRef executor);
+SWIFT_EXPORT_FROM(swift_Concurrency)
+SWIFT_CC(swift) void (*swift_task_checkIsolated_hook)(
+    SerialExecutorRef executor, swift_task_checkIsolated_original original);
+
+
 typedef SWIFT_CC(swift) bool (*swift_task_isOnExecutor_original)(
     HeapObject *executor,
     const Metadata *selfType,

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());