Merge pull request #83102 from gottesmm/release/6.2-revert

[6.2] Revert nonisolated(unsafe) closure inference

Author: gottesmm

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -2490,8 +2490,7 @@ class PartitionOpTranslator {
       }
     }
 
-    if (auto isolationRegionInfo = SILIsolationInfo::get(pai);
-        isolationRegionInfo && !isolationRegionInfo.isDisconnected()) {
+    if (auto isolationRegionInfo = SILIsolationInfo::get(pai)) {
       return translateIsolatedPartialApply(pai, isolationRegionInfo);
     }
 

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

@@ -363,69 +363,6 @@ inferIsolationInfoForTempAllocStack(AllocStackInst *asi) {
   return SILIsolationInfo::get(targetOperand->getUser());
 }
 
-static SILValue lookThroughNonVarDeclOwnershipInsts(SILValue v) {
-  while (true) {
-    if (auto *svi = dyn_cast<SingleValueInstruction>(v)) {
-      if (isa<CopyValueInst>(svi)) {
-        v = svi->getOperand(0);
-        continue;
-      }
-
-      if (auto *bbi = dyn_cast<BeginBorrowInst>(v)) {
-        if (!bbi->isFromVarDecl()) {
-          v = bbi->getOperand();
-          continue;
-        }
-        return v;
-      }
-
-      if (auto *mvi = dyn_cast<MoveValueInst>(v)) {
-        if (!mvi->isFromVarDecl()) {
-          v = mvi->getOperand();
-          continue;
-        }
-
-        return v;
-      }
-    }
-
-    return v;
-  }
-}
-
-/// See if \p pai has at least one nonisolated(unsafe) capture and that all
-/// captures are either nonisolated(unsafe) or sendable.
-static bool isPartialApplyNonisolatedUnsafe(PartialApplyInst *pai) {
-  bool foundOneNonIsolatedUnsafe = false;
-  for (auto &op : pai->getArgumentOperands()) {
-    if (SILIsolationInfo::isSendableType(op.get()))
-      continue;
-
-    // Normally we would not look through copy_value, begin_borrow, or
-    // move_value since this is meant to find the inherent isolation of a
-    // specific element. But since we are checking for captures rather
-    // than the actual value itself (just for unsafe nonisolated
-    // purposes), it is ok.
-    //
-    // E.x.: As an example of something we want to prevent, consider an
-    // invocation of a nonisolated function that is a parameter to an
-    // @MainActor function. That means from a region isolation
-    // perspective, the function parameter is in the MainActor region, but
-    // the actual function itself is not MainActor isolated, since the
-    // function will not hop onto the main actor.
-    //
-    // TODO: We should use some of the shared infrastructure to find the
-    // underlying value of op.get(). This is conservatively correct for
-    // now.
-    auto value = lookThroughNonVarDeclOwnershipInsts(op.get());
-    if (!SILIsolationInfo::get(value).isUnsafeNonIsolated())
-      return false;
-    foundOneNonIsolatedUnsafe = true;
-  }
-
-  return foundOneNonIsolatedUnsafe;
-}
-
 SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
   if (auto fas = FullApplySite::isa(inst)) {
     // Before we do anything, see if we have a sending result. In such a case,
@@ -520,23 +457,12 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
   }
 
   if (auto *pai = dyn_cast<PartialApplyInst>(inst)) {
-    // Check if we have any captures and if the isolation info for all captures
-    // are nonisolated(unsafe) or Sendable. In such a case, we consider the
-    // partial_apply to be nonisolated(unsafe). We purposely do not do this if
-    // the partial_apply does not have any parameters just out of paranoia... we
-    // shouldn't have such a partial_apply emitted by SILGen (it should use a
-    // thin to thick function or something like that)... but in that case since
-    // we do not have any nonisolated(unsafe), it doesn't make sense to
-    // propagate nonisolated(unsafe).
-    bool partialApplyIsNonIsolatedUnsafe = isPartialApplyNonisolatedUnsafe(pai);
-
     if (auto *ace = pai->getLoc().getAsASTNode<AbstractClosureExpr>()) {
       auto actorIsolation = ace->getActorIsolation();
 
       if (actorIsolation.isGlobalActor()) {
         return SILIsolationInfo::getGlobalActorIsolated(
-                   pai, actorIsolation.getGlobalActor())
-            .withUnsafeNonIsolated(partialApplyIsNonIsolatedUnsafe);
+            pai, actorIsolation.getGlobalActor());
       }
 
       if (actorIsolation.isActorInstanceIsolated()) {
@@ -559,16 +485,13 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
             if (auto *fArg = dyn_cast<SILFunctionArgument>(
                     actualIsolatedValue.getValue())) {
               if (auto info =
-                      SILIsolationInfo::getActorInstanceIsolated(pai, fArg)
-                          .withUnsafeNonIsolated(
-                              partialApplyIsNonIsolatedUnsafe))
+                      SILIsolationInfo::getActorInstanceIsolated(pai, fArg))
                 return info;
             }
           }
 
           return SILIsolationInfo::getActorInstanceIsolated(
-                     pai, actorInstance, actorIsolation.getActor())
-              .withUnsafeNonIsolated(partialApplyIsNonIsolatedUnsafe);
+              pai, actorInstance, actorIsolation.getActor());
         }
 
         // For now, if we do not have an actor instance, just create an actor
