[region-isolation] Emit a better error when a function parameter is assigned into a different transferring parameter.

Just eliminating another "call site passes `self`" error.

Author: gottesmm

include/swift/AST/DiagnosticsSIL.def

@@ -973,6 +973,13 @@ NOTE(regionbasedisolation_isolated_since_in_same_region_basename, none,
 ERROR(regionbasedisolation_named_transfer_yields_race, none,
       "transferring %0 may cause a race",
       (Identifier))
+ERROR(regionbasedisolation_stronglytransfer_assignment_yields_race_name, none,
+      "assigning %0 to transferring parameter %1 may cause a race",
+      (Identifier, Identifier))
+NOTE(regionbasedisolation_stronglytransfer_taskisolated_assign_note, none,
+      "%0 is a task isolated value that is assigned into transferring parameter %1. Transferred uses of %1 may race with caller uses of %0",
+      (Identifier, Identifier))
+
 NOTE(regionbasedisolation_named_info_transfer_yields_race, none,
      "%0 is transferred from %1 caller to %2 callee. Later uses in caller could race with potential uses in callee",
      (Identifier, ActorIsolation, ActorIsolation))

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

@@ -995,12 +995,17 @@ class TransferNonTransferrableDiagnosticInferrer {
 
     /// Used if we have a named variable.
     NamedIsolation = 5,
+
+    /// Used if a non-transferring function argument is assigned to a
+    /// transferring function argument.
+    FunctionArgumentAssignedToStronglyTransferredParam = 6,
   };
 
   class UseDiagnosticInfo {
     UseDiagnosticInfoKind kind;
     std::optional<ApplyIsolationCrossing> transferredIsolationCrossing = {};
     llvm::PointerUnion<Type, Identifier> inferredTypeOrIdentifier = {};
+    std::optional<Identifier> transferredParamIdentifier = {};
 
   public:
     UseDiagnosticInfoKind getKind() const { return kind; }
@@ -1010,6 +1015,9 @@ class TransferNonTransferrableDiagnosticInferrer {
     Identifier getName() const {
       return inferredTypeOrIdentifier.get<Identifier>();
     }
+    Identifier getTransferredParamIdentifier() const {
+      return transferredParamIdentifier.value();
+    }
     Type getType() const { return inferredTypeOrIdentifier.get<Type>(); }
 
     static UseDiagnosticInfo forMiscUse() {
@@ -1038,24 +1046,37 @@ class TransferNonTransferrableDiagnosticInferrer {
       return {UseDiagnosticInfoKind::FunctionArgumentApplyStronglyTransferred};
     }
 
+    static UseDiagnosticInfo
+    forFunctionArgumentAssignedToStronglyTransferredParam(
+        Identifier funcArgName, Identifier transferredParam) {
+      return {UseDiagnosticInfoKind::
+                  FunctionArgumentAssignedToStronglyTransferredParam,
+              {},
+              funcArgName,
+              transferredParam};
+    }
+
   private:
     UseDiagnosticInfo(
         UseDiagnosticInfoKind kind,
         std::optional<ApplyIsolationCrossing> isolation = {},
-        llvm::PointerUnion<Type, Identifier> inferredTypeOrIdentifier = {})
+        llvm::PointerUnion<Type, Identifier> inferredTypeOrIdentifier = {},
+        std::optional<Identifier> transferredParamIdentifier = {})
         : kind(kind), transferredIsolationCrossing(isolation),
-          inferredTypeOrIdentifier(inferredTypeOrIdentifier) {}
+          inferredTypeOrIdentifier(inferredTypeOrIdentifier),
+          transferredParamIdentifier(transferredParamIdentifier) {}
   };
 
 private:
+  RegionAnalysisValueMap &valueMap;
   TransferredNonTransferrableInfo info;
   std::optional<UseDiagnosticInfo> diagnosticInfo;
   SourceLoc loc;
 
 public:
   TransferNonTransferrableDiagnosticInferrer(
-      TransferredNonTransferrableInfo info)
-      : info(info),
+      RegionAnalysisValueMap &valueMap, TransferredNonTransferrableInfo info)
+      : valueMap(valueMap), info(info),
         loc(info.transferredOperand->getUser()->getLoc().getSourceLoc()) {}
 
   /// Gathers diagnostics. Returns false if we emitted a "I don't understand
@@ -1099,6 +1120,28 @@ bool TransferNonTransferrableDiagnosticInferrer::run() {
   auto *op = info.transferredOperand;
   auto loc = info.transferredOperand->getUser()->getLoc();
 
+  // Before we do anything, see if the transfer instruction was into a
+  // transferring parameter alloc_stack. In such a case, we emit a special
+  // message.
+  if (auto destValue = getDestOfStoreOrCopyAddr(op)) {
+    auto trackedValue = valueMap.getTrackableValue(destValue);
+    if (trackedValue.isTransferringParameter()) {
+      auto valueName = inferNameFromValue(op->get());
+      auto paramName =
+          inferNameFromValue(trackedValue.getRepresentative().getValue());
+      if (!valueName || !paramName) {
+        diagnoseError(op->getUser(),
+                      diag::regionbasedisolation_unknown_pattern);
+        return false;
+      }
+
+      diagnosticInfo = UseDiagnosticInfo::
+          forFunctionArgumentAssignedToStronglyTransferredParam(*valueName,
+                                                                *paramName);
+      return true;
+    }
+  }
+
   if (auto *sourceApply = loc.getAsASTNode<ApplyExpr>()) {
     std::optional<ApplyIsolationCrossing> isolation = {};
 
@@ -1182,7 +1225,8 @@ void TransferNonSendableImpl::emitTransferredNonTransferrableDiagnostics() {
   auto &astContext = regionInfo->getFunction()->getASTContext();
   for (auto info : transferredNonTransferrable) {
     auto *op = info.transferredOperand;
-    TransferNonTransferrableDiagnosticInferrer diagnosticInferrer(info);
+    TransferNonTransferrableDiagnosticInferrer diagnosticInferrer(
+        regionInfo->getValueMap(), info);
     if (!diagnosticInferrer.run())
       continue;
 
@@ -1254,6 +1298,20 @@ void TransferNonSendableImpl::emitTransferredNonTransferrableDiagnostics() {
           diagnosticInfo.getIsolationCrossing().getCallerIsolation(),
           diagnosticInfo.getIsolationCrossing().getCalleeIsolation());
       break;
+    case UseDiagnosticInfoKind::
+        FunctionArgumentAssignedToStronglyTransferredParam:
+      diagnoseError(
+          astContext, loc,
+          diag::
+              regionbasedisolation_stronglytransfer_assignment_yields_race_name,
+          diagnosticInfo.getName(),
+          diagnosticInfo.getTransferredParamIdentifier());
+      diagnoseNote(
+          astContext, loc,
+          diag::regionbasedisolation_stronglytransfer_taskisolated_assign_note,
+          diagnosticInfo.getName(),
+          diagnosticInfo.getTransferredParamIdentifier());
+      break;
     }
   }
 }

test/Concurrency/transfernonsendable_strong_transferring_params.swift

@@ -13,6 +13,26 @@ struct NonSendableStruct {
   var second = Klass()
 }
 
+class KlassWithNonSendableStructPair {
+  var ns1: NonSendableStruct
+  var ns2: (NonSendableStruct, NonSendableStruct)
+
+  init() {
+    ns1 = NonSendableStruct()
+    ns2 = (ns1, ns1)
+  }
+}
+
+final class FinalKlassWithNonSendableStructPair {
+  var ns1: NonSendableStruct
+  var ns2: (NonSendableStruct, NonSendableStruct)
+
+  init() {
+    ns1 = NonSendableStruct()
+    ns2 = (ns1, ns1)
+  }
+}
+
 func useValue<T>(_ t: T) {}
 func getAny() -> Any { fatalError() }
 
@@ -299,3 +319,30 @@ func testTransferSrc(_ x: transferring Klass) async {
   await transferToMain(y) // expected-warning {{transferring value of non-Sendable type 'Klass' from nonisolated context to main actor-isolated context}}
   x = y // expected-note {{access here could race}}
 }
+
+func testTransferOtherParam(_ x: transferring Klass, y: Klass) async {
+  x = y // expected-warning {{assigning 'y' to transferring parameter 'x' may cause a race}}
+  // expected-note @-1 {{'y' is a task isolated value that is assigned into transferring parameter 'x'. Transferred uses of 'x' may race with caller uses of 'y'}}
+}
+
+func testTransferOtherParamTuple(_ x: transferring Klass, y: (Klass, Klass)) async {
+  x = y.0 // expected-warning {{assigning 'y.0' to transferring parameter 'x' may cause a race}}
+  // expected-note @-1 {{'y.0' is a task isolated value that is assigned into transferring parameter 'x'. Transferred uses of 'x' may race with caller uses of 'y.0'}}
+}
+
+func testTransferOtherParamStruct(_ x: transferring Klass, y: NonSendableStruct) async {
+  x = y.first // expected-warning {{assigning 'y.first' to transferring parameter 'x' may cause a race}}
+  // expected-note @-1 {{'y.first' is a task isolated value that is assigned into transferring parameter 'x'. Transferred uses of 'x' may race with caller uses of 'y.first'}}
+}
+
+func testTransferOtherParamTupleStruct(_ x: transferring Klass, y: (NonSendableStruct, NonSendableStruct)) async {
+  x = y.1.second // expected-warning {{assigning 'y.1.second' to transferring parameter 'x' may cause a race}}
+  // expected-note @-1 {{'y.1.second' is a task isolated value that is assigned into transferring parameter 'x'. Transferred uses of 'x' may race with caller uses of 'y.1.second'}}
+}
+
+func testTransferOtherParamClassStructTuple(_ x: transferring Klass, y: KlassWithNonSendableStructPair) async {
+  x = y.ns1.second // expected-warning {{assigning 'y.ns1.second' to transferring parameter 'x' may cause a race}}
+  // expected-note @-1 {{'y.ns1.second' is a task isolated value that is assigned into transferring parameter 'x'}}
+  x = y.ns2.1.second // expected-warning {{assigning 'y.ns2.1.second' to transferring parameter 'x' may cause a race}}
+  // expected-note @-1 {{'y.ns2.1.second' is a task isolated value that is assigned into transferring parameter 'x'. Transferred uses of 'x' may race with caller uses of 'y.ns2.1.second'}}
+}

test/Concurrency/transfernonsendable_warning_until_swift6.swift

@@ -39,7 +39,8 @@ func testAssignmentIntoTransferringParameter(_ x: transferring NonSendableType)
 }
 
 func testAssigningParameterIntoTransferringParameter(_ x: transferring NonSendableType, _ y: NonSendableType) async {
-  x = y // expected-error {{call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller}}
+  x = y // expected-error {{assigning 'y' to transferring parameter 'x' may cause a race}}
+  // expected-note @-1 {{'y' is a task isolated value that is assigned into transferring parameter 'x'. Transferred uses of 'x' may race with caller uses of 'y'}}
 }
 
 func testIsolationCrossingDueToCapture() async {