[region-isolation] Handle non-transferrable function arguments via an explicitly transferred parameter.

I also while doing this I replaced a bunch of places where we used to crash due
to invariants failing to instead emit a "I don't know error" since it is more
actionable for the user.

Author: gottesmm

include/swift/AST/DiagnosticsSIL.def

@@ -910,6 +910,9 @@ ERROR(regionbasedisolation_transfer_yields_race_stronglytransferred_binding, non
 ERROR(regionbasedisolation_arg_transferred, none,
       "task isolated value of type %0 transferred to %1 context; later accesses to value could race",
       (Type, ActorIsolation))
+ERROR(regionbasedisolation_arg_passed_to_strongly_transferred_param, none,
+      "task isolated value of type %0 passed as a strongly transferred parameter; later accesses could race",
+      (Type))
 NOTE(regionbasedisolation_maybe_race, none,
      "access here could race", ())
 NOTE(regionbasedisolation_isolated_since_in_same_region_basename, none,

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

@@ -662,7 +662,8 @@ void UseAfterTransferDiagnosticInferrer::init(const Operand *op) {
 
   auto *autoClosureExpr = loc.getAsASTNode<AutoClosureExpr>();
   if (!autoClosureExpr) {
-    llvm::report_fatal_error("Transfer error emission missing a case?!");
+    diagnoseError(op->getUser(), diag::regionbasedisolation_unknown_pattern);
+    return;
   }
 
   auto *i = const_cast<SILInstruction *>(op->getUser());
@@ -710,6 +711,10 @@ class TransferNonTransferrableDiagnosticInferrer {
 
     /// Used if we have a function argument that is transferred into an closure.
     FunctionArgumentClosure = 3,
+
+    /// Used if we have a function argument passed as an explicitly strongly
+    /// transferring argument to a function.
+    FunctionArgumentApplyStronglyTransferred = 4,
   };
 
   struct UseDiagnosticInfo {
@@ -730,6 +735,11 @@ class TransferNonTransferrableDiagnosticInferrer {
       return {UseDiagnosticInfoKind::FunctionArgumentClosure, isolation};
     }
 
+    static UseDiagnosticInfo forFunctionArgumentApplyStronglyTransferred() {
+      return {UseDiagnosticInfoKind::FunctionArgumentApplyStronglyTransferred,
+              {}};
+    }
+
   private:
     UseDiagnosticInfo(UseDiagnosticInfoKind kind,
                       std::optional<ActorIsolation> isolation)
@@ -747,7 +757,11 @@ class TransferNonTransferrableDiagnosticInferrer {
       : info(info),
         loc(info.transferredOperand->getUser()->getLoc().getSourceLoc()) {}
 
-  void run();
+  /// Gathers diagnostics. Returns false if we emitted a "I don't understand
+  /// error". If we emit such an error, we should bail without emitting any
+  /// further diagnostics, since we may not have any diagnostics or be in an
+  /// inconcistent state.
+  bool run();
 
   UseDiagnosticInfo getDiagnostic() const { return diagnosticInfo.value(); }
   SourceLoc getLoc() const { return loc; }
@@ -779,27 +793,55 @@ bool TransferNonTransferrableDiagnosticInferrer::initForIsolatedPartialApply(
   return false;
 }
 
-void TransferNonTransferrableDiagnosticInferrer::run() {
-  if (isa<SILFunctionArgument>(info.nonTransferrableValue)) {
-    // We need to find the isolation info.
-    auto loc = info.transferredOperand->getUser()->getLoc();
+bool TransferNonTransferrableDiagnosticInferrer::run() {
+  if (!isa<SILFunctionArgument>(info.nonTransferrableValue)) {
+    diagnosticInfo = UseDiagnosticInfo::forMiscUse();
+    return true;
+  }
 
-    if (auto *sourceApply = loc.getAsASTNode<ApplyExpr>()) {
-      diagnosticInfo = UseDiagnosticInfo::forFunctionArgumentApply(
-          sourceApply->getIsolationCrossing().value().getCalleeIsolation());
-      return;
-    }
+  // We need to find the isolation info.
+  auto loc = info.transferredOperand->getUser()->getLoc();
 
-    if (auto *ace = loc.getAsASTNode<AbstractClosureExpr>()) {
-      if (ace->getActorIsolation().isActorIsolated()) {
-        if (initForIsolatedPartialApply(info.transferredOperand, ace)) {
-          return;
+  if (auto *sourceApply = loc.getAsASTNode<ApplyExpr>()) {
+    std::optional<ActorIsolation> isolation = {};
+
+    // First try to get the apply from the isolation crossing.
+    if (auto value = sourceApply->getIsolationCrossing())
+      isolation = value->getCalleeIsolation();
+
+    // If we could not infer an isolation...
+    if (!isolation) {
+      // First see if we have a transferring argument.
+      if (auto fas = FullApplySite::isa(info.transferredOperand->getUser())) {
+        if (fas.getArgumentParameterInfo(*info.transferredOperand)
+                .hasOption(SILParameterInfo::Transferring)) {
+          diagnosticInfo =
+              UseDiagnosticInfo::forFunctionArgumentApplyStronglyTransferred();
+          return true;
         }
       }
+
+      // Otherwise, emit a "we don't know error" that tells the user to file a
+      // bug.
+      diagnoseError(info.transferredOperand->getUser(),
+                    diag::regionbasedisolation_unknown_pattern);
+      return false;
+    }
+
+    diagnosticInfo = UseDiagnosticInfo::forFunctionArgumentApply(*isolation);
+    return true;
+  }
+
+  if (auto *ace = loc.getAsASTNode<AbstractClosureExpr>()) {
+    if (ace->getActorIsolation().isActorIsolated()) {
+      if (initForIsolatedPartialApply(info.transferredOperand, ace)) {
+        return true;
+      }
     }
   }
 
   diagnosticInfo = UseDiagnosticInfo::forMiscUse();
+  return true;
 }
 
 //===----------------------------------------------------------------------===//
@@ -941,6 +983,8 @@ void TransferNonSendableImpl::runDiagnosticEvaluator() {
                workingPartition.print(llvm::dbgs()));
   }
 
+  LLVM_DEBUG(llvm::dbgs() << "Finished walking blocks for diagnostics.\n");
+
   // Now that we have found all of our transferInsts/Requires emit errors.
   transferOpToRequireInstMultiMap.setFrozen();
 }
@@ -1065,18 +1109,21 @@ void TransferNonSendableImpl::emitTransferredNonTransferrableDiagnostics() {
   if (transferredNonTransferrable.empty())
     return;
 
+  LLVM_DEBUG(
+      llvm::dbgs() << "Emitting transfer non transferrable diagnostics.\n");
+
   using UseDiagnosticInfoKind =
       TransferNonTransferrableDiagnosticInferrer::UseDiagnosticInfoKind;
 
   auto &astContext = regionInfo->getFunction()->getASTContext();
   for (auto info : transferredNonTransferrable) {
     auto *op = info.transferredOperand;
     TransferNonTransferrableDiagnosticInferrer diagnosticInferrer(info);
-    diagnosticInferrer.run();
+    if (!diagnosticInferrer.run())
+      continue;
 
     auto diagnosticInfo = diagnosticInferrer.getDiagnostic();
     auto loc = diagnosticInferrer.getLoc();
-
     switch (diagnosticInfo.kind) {
     case UseDiagnosticInfoKind::Invalid:
       llvm_unreachable("Should never see this");
@@ -1120,6 +1167,15 @@ void TransferNonSendableImpl::emitTransferredNonTransferrableDiagnostics() {
           astContext, fArg->getDecl()->getLoc(),
           diag::regionbasedisolation_isolated_since_in_same_region_basename,
           "task isolated", fArg->getDecl()->getBaseName());
+      break;
+    }
+    case UseDiagnosticInfoKind::FunctionArgumentApplyStronglyTransferred: {
+      diagnoseError(
+          astContext, loc,
+          diag::regionbasedisolation_arg_passed_to_strongly_transferred_param,
+          op->get()->getType().getASTType())
+          .highlight(op->getUser()->getLoc().getSourceRange());
+      break;
     }
     }
   }

test/Concurrency/transfernonsendable_warning_until_swift6.swift

@@ -28,3 +28,7 @@ func testIsolationError() async {
 func testArgumentError(_ x: NonSendableType) async { // expected-note {{value is task isolated since it is in the same region as 'x'}}
   await transferToMain(x) // expected-error {{task isolated value of type 'NonSendableType' transferred to main actor-isolated context; later accesses to value could race}}
 }
+
+func testTransferringArgument(_ x: NonSendableType) async {
+  transferValue(x) // expected-error {{task isolated value of type 'NonSendableType' passed as a strongly transferred parameter; later accesses could race}}
+}