@@ -582,16 +505,12 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
         //
         // TODO: How do we want to resolve this.
         return SILIsolationInfo::getPartialApplyActorInstanceIsolated(
-                   pai, actorIsolation.getActor())
-            .withUnsafeNonIsolated(partialApplyIsNonIsolatedUnsafe);
+            pai, actorIsolation.getActor());
       }
 
       assert(actorIsolation.getKind() != ActorIsolation::Erased &&
              "Implement this!");
     }
-
-    if (partialApplyIsNonIsolatedUnsafe)
-      return SILIsolationInfo::getDisconnected(partialApplyIsNonIsolatedUnsafe);
   }
 
   // See if the memory base is a ref_element_addr from an address. If so, add

test/Concurrency/transfernonsendable_nonisolatedunsafe.swift

@@ -12,7 +12,7 @@
 // MARK: Declarations //
 ////////////////////////
 
-class NonSendableKlass {
+class NonSendableKlass { // expected-complete-note 98{{}}
   var field: NonSendableKlass? = nil
 }
 
@@ -823,223 +823,3 @@ actor ActorContainingSendableStruct {
 }
 
 
-////////////////////
-// MARK: Closures //
-////////////////////
-
-func closureTests() async {
-  func sendingClosure(_ x: sending () -> ()) {
-  }
-
-  func testLetOneNSVariableError() async {
-    let x = NonSendableKlass()
-    sendingClosure { _ = x } // expected-warning {{sending value of non-Sendable type '() -> ()' risks causing data races}}
-    // expected-note @-1 {{Passing value of non-Sendable type '() -> ()' as a 'sending' argument to local function 'sendingClosure' risks causing races in between local and caller code}}
-    sendingClosure { _ = x } // expected-note {{access can happen concurrently}}
-  }
-
-  func testLetNonIsolatedUnsafeNSVariableNoError() async {
-    nonisolated(unsafe) let x = NonSendableKlass()
-    sendingClosure { _ = x }
-    sendingClosure { _ = x }
-  }
-
-  func testLetOneNSVariableSVariableError() async {
-    let x = NonSendableKlass()
-    let y = CustomActorInstance()
-    sendingClosure { // expected-warning {{sending value of non-Sendable type '() -> ()' risks causing data races}}
-      // expected-note @-1 {{Passing value of non-Sendable type '() -> ()' as a 'sending' argument to local function 'sendingClosure' risks causing races in between local and caller code}}
-      _ = x
-      _ = y
-    }
-    sendingClosure { // expected-note {{access can happen concurrently}}
-      _ = x
-      _ = y
-    }
-  }
-
-  func testLetNonIsolatedUnsafeNSSVariableNoError() async {
-    nonisolated(unsafe) let x = NonSendableKlass()
-    let y = CustomActorInstance()
-    sendingClosure {
-      _ = x
-      _ = y
-    }
-    sendingClosure {
-      _ = x
-      _ = y
-    }
-  }
-
-  func testLetTwoNSVariableError() async {
-    let x = NonSendableKlass()
-    let y = NonSendableKlass()
-    sendingClosure { // expected-warning {{sending value of non-Sendable type '() -> ()' risks causing data races}}
-      // expected-note @-1 {{Passing value of non-Sendable type '() -> ()' as a 'sending' argument to local function 'sendingClosure' risks causing races in between local and caller code}}
-      _ = x
-      _ = y
-    }
-    sendingClosure { // expected-note {{access can happen concurrently}}
-      _ = x
-      _ = y
-    }
-  }
-
-  func testLetTwoNSVariableError2() async {
-    nonisolated(unsafe) let x = NonSendableKlass()
-    let y = NonSendableKlass()
-    sendingClosure { // expected-warning {{sending value of non-Sendable type '() -> ()' risks causing data races}}
-      // expected-note @-1 {{Passing value of non-Sendable type '() -> ()' as a 'sending' argument to local function 'sendingClosure' risks causing races in between local and caller code}}
-      _ = x
-      _ = y
-    }
-    sendingClosure { // expected-note {{access can happen concurrently}}
-      _ = x
-      _ = y
-    }
-  }
-
-  func testLetTwoNSVariableError3() async {
-    nonisolated(unsafe) let x = NonSendableKlass()
-    nonisolated(unsafe) let y = NonSendableKlass()
-    sendingClosure {
-      _ = x
-      _ = y
-    }
-    sendingClosure {
-      _ = x
-      _ = y
-    }
-  }
-
-  func testVarOneNSVariableError() async {
-    var x = NonSendableKlass()
-    x = NonSendableKlass()
-
-    sendingClosure { _ = x } // expected-warning {{sending value of non-Sendable type '() -> ()' risks causing data races}}
-    // expected-note @-1 {{Passing value of non-Sendable type '() -> ()' as a 'sending' argument to local function 'sendingClosure' risks causing races in between local and caller code}}
-    sendingClosure { _ = x } // expected-note {{access can happen concurrently}}
-  }
-
-  func testVarNonIsolatedUnsafeNSVariableNoError() async {
-    nonisolated(unsafe) var x = NonSendableKlass()
-    x = NonSendableKlass()
-
-    sendingClosure { _ = x }
-    sendingClosure { _ = x }
-  }
-
-  func testVarOneNSVariableSVariableError() async {
-    var x = NonSendableKlass()
-    x = NonSendableKlass()
-    var y = CustomActorInstance()
-    y = CustomActorInstance()
-    sendingClosure { // expected-warning {{sending value of non-Sendable type '() -> ()' risks causing data races}}
-      // expected-note @-1 {{Passing value of non-Sendable type '() -> ()' as a 'sending' argument to local function 'sendingClosure' risks causing races in between local and caller code}}
-      _ = x
-      _ = y
-    }
-    sendingClosure { // expected-note {{access can happen concurrently}}
-      _ = x
-      _ = y
-    }
-  }
-
-  func testVarNonIsolatedUnsafeNSSVariableNoError() async {
-    nonisolated(unsafe) var x = NonSendableKlass()
-    x = NonSendableKlass()
-    var y = CustomActorInstance()
-    y = CustomActorInstance()
-    sendingClosure {
-      _ = x
-      _ = y
-    }
-    sendingClosure {
-      _ = x
-      _ = y
-    }
-  }
-
-  func testVarTwoNSVariableError() async {
-    var x = NonSendableKlass()
-    x = NonSendableKlass()
-    var y = NonSendableKlass()
-    y = NonSendableKlass()
-    sendingClosure { // expected-warning {{sending value of non-Sendable type '() -> ()' risks causing data races}}
-      // expected-note @-1 {{Passing value of non-Sendable type '() -> ()' as a 'sending' argument to local function 'sendingClosure' risks causing races in between local and caller code}}
-      _ = x
-      _ = y
-    }
-    sendingClosure { // expected-note {{access can happen concurrently}}
-      _ = x
-      _ = y
-    }
-  }
-
-  func testVarTwoNSVariableError2() async {
-    nonisolated(unsafe) var x = NonSendableKlass()
-    x = NonSendableKlass()
-    var y = NonSendableKlass()
-    y = NonSendableKlass()
-    sendingClosure { // expected-warning {{sending value of non-Sendable type '() -> ()' risks causing data races}}
-      // expected-note @-1 {{Passing value of non-Sendable type '() -> ()' as a 'sending' argument to local function 'sendingClosure' risks causing races in between local and caller code}}
-      _ = x
-      _ = y
-    }
-    sendingClosure { // expected-note {{access can happen concurrently}}
-      _ = x
-      _ = y
-    }
-  }
-
-  func testVarTwoNSVariableError3() async {
-    nonisolated(unsafe) var x = NonSendableKlass()
-    x = NonSendableKlass()
-    nonisolated(unsafe) var y = NonSendableKlass()
-    y = NonSendableKlass()
-    sendingClosure {
-      _ = x
-      _ = y
-    }
-    sendingClosure {
-      _ = x
-      _ = y
-    }
-  }
-
-  func testWithTaskDetached() async {
-    let x1 = NonSendableKlass()
-    Task.detached { _ = x1 } // expected-warning {{sending value of non-Sendable type '() async -> ()' risks causing data races}}
-    // expected-note @-1 {{Passing value of non-Sendable type '() async -> ()' as a 'sending' argument to static method 'detached(name:priority:operation:)' risks causing races in between local and caller code}}
-    Task.detached { _ = x1 } // expected-note {{access can happen concurrently}}
-
-    nonisolated(unsafe) let x2 = NonSendableKlass()
-    Task.detached { _ = x2 }
-    Task.detached { _ = x2 }
-
-    nonisolated(unsafe) let x3a = NonSendableKlass()
-    nonisolated(unsafe) let x3b = NonSendableKlass()
-    Task.detached { _ = x3a; _ = x3b }
-    Task.detached { _ = x3a; _ = x3b }
-
-    nonisolated(unsafe) let x4a = NonSendableKlass()
-    let x4b = NonSendableKlass()
-    Task.detached { _ = x4a; _ = x4b } // expected-warning {{sending value of non-Sendable type '() async -> ()' risks causing data races}}
-    // expected-note @-1 {{Passing value of non-Sendable type '() async -> ()' as a 'sending' argument to static method 'detached(name:priority:operation:)' risks causing races in between local and caller code}}
-    Task.detached { _ = x4a; _ = x4b } // expected-note {{access can happen concurrently}}
-  }
-
-  // The reason why this works is that we do not infer nonisolated(unsafe)
-  // passed the begin_borrow [var_decl] of y. So we think the closure is
-  // nonisolated(unsafe), but its uses via the begin_borrow [var_decl] is
-  // not.
-  func testNamedClosure() async {
-    nonisolated(unsafe) let x = NonSendableKlass()
-    let y = {
-      _ = x
-    }
-    sendingClosure(y) // expected-warning {{sending 'y' risks causing data races}}
-    // expected-note @-1 {{'y' used after being passed as a 'sending' parameter}}
-    sendingClosure(y) // expected-note {{access can happen concurrently}}
-  }
-}