[rbi] Change Region Based Isolation for closures to not use the AST and instead just use SIL.

The reason why I am doing this is that in certain cases the AST captures indices
will never actually line up with partial apply capture indices since we seem to
"smush" together closures and locally defined functions.

NOTE: The reason for the really small amount of test changes is that this change
does not change the actual output by design. The only cases I had to change were
a case where we began to emit a better diagnostic and also where I added code
coverage around _ and let _ since those require ignored_use to be implemented so
that they would be diagnosed (previously we just did not emit anything so we
couldn't emit the diagnostic at the SIL level).

rdar://142661388

include/swift/AST/DiagnosticsSIL.def

@@ -1056,6 +1056,12 @@ NOTE(regionbasedisolation_typed_tns_passed_to_sending_closure_helper_have_value_
 NOTE(regionbasedisolation_typed_tns_passed_to_sending_closure_helper_multiple_value, none,
      "closure captures non-Sendable %0",
      (DeclName))
+NOTE(regionbasedisolation_closure_captures, none,
+     "closure captures %0",
+     (DeclName))
+NOTE(regionbasedisolation_closure_captures_actor, none,
+     "closure captures %0 allowing access to %1 state within the closure",
+     (DeclName, StringRef))
 
 NOTE(regionbasedisolation_typed_tns_passed_to_sending_callee, none,
      "Passing %0 value of non-Sendable type %1 as a 'sending' parameter to %2 %3 risks causing races inbetween %0 uses and uses reachable from %3",

include/swift/SIL/InstructionUtils.h

@@ -129,6 +129,11 @@ bool isEndOfScopeMarker(SILInstruction *user);
 /// only used in recognizable patterns without otherwise "escaping".
 bool isIncidentalUse(SILInstruction *user);
 
+/// Returns true if this is a move only wrapper use.
+///
+/// E.x.: moveonlywrapper_to_copyable_addr, copyable_to_moveonlywrapper_value
+bool isMoveOnlyWrapperUse(SILInstruction *user);
+
 /// Return true if the given `user` instruction modifies the value's refcount
 /// without propagating the value or having any other effect aside from
 /// potentially destroying the value itself (and executing associated cleanups).

include/swift/SILOptimizer/Utils/PartitionUtils.h

@@ -1374,6 +1374,12 @@ public:
       Region sentRegion = p.getRegion(sentElement);
       bool isClosureCapturedElt = false;
       SILDynamicMergedIsolationInfo sentRegionIsolation;
+
+      // TODO: Today we only return the first element in our region that has
+      // some form of isolation. This causes us to in the case of sending
+      // partial_applies to only emit a diagnostic for the first element in the
+      // capture list of the partial_apply. If we returned a list of potential
+      // errors... we could emit the error for each capture individually.
       auto pairOpt = getIsolationRegionInfo(sentRegion, op.getSourceOp());
       if (!pairOpt) {
         return handleError(UnknownCodePatternError(op));

lib/SIL/Utils/InstructionUtils.cpp

@@ -339,6 +339,19 @@ bool swift::isIncidentalUse(SILInstruction *user) {
          isa<IgnoredUseInst>(user);
 }
 
+bool swift::isMoveOnlyWrapperUse(SILInstruction *user) {
+  switch (user->getKind()) {
+  case SILInstructionKind::MoveOnlyWrapperToCopyableValueInst:
+  case SILInstructionKind::MoveOnlyWrapperToCopyableBoxInst:
+  case SILInstructionKind::MoveOnlyWrapperToCopyableAddrInst:
+  case SILInstructionKind::CopyableToMoveOnlyWrapperValueInst:
+  case SILInstructionKind::CopyableToMoveOnlyWrapperAddrInst:
+    return true;
+  default:
+    return false;
+  }
+}
+
 bool swift::onlyAffectsRefCount(SILInstruction *user) {
   switch (user->getKind()) {
   default:

lib/SILOptimizer/Mandatory/SendNonSendable.cpp

@@ -87,31 +87,30 @@ static SILValue stripFunctionConversions(SILValue val) {
       continue;
     }
 
+    // Look through thunks.
+    if (auto pai = dyn_cast<PartialApplyInst>(val)) {
+      if (pai->getCalleeFunction()->isThunk()) {
+        val = pai->getArgument(0);
+        continue;
+      }
+    }
+
     break;
   }
 
   return val;
 }
 
-static std::optional<DiagnosticBehavior>
-getDiagnosticBehaviorLimitForCapturedValue(SILFunction *fn,
-                                           CapturedValue value) {
-  ValueDecl *decl = value.getDecl();
-  auto *ctx = decl->getInnermostDeclContext();
-  auto type = fn->mapTypeIntoContext(decl->getInterfaceType());
-  return type->getConcurrencyDiagnosticBehaviorLimit(ctx);
-}
-
 /// Find the most conservative diagnostic behavior by taking the max over all
 /// DiagnosticBehavior for the captured values.
 static std::optional<DiagnosticBehavior>
-getDiagnosticBehaviorLimitForCapturedValues(
-    SILFunction *fn, ArrayRef<CapturedValue> capturedValues) {
+getDiagnosticBehaviorLimitForOperands(SILFunction *fn,
+                                      ArrayRef<Operand *> capturedValues) {
   std::optional<DiagnosticBehavior> diagnosticBehavior;
   for (auto value : capturedValues) {
     auto lhs = diagnosticBehavior.value_or(DiagnosticBehavior::Unspecified);
-    auto rhs = getDiagnosticBehaviorLimitForCapturedValue(fn, value).value_or(
-        DiagnosticBehavior::Unspecified);
+    auto limit = value->get()->getType().getConcurrencyDiagnosticBehavior(fn);
+    auto rhs = limit.value_or(DiagnosticBehavior::Unspecified);
     auto result = lhs.merge(rhs);
     if (result != DiagnosticBehavior::Unspecified)
       diagnosticBehavior = result;
@@ -217,6 +216,103 @@ inferNameAndRootHelper(SILValue value) {
   return VariableNameInferrer::inferNameAndRoot(value);
 }
 
+/// Find a use corresponding to the potentially recursive capture of \p
+/// initialOperand that would be appropriate for diagnostics.
+///
+/// \returns the use and the function argument that is used. We return the
+/// function argument since it is a clever way to correctly grab the name of the
+/// captured value since the ValueDecl will point at the actual ValueDecl in the
+/// AST that is captured.
+static std::optional<std::pair<Operand *, SILArgument *>>
+findClosureUse(Operand *initialOperand) {
+  // We have to use a small vector worklist here since we are iterating through
+  // uses from different functions.
+  llvm::SmallVector<std::pair<Operand *, SILArgument *>, 64> worklist;
+  llvm::SmallPtrSet<Operand *, 8> visitedOperand;
+
+  // Initialize our worklist with uses in the initial closure. We do not want to
+  // analyze uses in the original function.
+  {
+    auto as = ApplySite::isa(initialOperand->getUser());
+    if (!as)
+      return {};
+
+    auto *f = as.getCalleeFunction();
+    if (!f)
+      return {};
+
+    unsigned argumentIndex = as.getCalleeArgIndex(*initialOperand);
+    auto *arg = f->getArgument(argumentIndex);
+    for (auto *use : arg->getUses()) {
+      worklist.emplace_back(use, arg);
+      visitedOperand.insert(use);
+    }
+  }
+
+  while (!worklist.empty()) {
+    auto pair = worklist.pop_back_val();
+    auto *op = pair.first;
+    auto *fArg = pair.second;
+    auto *user = op->getUser();
+
+    // Ignore incidental uses that are not specifically ignored use. We want to
+    // visit those since they represent `let _ = $VAR` and `_ = $VAR`
+    if (isIncidentalUse(user) && !isa<IgnoredUseInst>(user))
+      continue;
+
+    // Look through some insts we do not care about.
+    if (isa<CopyValueInst, BeginBorrowInst, ProjectBoxInst, BeginAccessInst>(
+            user) ||
+        isMoveOnlyWrapperUse(user) ||
+        // We want to treat move_value [var_decl] as a real use since we are
+        // assigning to a var.
+        (isa<MoveValueInst>(user) &&
+         !cast<MoveValueInst>(user)->isFromVarDecl())) {
+      for (auto result : user->getResults()) {
+        for (auto *use : result->getUses()) {
+          if (visitedOperand.insert(use).second)
+            worklist.emplace_back(use, fArg);
+        }
+      }
+      continue;
+    }
+
+    // See if we have a callee function. In such a case, find our operand in the
+    // callee and visit its uses.
+    if (auto as = dyn_cast<PartialApplyInst>(op->getUser())) {
+      if (auto *f = as->getCalleeFunction()) {
+        auto *fArg = f->getArgument(ApplySite(as).getCalleeArgIndex(*op));
+        for (auto *use : fArg->getUses()) {
+          if (visitedOperand.insert(use).second)
+            worklist.emplace_back(use, fArg);
+        }
+        continue;
+      }
+    }
+
+    // See if we have a full apply site that was from a closure that was
+    // immediately invoked. In such a case, we can emit a better diagnostic in
+    // the called closure.
+    if (auto fas = FullApplySite::isa(op->getUser())) {
+      if (auto *f = fas.getCalleeFunction();
+          f && f->getDeclRef().getClosureExpr()) {
+        auto *fArg = f->getArgument(fas.getCalleeArgIndex(*op));
+        for (auto *use : fArg->getUses()) {
+          if (visitedOperand.insert(use).second)
+            worklist.emplace_back(use, fArg);
+        }
+        continue;
+      }
+    }
+
+    // Otherwise, we have a real use. Return it and the function argument that
+    // it was derived from.
+    return pair;
+  }
+
+  return {};
+}
+
 //===----------------------------------------------------------------------===//
 //                             MARK: Diagnostics
 //===----------------------------------------------------------------------===//
@@ -855,33 +951,28 @@ private:
 
 bool UseAfterSendDiagnosticInferrer::initForIsolatedPartialApply(
     Operand *op, AbstractClosureExpr *ace) {
-  SmallVector<std::tuple<CapturedValue, unsigned, ApplyIsolationCrossing>, 8>
-      foundCapturedIsolationCrossing;
-  ace->getIsolationCrossing(foundCapturedIsolationCrossing);
-  if (foundCapturedIsolationCrossing.empty())
+  auto diagnosticPair = findClosureUse(op);
+  if (!diagnosticPair) {
     return false;
-
-  unsigned opIndex = ApplySite(op->getUser()).getASTAppliedArgIndex(*op);
-  bool emittedDiagnostic = false;
-  for (auto &p : foundCapturedIsolationCrossing) {
-    if (std::get<1>(p) != opIndex)
-      continue;
-    emittedDiagnostic = true;
-
-    auto &state = sendingOpToStateMap.get(sendingOp);
-    if (auto rootValueAndName = inferNameAndRootHelper(sendingOp->get())) {
-      diagnosticEmitter.emitNamedIsolationCrossingDueToCapture(
-          RegularLocation(std::get<0>(p).getLoc()), rootValueAndName->first,
-          state.isolationInfo.getIsolationInfo(), std::get<2>(p));
-      continue;
-    }
-
-    diagnosticEmitter.emitTypedIsolationCrossingDueToCapture(
-        RegularLocation(std::get<0>(p).getLoc()), baseInferredType,
-        std::get<2>(p));
   }
 
-  return emittedDiagnostic;
+  auto *diagnosticOp = diagnosticPair->first;
+
+  ApplyIsolationCrossing crossing(
+      *op->getFunction()->getActorIsolation(),
+      *diagnosticOp->getFunction()->getActorIsolation());
+
+  auto &state = sendingOpToStateMap.get(sendingOp);
+  if (auto rootValueAndName = inferNameAndRootHelper(sendingOp->get())) {
+    diagnosticEmitter.emitNamedIsolationCrossingDueToCapture(
+        diagnosticOp->getUser()->getLoc(), rootValueAndName->first,
+        state.isolationInfo.getIsolationInfo(), crossing);
+    return true;
+  }
+
+  diagnosticEmitter.emitTypedIsolationCrossingDueToCapture(
+      diagnosticOp->getUser()->getLoc(), baseInferredType, crossing);
+  return true;
 }
 
 void UseAfterSendDiagnosticInferrer::initForApply(Operand *op,
@@ -1327,45 +1418,69 @@ public:
     }
   }
 
-  /// Only use if we were able to find the actual isolated value.
-  void emitTypedSendingNeverSendableToSendingClosureParamDirectlyIsolated(
-      SILLocation loc, CapturedValue capturedValue) {
+  /// Emit an error for a case where we have captured a value like an actor and
+  /// thus a sending closure has become actor isolated (and thus unable to be
+  /// sent).
+  void emitClosureErrorWithCapturedActor(Operand *partialApplyOp,
+                                         Operand *actualUse,
+                                         SILArgument *fArg) {
     SmallString<64> descriptiveKindStr;
     {
       llvm::raw_svector_ostream os(descriptiveKindStr);
-      if (getIsolationRegionInfo().getIsolationInfo().isTaskIsolated()) {
-        os << "code in the current task";
-      } else {
-        getIsolationRegionInfo().printForDiagnostics(os);
-        os << " code";
-      }
+      getIsolationRegionInfo().getIsolationInfo().printForCodeDiagnostic(os);
     }
 
-    diagnoseError(loc,
+    diagnoseError(partialApplyOp,
                   diag::regionbasedisolation_typed_tns_passed_sending_closure,
                   descriptiveKindStr)
-        .highlight(loc.getSourceRange())
-        .limitBehaviorIf(getDiagnosticBehaviorLimitForCapturedValue(
-            getFunction(), capturedValue));
+        .limitBehaviorIf(getDiagnosticBehaviorLimitForOperands(
+            actualUse->getFunction(), {actualUse}));
 
-    auto capturedLoc = RegularLocation(capturedValue.getLoc());
+    descriptiveKindStr.clear();
+    {
+      llvm::raw_svector_ostream os(descriptiveKindStr);
+      getIsolationRegionInfo().getIsolationInfo().printForDiagnostics(os);
+    }
+    diagnoseNote(actualUse, diag::regionbasedisolation_closure_captures_actor,
+                 fArg->getDecl()->getName(), descriptiveKindStr);
+  }
+
+  /// Emit a typed error for an isolated closure being passed as a sending
+  /// parameter.
+  ///
+  /// \arg partialApplyOp the operand of the outermost partial apply.
+  /// \arg actualUse the operand inside the closure that actually caused the
+  /// capture to occur. This maybe inside a different function from the partial
+  /// apply since we want to support a use inside a recursive closure.
+  void emitSendingClosureParamDirectlyIsolated(Operand *partialApplyOp,
+                                               Operand *actualUse,
+                                               SILArgument *fArg) {
+    SmallString<64> descriptiveKindStr;
+    {
+      llvm::raw_svector_ostream os(descriptiveKindStr);
+      getIsolationRegionInfo().getIsolationInfo().printForCodeDiagnostic(os);
+    }
+
+    diagnoseError(partialApplyOp,
+                  diag::regionbasedisolation_typed_tns_passed_sending_closure,
+                  descriptiveKindStr)
+        .limitBehaviorIf(getDiagnosticBehaviorLimitForOperands(
+            actualUse->getFunction(), {actualUse}));
+
+    // If we have a closure capture box, emit a special diagnostic.
     if (getIsolationRegionInfo().getIsolationInfo().isTaskIsolated()) {
-      // If we have a closure capture box, emit a special diagnostic.
-      if (auto *fArg = dyn_cast<SILFunctionArgument>(
-              getIsolationRegionInfo().getIsolationInfo().getIsolatedValue())) {
-        if (fArg->isClosureCapture() && fArg->getType().is<SILBoxType>()) {
-          auto diag = diag::
-              regionbasedisolation_typed_tns_passed_to_sending_closure_helper_have_boxed_value_task_isolated;
-          auto *decl = capturedValue.getDecl();
-          diagnoseNote(capturedLoc, diag, decl->getName(),
-                       decl->getDescriptiveKind());
-          return;
-        }
+      if (cast<SILFunctionArgument>(fArg)->isClosureCapture() &&
+          fArg->getType().is<SILBoxType>()) {
+        auto diag = diag::
+            regionbasedisolation_typed_tns_passed_to_sending_closure_helper_have_boxed_value_task_isolated;
+        diagnoseNote(actualUse, diag, fArg->getDecl()->getName(),
+                     fArg->getDecl()->getDescriptiveKind());
+        return;
       }
 
       auto diag = diag::
           regionbasedisolation_typed_tns_passed_to_sending_closure_helper_have_value_task_isolated;
-      diagnoseNote(capturedLoc, diag, capturedValue.getDecl()->getName());
+      diagnoseNote(actualUse, diag, fArg->getDecl()->getName());
       return;
     }
 
@@ -1377,41 +1492,55 @@ public:
 
     auto diag = diag::
         regionbasedisolation_typed_tns_passed_to_sending_closure_helper_have_value;
-    diagnoseNote(capturedLoc, diag, descriptiveKindStr,
-                 capturedValue.getDecl()->getName());
+    diagnoseNote(actualUse, diag, descriptiveKindStr,
+                 fArg->getDecl()->getName());
   }
 
-  void emitTypedSendingNeverSendableToSendingClosureParam(
-      SILLocation loc, ArrayRef<CapturedValue> capturedValues) {
+  void emitSendingClosureMultipleCapturedOperandError(
+      SILLocation loc, ArrayRef<Operand *> capturedOperands) {
+    // Our caller should have passed at least one operand. Emit an unknown error
+    // to signal we need a bug report.
+    if (capturedOperands.empty()) {
+      emitUnknownPatternError();
+      return;
+    }
+
     SmallString<64> descriptiveKindStr;
     {
       llvm::raw_svector_ostream os(descriptiveKindStr);
-      if (getIsolationRegionInfo().getIsolationInfo().isTaskIsolated()) {
-        os << "code in the current task";
-      } else {
-        getIsolationRegionInfo().printForDiagnostics(os);
-        os << " code";
-      }
+      getIsolationRegionInfo()->printForCodeDiagnostic(os);
     }
 
-    auto behaviorLimit = getDiagnosticBehaviorLimitForCapturedValues(
-        getFunction(), capturedValues);
-    diagnoseError(loc,
-                  diag::regionbasedisolation_typed_tns_passed_sending_closure,
-                  descriptiveKindStr)
-        .highlight(loc.getSourceRange())
-        .limitBehaviorIf(behaviorLimit);
+    auto emitMainError = [&] {
+      auto behaviorLimit = getDiagnosticBehaviorLimitForOperands(
+          getFunction(), capturedOperands);
+      diagnoseError(loc,
+                    diag::regionbasedisolation_typed_tns_passed_sending_closure,
+                    descriptiveKindStr)
+          .highlight(loc.getSourceRange())
+          .limitBehaviorIf(behaviorLimit);
+    };
 
-    if (capturedValues.size() == 1) {
-      auto captured = capturedValues.front();
-      auto capturedLoc = RegularLocation(captured.getLoc());
-      if (getIsolationRegionInfo().getIsolationInfo().isTaskIsolated()) {
-        auto diag = diag::
-            regionbasedisolation_typed_tns_passed_to_sending_closure_helper_have_value_task_isolated;
-        diagnoseNote(capturedLoc, diag, captured.getDecl()->getName());
+    if (capturedOperands.size() == 1) {
+      auto captured = capturedOperands.front();
+      auto actualUseInfo = findClosureUse(captured);
+
+      // If we fail to find actual use info, emit an unknown error.
+      if (!actualUseInfo) {
+        emitUnknownPatternError();
         return;
       }
 
+      if (getIsolationRegionInfo()->isTaskIsolated()) {
+        emitMainError();
+        auto diag = diag::
+            regionbasedisolation_typed_tns_passed_to_sending_closure_helper_have_value_task_isolated;
+        diagnoseNote(actualUseInfo->first, diag,
+                     actualUseInfo->second->getDecl()->getName());
+        return;
+      }
+
+      emitMainError();
       descriptiveKindStr.clear();
       {
         llvm::raw_svector_ostream os(descriptiveKindStr);
@@ -1419,16 +1548,29 @@ public:
       }
       auto diag = diag::
           regionbasedisolation_typed_tns_passed_to_sending_closure_helper_have_value_region;
-      diagnoseNote(capturedLoc, diag, descriptiveKindStr,
-                   captured.getDecl()->getName());
+      diagnoseNote(actualUseInfo->first, diag, descriptiveKindStr,
+                   actualUseInfo->second->getDecl()->getName());
       return;
     }
 
-    for (auto captured : capturedValues) {
-      auto capturedLoc = RegularLocation(captured.getLoc());
+    emitMainError();
+
+    bool emittedDiagnostic = false;
+    for (auto captured : capturedOperands) {
+      auto actualUseInfo = findClosureUse(captured);
+      if (!actualUseInfo)
+        continue;
+      emittedDiagnostic = true;
       auto diag = diag::
           regionbasedisolation_typed_tns_passed_to_sending_closure_helper_multiple_value;
-      diagnoseNote(capturedLoc, diag, captured.getDecl()->getName());
+      diagnoseNote(actualUseInfo->first, diag,
+                   actualUseInfo->second->getDecl()->getName());
+    }
+
+    // Check if we did not emit a diagnostic. In such a case, we need to emit an
+    // unknown patten error so that we get a bug report from the user.
+    if (!emittedDiagnostic) {
+      emitUnknownPatternError();
     }
   }
 
@@ -1543,6 +1685,12 @@ private:
     return diagnoseError(inst->getLoc(), diag, std::forward<U>(args)...);
   }
 
+  template <typename... T, typename... U>
+  InFlightDiagnostic diagnoseError(Operand *op, Diag<T...> diag, U &&...args) {
+    return diagnoseError(op->getUser()->getLoc(), diag,
+                         std::forward<U>(args)...);
+  }
+
   template <typename... T, typename... U>
   InFlightDiagnostic diagnoseNote(SourceLoc loc, Diag<T...> diag, U &&...args) {
     return getASTContext().Diags.diagnose(loc, diag, std::forward<U>(args)...);
@@ -1559,6 +1707,12 @@ private:
                                   U &&...args) {
     return diagnoseNote(inst->getLoc(), diag, std::forward<U>(args)...);
   }
+
+  template <typename... T, typename... U>
+  InFlightDiagnostic diagnoseNote(Operand *op, Diag<T...> diag, U &&...args) {
+    return diagnoseNote(op->getUser()->getLoc(), diag,
+                        std::forward<U>(args)...);
+  }
 };
 
 class SentNeverSendableDiagnosticInferrer {
@@ -1609,67 +1763,101 @@ private:
 } // namespace
 
 bool SentNeverSendableDiagnosticInferrer::initForSendingPartialApply(
-    FullApplySite fas, Operand *paiOp) {
-  auto *pai =
-      dyn_cast<PartialApplyInst>(stripFunctionConversions(paiOp->get()));
-  if (!pai)
+    FullApplySite fas, Operand *callsiteOp) {
+  // This is the partial apply that is being passed as a sending parameter.
+  auto *sendingPAI =
+      dyn_cast<PartialApplyInst>(stripFunctionConversions(callsiteOp->get()));
+  if (!sendingPAI)
     return false;
 
-  // For now we want this to be really narrow and to only apply to closure
-  // literals.
-  auto *ce = pai->getLoc().getAsASTNode<ClosureExpr>();
-  if (!ce)
+  // Make sure that we only handle closure literals.
+  //
+  // TODO: This should be marked on closures at the SIL level... I shouldn't
+  // have to refer to the AST.
+  if (!sendingPAI->getLoc().getAsASTNode<ClosureExpr>())
     return false;
 
-  // Ok, we now know we have a partial apply and it is a closure literal. Lets
-  // see if we can find the exact thing that caused the closure literal to be
-  // actor isolated.
-  auto isolationInfo = diagnosticEmitter.getIsolationRegionInfo();
-  if (isolationInfo->hasIsolatedValue()) {
-    // Now that we have the value, see if that value is one of our captured
-    // values.
-    auto isolatedValue = isolationInfo->getIsolatedValue();
-    auto matchingElt = getIsolatedValuePartialApplyIndex(pai, isolatedValue);
-    if (matchingElt) {
-      // Ok, we found the matching element. Lets emit our diagnostic!
-      auto capture = ce->getCaptureInfo().getCaptures()[*matchingElt];
-      diagnosticEmitter
-          .emitTypedSendingNeverSendableToSendingClosureParamDirectlyIsolated(
-              ce, capture);
-      return true;
-    }
-  }
-
-  // Ok, we are not tracking an actual isolated value or we do not capture the
-  // isolated value directly... we need to be smarter here. First lets gather up
-  // all non-Sendable values captured by the closure.
-  SmallVector<CapturedValue, 8> nonSendableCaptures;
-  for (auto capture : ce->getCaptureInfo().getCaptures()) {
-    auto *decl = capture.getDecl();
-    auto type = decl->getInterfaceType()->getCanonicalType();
-    auto silType = SILType::getPrimitiveObjectType(type);
-    if (!SILIsolationInfo::isNonSendableType(silType, pai->getFunction()))
+  // Ok, we have a closure literal. First we handle a potential capture of
+  // 'self' before we do anything by looping over our captured parameters.
+  //
+  // DISCUSSION: The reason why we do this early is that as a later heuristic,
+  // we check if any of the values are directly task isolated (i.e. they are
+  // actually the task isolated value, not a value that is in the same region as
+  // something that is task isolated). This could potentially result in us
+  // emitting a task isolated error instead of an actor isolated error here if
+  // for some reason SILGen makes the self capture come later in the capture
+  // list. From a compile time perspective, going over a list of captures twice
+  // is not going to hurt especially since we are going to emit a diagnostic
+  // here anyways.
+  for (auto &sendingPAIOp : sendingPAI->getArgumentOperands()) {
+    // NOTE: If we access a field on self in the closure, we will still just
+    // capture self... so we do not have to handle that case due to the way
+    // SILGen codegens today. This is also true if we use a capture list [x =
+    // self.field] (i.e. a closure that captures a field from self is still
+    // nonisolated).
+    if (!sendingPAIOp.get()->getType().isAnyActor())
       continue;
 
-    auto *fromDC = decl->getInnermostDeclContext();
-    auto *nom = silType.getNominalOrBoundGenericNominal();
-    if (nom && fromDC) {
-      if (auto diagnosticBehavior =
-              getConcurrencyDiagnosticBehaviorLimit(nom, fromDC)) {
-        if (*diagnosticBehavior == DiagnosticBehavior::Ignore)
-          continue;
-      }
+    auto *fArg = dyn_cast<SILFunctionArgument>(
+        lookThroughOwnershipInsts(sendingPAIOp.get()));
+    if (!fArg || !fArg->isSelf())
+      continue;
+
+    auto capturedValue = findClosureUse(&sendingPAIOp);
+    if (!capturedValue) {
+      // If we failed to find the direct capture of self, emit an unknown code
+      // pattern error so the user knows to send a bug report. This should never
+      // fail.
+      diagnosticEmitter.emitUnknownPatternError();
+      return true;
     }
-    nonSendableCaptures.push_back(capture);
+
+    // Otherwise, emit our captured actor error.
+    diagnosticEmitter.emitClosureErrorWithCapturedActor(
+        &sendingPAIOp, capturedValue->first, capturedValue->second);
+    return true;
   }
 
-  // If we do not have any non-Sendable captures... bail.
-  if (nonSendableCaptures.empty())
-    return false;
+  // Ok, we know that we have a closure expr. We now need to find the specific
+  // closure captured value that is actor or task isolated. Then we search for
+  // the potentially recursive closure use so we can show a nice loc to the
+  // user.
+  auto maybeIsolatedValue =
+      diagnosticEmitter.getIsolationRegionInfo()->maybeGetIsolatedValue();
 
-  // Otherwise, emit the diagnostic.
-  diagnosticEmitter.emitTypedSendingNeverSendableToSendingClosureParam(
-      ce, nonSendableCaptures);
+  // If we do not find an actual task isolated value while looping below, this
+  // contains the non sendable captures of the partial apply that we want to
+  // emit a more heuristic based error for. See documentation below.
+  SmallVector<Operand *, 8> nonSendableOps;
+
+  for (auto &sendingPAIOp : sendingPAI->getArgumentOperands()) {
+    // If our value's rep is task isolated or is the dynamic isolated
+    // value... then we are done. This is a 'correct' error value to emit.
+    auto trackableValue = valueMap.getTrackableValue(sendingPAIOp.get());
+    if (trackableValue.isSendable())
+      continue;
+
+    auto rep = trackableValue.getRepresentative().maybeGetValue();
+    nonSendableOps.push_back(&sendingPAIOp);
+
+    if (trackableValue.getIsolationRegionInfo().isTaskIsolated() ||
+        rep == maybeIsolatedValue) {
+      if (auto capturedValue = findClosureUse(&sendingPAIOp)) {
+        diagnosticEmitter.emitSendingClosureParamDirectlyIsolated(
+            callsiteOp, capturedValue->first, capturedValue->second);
+        return true;
+      }
+    }
+  }
+
+  // If we did not find a clear answer in terms of an isolated value, we emit a
+  // more general error based on:
+  //
+  // 1. If we have one non-Sendable value then we know that must be the value.
+  // 2. Otherwise, we emit a generic captured non-Sendable value error to give
+  //    people something to work off of.
+  diagnosticEmitter.emitSendingClosureMultipleCapturedOperandError(
+      callsiteOp->getUser()->getLoc(), nonSendableOps);
   return true;
 }
 

test/Concurrency/sendable_preconcurrency.swift

@@ -32,11 +32,11 @@ struct MyType3 {
 
 func testA(ns: NS, mt: MyType, mt2: MyType2, mt3: MyType3, sc: StrictClass, nsc: NonStrictClass) async {
   Task { // expected-tns-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}}
-    print(ns)
+    print(ns) // expected-tns-note {{closure captures 'ns' which is accessible to code in the current task}}
     print(mt)
     print(mt2)
     print(mt3)
-    print(sc) // expected-tns-note {{closure captures 'sc' which is accessible to code in the current task}}
+    print(sc)
     print(nsc)
   }
 }

test/Concurrency/sendable_without_preconcurrency.swift

@@ -27,10 +27,10 @@ struct MyType2 {
 
 func testA(ns: NS, mt: MyType, mt2: MyType2, sc: StrictClass, nsc: NonStrictClass) async {
   Task { // expected-tns-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}}
-    print(ns) // expected-tns-note {{closure captures non-Sendable 'ns'}}
-    print(mt) // expected-tns-note {{closure captures non-Sendable 'mt'}}
-    print(mt2) // expected-tns-note {{closure captures non-Sendable 'mt2'}}
-    print(sc) // expected-tns-note {{closure captures non-Sendable 'sc'}}
+    print(ns) // expected-tns-note {{closure captures 'ns' which is accessible to code in the current task}}
+    print(mt)
+    print(mt2)
+    print(sc)
   }
 }
 

test/Concurrency/sendable_without_preconcurrency_2.swift

@@ -30,11 +30,30 @@ struct MyType2: Sendable {
 
 func testA(ns: NS, mt: MyType, mt2: MyType2, sc: StrictClass, nsc: NonStrictClass) async {
   Task { // expected-tns-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}}
-    print(ns)
-    print(mt) // no warning with targeted: MyType is Sendable because we suppressed NonStrictClass's warning
+    print(ns) // expected-tns-note {{closure captures 'ns' which is accessible to code in the current task}}
+    print(mt)
     print(mt2)
     print(sc)
-    print(nsc) // expected-tns-note {{closure captures 'nsc' which is accessible to code in the current task}}
+    print(nsc)
+  }
+}
+
+// No warning with targeted: MyType is Sendable because we suppressed NonStrictClass's warning.
+func testB(mt: MyType) async {
+  Task { // expected-tns-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}}
+    print(mt) // expected-tns-note {{closure captures 'mt' which is accessible to code in the current task}}
+  }
+}
+
+func testNonStrictClass(_ mt: NonStrictClass) async {
+  Task { // expected-tns-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}}
+    print(mt) // expected-tns-note {{closure captures 'mt' which is accessible to code in the current task}}
+  }
+}
+
+func testStrictClass(_ mt: StrictClass) async {
+  Task { // expected-tns-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}}
+    print(mt) // expected-tns-note {{closure captures 'mt' which is accessible to code in the current task}}
   }
 }
 

test/Concurrency/transfernonsendable_closureliterals_isolationinference.swift

@@ -475,8 +475,9 @@ extension MyActor {
         //
         // CHECK-LABEL: // closure #2 in MyActor.test_CallerSyncInheritsActorContext_CalleeSyncNonisolated()
         // CHECK-NEXT: // Isolation: actor_instance. name: 'self'
-        inheritActorContextAcceptsSendingClosure { print(self) } // expected-error {{sending value of non-Sendable type '() -> ()' risks causing data races}}
-        // expected-note @-1 {{Passing 'self'-isolated value of non-Sendable type '() -> ()' as a 'sending' parameter to global function 'inheritActorContextAcceptsSendingClosure' risks causing races inbetween 'self'-isolated uses and uses reachable from 'inheritActorContextAcceptsSendingClosure'}}
+        inheritActorContextAcceptsSendingClosure { // expected-error {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}}
+            print(self) // expected-note {{closure captures 'self'}}
+        }
 
         // CHECK-LABEL: // closure #3 in MyActor.test_CallerSyncInheritsActorContext_CalleeSyncNonisolated()
         // CHECK-NEXT: // Isolation: actor_instance. name: 'self'
@@ -512,8 +513,9 @@ extension MyActor {
         //
         // CHECK-LABEL: // closure #2 in MyActor.test_CallerSyncInheritsActorContext_CalleeSyncMainActorIsolated()
         // CHECK-NEXT: // Isolation: actor_instance. name: 'self'
-        await inheritActorContextGlobalActorAcceptsSendingClosure { print(self) } // expected-error {{sending value of non-Sendable type '() -> ()' risks causing data races}}
-        // expected-note @-1 {{Passing 'self'-isolated value of non-Sendable type '() -> ()' as a 'sending' parameter to global function 'inheritActorContextGlobalActorAcceptsSendingClosure' risks causing races inbetween 'self'-isolated uses and uses reachable from 'inheritActorContextGlobalActorAcceptsSendingClosure'}}
+        await inheritActorContextGlobalActorAcceptsSendingClosure { // expected-error {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}}
+            print(self) // expected-note {{closure captures 'self'}}
+        }
 
         // CHECK-LABEL: // closure #3 in MyActor.test_CallerSyncInheritsActorContext_CalleeSyncMainActorIsolated()
         // CHECK-NEXT: // Isolation: actor_instance. name: 'self'
@@ -630,8 +632,9 @@ extension MyActor {
         //
         // CHECK-LABEL: // closure #2 in MyActor.test_CallerAsyncInheritsActorContext_CalleeSyncNonisolated()
         // CHECK-NEXT: // Isolation: actor_instance. name: 'self'
-        await asyncInheritActorContextAcceptsSendingClosure { print(self) } // expected-error {{sending value of non-Sendable type '() -> ()' risks causing data races}}
-        // expected-note @-1 {{Passing 'self'-isolated value of non-Sendable type '() -> ()' as a 'sending' parameter to global function 'asyncInheritActorContextAcceptsSendingClosure' risks causing races inbetween 'self'-isolated uses and uses reachable from 'asyncInheritActorContextAcceptsSendingClosure'}}
+        await asyncInheritActorContextAcceptsSendingClosure { // expected-error {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}}
+            print(self) // expected-note {{closure captures 'self'}}
+        }
 
         // CHECK-LABEL: // closure #3 in MyActor.test_CallerAsyncInheritsActorContext_CalleeSyncNonisolated()
         // CHECK-NEXT: // Isolation: actor_instance. name: 'self'
@@ -667,8 +670,9 @@ extension MyActor {
         //
         // CHECK-LABEL: // closure #2 in MyActor.test_CallerAsyncInheritsActorContext_CalleeSyncMainActorIsolated()
         // CHECK-NEXT: // Isolation: actor_instance. name: 'self'
-        await asyncInheritActorContextGlobalActorAcceptsSendingClosure { print(self) } // expected-error {{sending value of non-Sendable type '() -> ()' risks causing data races}}
-        // expected-note @-1 {{Passing 'self'-isolated value of non-Sendable type '() -> ()' as a 'sending' parameter to global function 'asyncInheritActorContextGlobalActorAcceptsSendingClosure' risks causing races inbetween 'self'-isolated uses and uses reachable from 'asyncInheritActorContextGlobalActorAcceptsSendingClosure'}}
+        await asyncInheritActorContextGlobalActorAcceptsSendingClosure { // expected-error {{passing closure as a 'sending' parameter risks causing data races between 'self'-isolated code and concurrent execution of the closure}}
+            print(self) // expected-note {{closure captures 'self'}}
+        }
 
         // CHECK-LABEL: // closure #3 in MyActor.test_CallerAsyncInheritsActorContext_CalleeSyncMainActorIsolated()
         // CHECK-NEXT: // Isolation: actor_instance. name: 'self'

test/Concurrency/transfernonsendable_sending_params.swift

@@ -518,6 +518,12 @@ func taskIsolatedCaptureInSendingClosureLiteral(_ x: NonSendableKlass) {
     print(x) // expected-note {{closure captures 'x' which is accessible to code in the current task}}
   }
 
+  Task { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}}
+    {
+      print(x) // expected-note {{closure captures 'x' which is accessible to code in the current task}}
+    }()
+  }
+
   takeClosure { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}}
     print(x) // expected-note {{closure captures 'x' which is accessible to code in the current task}}
   }
@@ -617,3 +623,32 @@ func disconnectedPassedSendingToNonIsolatedCalleeIsolatedParam3(
     // expected-note @-1 {{'c' used after being passed as a 'sending' parameter}}
     c.use() // expected-note {{access can happen concurrently}}
 }
+
+// In all of the below, we don't know that 'a' is the same isolation as the
+// closure isolation.
+func testNonSendableCaptures(ns: NonSendableKlass, a: isolated MyActor) {
+  Task {
+    _ = a
+    _ = ns
+  }
+
+  Task { [a] in // expected-warning {{passing closure as a 'sending' parameter risks causing data races between 'a'-isolated code and concurrent execution of the closure}}
+    _ = a
+    _ = ns // expected-note {{closure captures 'a'-isolated 'ns'}}
+  }
+
+  Task {
+    let _ = a
+    let _ = ns
+  }
+
+  Task { [a] in // expected-warning {{passing closure as a 'sending' parameter risks causing data races between 'a'-isolated code and concurrent execution of the closure}}
+    let _ = a
+    let _ = ns // expected-note {{closure captures 'a'-isolated 'ns'}}
+  }
+
+  Task { [a] in // expected-warning {{passing closure as a 'sending' parameter risks causing data races between 'a'-isolated code and concurrent execution of the closure}}
+    let (_, _) = (a, ns) // expected-note {{closure captures 'a'-isolated 'ns'}}
+    let _ = ns
+  }
+}