[region-isolation] Assigning into a transferring parameter should be a store, not a transfer.

rdar://123865943

Author: gottesmm

include/swift/AST/DiagnosticsSIL.def

@@ -950,9 +950,6 @@ ERROR(regionbasedisolation_transfer_yields_race_with_isolation, none,
 ERROR(regionbasedisolation_isolated_capture_yields_race, none,
       "%1 closure captures value of non-Sendable type %0 from %2 context; later accesses to value could race",
       (Type, ActorIsolation, ActorIsolation))
-ERROR(regionbasedisolation_transfer_yields_race_transferring_parameter, none,
-      "transferring value of non-Sendable type %0 into transferring parameter; later accesses could race",
-      (Type))
 ERROR(regionbasedisolation_transfer_yields_race_stronglytransferred_binding, none,
       "binding of non-Sendable type %0 accessed after being transferred; later accesses could race",
       (Type))

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -1986,16 +1986,6 @@ class PartitionOpTranslator {
     SILValue srcValue = src->get();
 
     if (auto nonSendableDest = tryToTrackValue(destValue)) {
-      // Before we do anything check if we have an assignment into an
-      // alloc_stack for a consuming transferring parameter... in such a case,
-      // we need to handle this specially.
-      if (nonSendableDest->isTransferringParameter()) {
-        if (auto nonSendableSrc = tryToTrackValue(srcValue)) {
-          return translateSILAssignmentToTransferringParameter(
-              *nonSendableDest, dest, *nonSendableSrc, src);
-        }
-      }
-
       // In the following situations, we can perform an assign:
       //
       // 1. A store to unaliased storage.

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

@@ -442,10 +442,6 @@ class UseAfterTransferDiagnosticInferrer {
     /// error though.
     TypedRaceWithoutKnownIsolationCrossing = 2,
 
-    /// Used if the error is due to a transfer into an assignment into a
-    /// transferring parameter.
-    AssignmentIntoTransferringParameter = 3,
-
     /// Set to true if this is a use of a normal value that was strongly
     /// transferred.
     UseOfStronglyTransferredValue = 4,
@@ -501,14 +497,6 @@ class UseAfterTransferDiagnosticInferrer {
           {}, inferredType, SILLocation::invalid());
     }
 
-    static UseDiagnosticInfo
-    forTypedAssignmentIntoTransferringParameter(SILLocation loc,
-                                                Type inferredType) {
-      return UseDiagnosticInfo(
-          UseDiagnosticInfoKind::AssignmentIntoTransferringParameter, loc, {},
-          inferredType, SILLocation::invalid());
-    }
-
     static UseDiagnosticInfo
     forTypedUseOfStronglyTransferredValue(SILLocation loc, Type inferredType) {
       return UseDiagnosticInfo(
@@ -578,12 +566,6 @@ class UseAfterTransferDiagnosticInferrer {
   void initForApply(const Operand *op, ApplyExpr *expr);
   void initForAutoclosure(const Operand *op, AutoClosureExpr *expr);
 
-  void initForAssignmentToTransferringParameter(const Operand *op) {
-    appendUseInfo(
-        UseDiagnosticInfo::forTypedAssignmentIntoTransferringParameter(
-            baseLoc, op->get()->getType().getASTType()));
-  }
-
   void initForUseOfStronglyTransferredValue(const Operand *op) {
     appendUseInfo(UseDiagnosticInfo::forTypedUseOfStronglyTransferredValue(
         baseLoc, op->get()->getType().getASTType()));
@@ -726,31 +708,11 @@ struct UseAfterTransferDiagnosticInferrer::Walker : ASTWalker {
   }
 };
 
-static SILValue getDestOfStoreOrCopyAddr(Operand *op) {
-  if (auto *si = dyn_cast<StoreInst>(op->getUser()))
-    return si->getDest();
-
-  if (auto *copyAddr = dyn_cast<CopyAddrInst>(op->getUser()))
-    return copyAddr->getDest();
-
-  return SILValue();
-}
-
 void UseAfterTransferDiagnosticInferrer::init(const Operand *op) {
   baseLoc = op->getUser()->getLoc();
   baseInferredType = op->get()->getType().getASTType();
   auto *nonConstOp = const_cast<Operand *>(op);
 
-  // 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(nonConstOp)) {
-    auto trackedValue = valueMap.getTrackableValue(destValue);
-    if (trackedValue.isTransferringParameter()) {
-      return initForAssignmentToTransferringParameter(op);
-    }
-  }
-
   // Otherwise, see if our operand's instruction is a transferring parameter.
   if (auto fas = FullApplySite::isa(nonConstOp->getUser())) {
     assert(!fas.getArgumentConvention(*nonConstOp).isIndirectOutParameter() &&
@@ -831,8 +793,8 @@ void TransferNonSendableImpl::emitUseAfterTransferDiagnostics() {
   if (transferOpToRequireInstMultiMap.empty())
     return;
 
-  LLVM_DEBUG(llvm::dbgs()
-             << "Visiting found transfer+[requireInsts] for diagnostics.\n");
+  LLVM_DEBUG(llvm::dbgs() << "Emitting use after transfer diagnostics.\n");
+
   auto &astContext = regionInfo->getFunction()->getASTContext();
   for (auto [transferOp, requireInsts] :
        transferOpToRequireInstMultiMap.getRange()) {
@@ -940,14 +902,6 @@ void TransferNonSendableImpl::emitUseAfterTransferDiagnostics() {
             info.getInferredType())
             .highlight(info.getLoc().getSourceRange());
         break;
-      case UseDiagnosticInfoKind::AssignmentIntoTransferringParameter:
-        diagnoseError(
-            astContext, info.getLoc(),
-            diag::
-                regionbasedisolation_transfer_yields_race_transferring_parameter,
-            info.getInferredType())
-            .highlight(info.getLoc().getSourceRange());
-        break;
       case UseDiagnosticInfoKind::TypedIsolationCrossingDueToCapture:
         auto isolation = info.getIsolationCrossing();
         diagnoseError(astContext, info.getLoc(),
@@ -995,10 +949,6 @@ 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 {
@@ -1047,16 +997,6 @@ class TransferNonTransferrableDiagnosticInferrer {
         inferredType};
     }
 
-    static UseDiagnosticInfo
-    forFunctionArgumentAssignedToStronglyTransferredParam(
-        Identifier funcArgName, Identifier transferredParam) {
-      return {UseDiagnosticInfoKind::
-                  FunctionArgumentAssignedToStronglyTransferredParam,
-              {},
-              funcArgName,
-              transferredParam};
-    }
-
   private:
     UseDiagnosticInfo(
         UseDiagnosticInfoKind kind,
@@ -1069,15 +1009,14 @@ class TransferNonTransferrableDiagnosticInferrer {
   };
 
 private:
-  RegionAnalysisValueMap &valueMap;
   TransferredNonTransferrableInfo info;
   std::optional<UseDiagnosticInfo> diagnosticInfo;
   SourceLoc loc;
 
 public:
   TransferNonTransferrableDiagnosticInferrer(
-      RegionAnalysisValueMap &valueMap, TransferredNonTransferrableInfo info)
-      : valueMap(valueMap), info(info),
+      TransferredNonTransferrableInfo info)
+      : info(info),
         loc(info.transferredOperand->getUser()->getLoc().getSourceLoc()) {}
 
   /// Gathers diagnostics. Returns false if we emitted a "I don't understand
@@ -1122,28 +1061,6 @@ 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 = {};
 
@@ -1233,8 +1150,7 @@ void TransferNonSendableImpl::emitTransferredNonTransferrableDiagnostics() {
   auto &astContext = regionInfo->getFunction()->getASTContext();
   for (auto info : transferredNonTransferrable) {
     auto *op = info.transferredOperand;
-    TransferNonTransferrableDiagnosticInferrer diagnosticInferrer(
-        regionInfo->getValueMap(), info);
+    TransferNonTransferrableDiagnosticInferrer diagnosticInferrer(info);
     if (!diagnosticInferrer.run())
       continue;
 
@@ -1306,20 +1222,6 @@ 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

@@ -195,47 +195,58 @@ func canTransferAssigningIntoLocal2(_ x: transferring Klass) async {
 // MARK: Transferring is "var" like //
 //////////////////////////////////////
 
-// Assigning into a transferring parameter is a transfer!
-func assigningIsATransfer(_ x: transferring Klass) async {
-  // Ok, this is disconnected.
+// Assigning into a transferring parameter is a merge.
+func assigningIsAMerge(_ x: transferring Klass) async {
   let y = Klass()
 
-  // y is transferred into x.
-  x = y // expected-warning {{transferring value of non-Sendable type 'Klass' into transferring parameter; later accesses could race}}
+  x = y
 
-  // Since y is now in x, we can't use y anymore so we emit an error.
-  useValue(y) // expected-note {{access here could race}}
+  // We can still transfer y since x is disconnected.
+  await transferToMain(y)
 }
 
-func assigningIsATransferNoError(_ x: transferring Klass) async {
-  // Ok, this is disconnected.
+func assigningIsAMergeError(_ x: transferring Klass) async {
   let y = Klass()
 
-  // y is transferred into x.
   x = y
 
-  useValue(x)
+  // We can still transfer y since x is disconnected.
+  await transferToMain(y) // expected-warning {{transferring value of non-Sendable type 'Klass' from nonisolated context to main actor-isolated context}}
+
+  useValue(x) // expected-note {{access here could race}}
 }
 
-func assigningIsATransferAny(_ x: transferring Any) async {
+func assigningIsAMergeAny(_ x: transferring Any) async {
   // Ok, this is disconnected.
   let y = getAny()
 
-  // y is transferred into x.
-  x = y // expected-warning {{transferring value of non-Sendable type 'Any' into transferring parameter; later accesses could race}}
+  x = y
+
+  await transferToMain(y)
+}
+
+func assigningIsAMergeAnyError(_ x: transferring Any) async {
+  // Ok, this is disconnected.
+  let y = getAny() // expected-note {{variable defined here}}
 
-  // Since y is now in x, we can't use y anymore so we emit an error.
-  useValue(y) // expected-note {{access here could race}}
+  x = y
+
+  await transferToMain(y) // expected-warning {{transferring 'y' may cause a race}}
+  // expected-note @-1 {{'y' is transferred from nonisolated caller to main actor-isolated callee}}
+
+  useValue(x) // expected-note {{access here could race}}
 }
 
 func canTransferAfterAssign(_ x: transferring Any) async {
   // Ok, this is disconnected.
   let y = getAny()
 
   // y is transferred into x.
+  await transferToMain(x)
+
   x = y
 
-  await transferToMain(x)
+  useValue(x)
 }
 
 func canTransferAfterAssignButUseIsError(_ x: transferring Any) async {
@@ -281,10 +292,10 @@ func mergeDoesNotEliminateEarlierTransfer(_ x: transferring NonSendableStruct) a
   await transferToMain(x) // expected-warning {{transferring 'x' may cause a race}}
   // expected-note @-1 {{'x' is transferred from nonisolated caller to main actor-isolated callee. Later uses in caller could race with potential uses in callee}}
 
-  // y is assigned into a field of x, so we treat this like a merge.
-  x.first = y
+  // y is assigned into a field of x.
+  x.first = y // expected-note {{access here could race}}
 
-  useValue(x) // expected-note {{access here could race}}
+  useValue(x)
 }
 
 func mergeDoesNotEliminateEarlierTransfer2(_ x: transferring NonSendableStruct) async {
@@ -299,12 +310,7 @@ func mergeDoesNotEliminateEarlierTransfer2(_ x: transferring NonSendableStruct)
   await transferToMain(x) // expected-warning {{transferring 'x' may cause a race}}
   // expected-note @-1 {{'x' is transferred from nonisolated caller to main actor-isolated callee. Later uses in caller could race with potential uses in callee}}
 
-  // y is assigned into a field of x, so we treat this like a merge.
-  x.first = y // expected-warning {{transferring value of non-Sendable type 'Klass' into transferring parameter; later accesses could race}}
-
-  useValue(x) // expected-note {{access here could race}}
-
-  useValue(y) // expected-note {{access here could race}}
+  x.first = y  // expected-note {{access here could race}}
 }
 
 func doubleArgument() async {
@@ -320,34 +326,25 @@ func testTransferSrc(_ x: transferring Klass) async {
 }
 
 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'}}
+  x = 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'}}
+  x = y.0
 }
 
 func useSugaredTypeNameWhenEmittingTaskIsolationError(_ x: @escaping @MainActor () async -> ()) {
   func fakeInit(operation: transferring @escaping () async -> ()) {}
 
   fakeInit(operation: x) // expected-warning {{task isolated value of type '@MainActor () async -> ()' passed as a strongly transferred parameter}}
 }
+
+// Make sure we error here on only the second since x by being assigned a part
+// of y becomes task isolated
+func testMergeWithTaskIsolated(_ x: transferring Klass, y: Klass) async {
+  await transferToMain(x)
+  x = y
+  // TODO: We need to say that this is task isolated.
+  await transferToMain(x) // expected-warning {{transferring 'x' may cause a race}}
+  // expected-note @-1 {{transferring nonisolated 'x' to main actor-isolated callee could cause races between main actor-isolated and nonisolated uses}}
+}

test/Concurrency/transfernonsendable_warning_until_swift6.swift

@@ -34,13 +34,13 @@ func testPassArgumentAsTransferringParameter(_ x: NonSendableType) async {
 
 func testAssignmentIntoTransferringParameter(_ x: transferring NonSendableType) async {
   let y = NonSendableType()
-  x = y // expected-error {{transferring value of non-Sendable type 'NonSendableType' into transferring parameter; later accesses could race}}
-  useValue(y) // expected-note {{access here could race}}
+  await transferToMain(x)
+  x = y
+  useValue(y)
 }
 
 func testAssigningParameterIntoTransferringParameter(_ x: transferring NonSendableType, _ y: NonSendableType) async {
-  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'}}
+  x = y
 }
 
 func testIsolationCrossingDueToCapture() async {