Propagate global actor-ness of functions/closures.

I fixed a bunch of small issues around here that resulted in a bunch of radars
being fixed. Specifically:

1. I made it so that we treat function_refs that are from an actor isolated
function as actor isolated instead of sendable.

2. I made it so that autoclosures which return global actor isolated functions
are treated as producing a global actor isolated function.

3. I made it so that we properly handle SILGen code patterns produced by
Sendable GlobalActor isolated things.

rdar://125452372
rdar://121954871
rdar://121955895
rdar://122692698

Author: gottesmm

include/swift/SILOptimizer/Utils/PartitionUtils.h

@@ -208,6 +208,10 @@ class SILIsolationInfo {
     return SILIsolationInfo();
   }
 
+  static SILIsolationInfo getGlobalActorIsolated(Type globalActorType) {
+    return getActorIsolated(ActorIsolation::forGlobalActor(globalActorType));
+  }
+
   static SILIsolationInfo getTaskIsolated(SILValue value) {
     return {Kind::Task, value};
   }

lib/SILGen/SILGen.cpp

@@ -725,6 +725,13 @@ SILFunction *SILGenModule::getFunction(SILDeclRef constant,
         return IGM.getFunction(constant, NotForDefinition);
       });
 
+  // If we have global actor isolation for our constant, put the isolation onto
+  // the function.
+  if (auto isolation =
+          getActorIsolationOfContext(constant.getInnermostDeclContext())) {
+    F->setActorIsolation(isolation);
+  }
+
   assert(F && "SILFunction should have been defined");
 
   emittedFunctions[constant] = F;
@@ -1217,9 +1224,12 @@ void SILGenModule::preEmitFunction(SILDeclRef constant, SILFunction *F,
   if (F->getLoweredFunctionType()->isPolymorphic())
     F->setGenericEnvironment(Types.getConstantGenericEnvironment(constant));
 
-  // Set the actor isolation of the function to its innermost decl context.
-  F->setActorIsolation(
-      getActorIsolationOfContext(constant.getInnermostDeclContext()));
+  // If we have global actor isolation for our constant, put the isolation onto
+  // the function.
+  if (auto isolation =
+          getActorIsolationOfContext(constant.getInnermostDeclContext())) {
+    F->setActorIsolation(isolation);
+  }
 
   // Create a debug scope for the function using astNode as source location.
   F->setDebugScope(new (M) SILDebugScope(Loc, F));

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -1848,7 +1848,7 @@ class PartitionOpTranslator {
     }
 
     // If our self parameter was transferring, transfer it. Otherwise, just
-    // stick it in the non seld operand values array and run multiassign on
+    // stick it in the non self operand values array and run multiassign on
     // it.
     if (fas.hasSelfArgument()) {
       auto &selfOperand = fas.getSelfArgumentOperand();
@@ -1864,6 +1864,12 @@ class PartitionOpTranslator {
       }
     }
 
+    // Add our callee to non-transferring parameters. This ensures that if it is
+    // actor isolated, that propagates into our results. This is especially
+    // important since our callee could be dynamically isolated and we cannot
+    // know that until we perform dataflow.
+    nonTransferringParameters.push_back(fas.getCallee());
+
     SmallVector<SILValue, 8> applyResults;
     getApplyResults(*fas, applyResults);
 
@@ -3283,12 +3289,59 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
     }
   }
 
-  // Then see if we have a sendable value. By default we assume values are not
-  // sendable.
   if (auto *defInst = value.getDefiningInstruction()) {
-    // Though these values are technically non-Sendable, we can safely and
-    // consistently treat them as Sendable.
-    if (isa<ClassMethodInst, FunctionRefInst>(defInst)) {
+    // Treat function ref as either actor isolated or sendable.
+    if (auto *fri = dyn_cast<FunctionRefInst>(defInst)) {
+      auto isolation = fri->getReferencedFunction()->getActorIsolation();
+      if (isolation.isActorIsolated()) {
+        iter.first->getSecond().mergeIsolationRegionInfo(
+            SILIsolationInfo::getActorIsolated(isolation));
+        return {iter.first->first, iter.first->second};
+      }
+
+      // Otherwise, lets look at the AST and see if our function ref is from an
+      // autoclosure.
+      if (auto *autoclosure = fri->getLoc().getAsASTNode<AutoClosureExpr>()) {
+        if (auto *funcType = autoclosure->getType()->getAs<AnyFunctionType>()) {
+          if (funcType->hasGlobalActor()) {
+            if (funcType->hasGlobalActor()) {
+              iter.first->getSecond().mergeIsolationRegionInfo(
+                  SILIsolationInfo::getActorIsolated(
+                      ActorIsolation::forGlobalActor(
+                          funcType->getGlobalActor())));
+              return {iter.first->first, iter.first->second};
+            }
+          }
+
+          if (auto *resultFType =
+                  funcType->getResult()->getAs<AnyFunctionType>()) {
+            if (resultFType->hasGlobalActor()) {
+              iter.first->getSecond().mergeIsolationRegionInfo(
+                  SILIsolationInfo::getActorIsolated(
+                      ActorIsolation::forGlobalActor(
+                          resultFType->getGlobalActor())));
+              return {iter.first->first, iter.first->second};
+            }
+          }
+        }
+      }
+
+      iter.first->getSecond().addFlag(TrackableValueFlag::isSendable);
+      return {iter.first->first, iter.first->second};
+    }
+
+    if (auto *cmi = dyn_cast<ClassMethodInst>(defInst)) {
+      if (auto *declRefExpr = cmi->getLoc().getAsASTNode<DeclRefExpr>()) {
+        // See if we are actor isolated. If so, treat this as non-Sendable so we
+        // propagate actor isolation.
+        if (auto isolation = getActorIsolation(declRefExpr->getDecl())) {
+          if (isolation.isActorIsolated()) {
+            iter.first->getSecond().mergeIsolationRegionInfo(
+                SILIsolationInfo::getActorIsolated(isolation));
+            return {iter.first->first, iter.first->second};
+          }
+        }
+      }
       iter.first->getSecond().addFlag(TrackableValueFlag::isSendable);
       return {iter.first->first, iter.first->second};
     }
@@ -3379,6 +3432,71 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
     }
   }
 
+  // See if we have a convert function from a Sendable actor isolated function,
+  // we want to treat the result of the convert function as being actor isolated
+  // so that we cannot escape the value.
+  //
+  // NOTE: At this point, we already know that cfi's result is not sendable,
+  // since we would have exited above already.
+  if (auto *cfi = dyn_cast<ConvertFunctionInst>(iter.first->first.getValue())) {
+    SILValue operand = cfi->getOperand();
+    if (operand->getType().getAs<SILFunctionType>()->isSendable()) {
+      SILValue newValue = operand;
+      do {
+        operand = newValue;
+
+        newValue = lookThroughOwnershipInsts(operand);
+        if (auto *ttfi = dyn_cast<ThinToThickFunctionInst>(newValue)) {
+          newValue = ttfi->getOperand();
+        }
+
+        if (auto *cfi = dyn_cast<ConvertFunctionInst>(newValue)) {
+          newValue = cfi->getOperand();
+        }
+
+        if (auto *pai = dyn_cast<PartialApplyInst>(newValue)) {
+          newValue = pai->getCallee();
+        }
+      } while (newValue != operand);
+
+      if (auto *ai = dyn_cast<ApplyInst>(operand)) {
+        if (auto *callExpr = ai->getLoc().getAsASTNode<ApplyExpr>()) {
+          if (auto *callType = callExpr->getType()->getAs<AnyFunctionType>()) {
+            if (callType->hasGlobalActor()) {
+              iter.first->getSecond().mergeIsolationRegionInfo(
+                  SILIsolationInfo::getGlobalActorIsolated(
+                      callType->getGlobalActor()));
+              return {iter.first->first, iter.first->second};
+            }
+          }
+        }
+      }
+
+      if (auto *fri = dyn_cast<FunctionRefInst>(operand)) {
+        if (auto actorIsolation =
+                fri->getReferencedFunction()->getActorIsolation()) {
+          if (actorIsolation.isActorIsolated()) {
+            iter.first->getSecond().mergeIsolationRegionInfo(
+                SILIsolationInfo::getActorIsolated(actorIsolation));
+            return {iter.first->first, iter.first->second};
+          }
+        }
+
+        // See if the function ref statically is known to have actor isolation.
+        //
+        // TODO: We should make it so that the closure constructed has actor
+        // isolation.
+        if (auto value = tryToTrackValue(fri)) {
+          auto isolation = value->getIsolationRegionInfo();
+          if (isolation.isActorIsolated()) {
+            iter.first->getSecond().mergeIsolationRegionInfo(isolation);
+            return {iter.first->first, iter.first->second};
+          }
+        }
+      }
+    }
+  }
+
   return {iter.first->first, iter.first->second};
 }
 

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

@@ -1152,13 +1152,18 @@ class TransferNonTransferrableDiagnosticInferrer {
   bool run();
 
 private:
-  bool initForIsolatedPartialApply(Operand *op, AbstractClosureExpr *ace);
+  /// \p actualCallerIsolation is used to override the caller isolation we use
+  /// when emitting the error if the closure would have the incorrect one.
+  bool initForIsolatedPartialApply(
+      Operand *op, AbstractClosureExpr *ace,
+      std::optional<ActorIsolation> actualCallerIsolation = {});
 };
 
 } // namespace
 
 bool TransferNonTransferrableDiagnosticInferrer::initForIsolatedPartialApply(
-    Operand *op, AbstractClosureExpr *ace) {
+    Operand *op, AbstractClosureExpr *ace,
+    std::optional<ActorIsolation> actualCallerIsolation) {
   SmallVector<std::tuple<CapturedValue, unsigned, ApplyIsolationCrossing>, 8>
       foundCapturedIsolationCrossing;
   ace->getIsolationCrossing(foundCapturedIsolationCrossing);
@@ -1169,8 +1174,15 @@ bool TransferNonTransferrableDiagnosticInferrer::initForIsolatedPartialApply(
   for (auto &p : foundCapturedIsolationCrossing) {
     if (std::get<1>(p) == opIndex) {
       auto loc = RegularLocation(std::get<0>(p).getLoc());
+      auto crossing = std::get<2>(p);
+      auto declIsolation = crossing.getCallerIsolation();
+      auto closureIsolation = crossing.getCalleeIsolation();
+      if (!bool(declIsolation) && actualCallerIsolation) {
+        declIsolation = *actualCallerIsolation;
+      }
       diagnosticEmitter.emitNamedFunctionArgumentClosure(
-          loc, std::get<0>(p).getDecl()->getBaseIdentifier(), std::get<2>(p));
+          loc, std::get<0>(p).getDecl()->getBaseIdentifier(),
+          ApplyIsolationCrossing(declIsolation, closureIsolation));
       return true;
     }
   }
@@ -1220,6 +1232,15 @@ bool TransferNonTransferrableDiagnosticInferrer::run() {
     }
     assert(isolation && "Expected non-null");
 
+    // Then if we are calling a closure expr. If so, we should use the loc of
+    // the closure.
+    if (auto *closureExpr =
+            dyn_cast<AbstractClosureExpr>(sourceApply->getFn())) {
+      initForIsolatedPartialApply(op, closureExpr,
+                                  isolation->getCallerIsolation());
+      return true;
+    }
+
     // See if we can infer a name from the value.
     SmallString<64> resultingName;
     if (auto name = inferNameFromValue(op->get())) {

test/Concurrency/Inputs/GlobalActorIsolatedFunction.swift

@@ -0,0 +1,2 @@
+
+@MainActor public func mainActorFunction() {}

test/Concurrency/isolated_captures.swift

@@ -39,9 +39,9 @@ class NotSendable {
   let ns = NotSendable(x: 0)
   MyActor.ns = ns
 
-  // expected-region-isolation-warning @+2 {{transferring 'ns' may cause a race}}
-  // expected-region-isolation-note @+1 {{transferring global actor 'MyActor'-isolated 'ns' to global actor 'YourActor'-isolated callee could cause races between global actor 'YourActor'-isolated and global actor 'MyActor'-isolated uses}}
   await { @YourActor in
+    // expected-region-isolation-warning @+3 {{transferring 'ns' may cause a race}}
+    // expected-region-isolation-note @+2 {{global actor 'MyActor'-isolated 'ns' is captured by a global actor 'YourActor'-isolated closure. global actor 'YourActor'-isolated uses in closure may race against later global actor 'MyActor'-isolated uses}}
     // expected-complete-warning@+1 {{capture of 'ns' with non-sendable type 'NotSendable' in an isolated closure; this is an error in the Swift 6 language mode}}
     YourActor.ns = ns
   }()
@@ -61,8 +61,9 @@ class NotSendable {
   let ns = NotSendable(x: 0)
   ns.stash()
 
-  // FIXME: Region isolation should diagnose this (https://github.com/apple/swift/issues/71533)
   await { @YourActor in
+    // expected-region-isolation-warning @+3 {{transferring 'ns' may cause a race}}
+    // expected-region-isolation-note @+2 {{global actor 'MyActor'-isolated 'ns' is captured by a global actor 'YourActor'-isolated closure. global actor 'YourActor'-isolated uses in closure may race against later global actor 'MyActor'-isolated uses}}
     // expected-complete-warning@+1 {{capture of 'ns' with non-sendable type 'NotSendable' in an isolated closure; this is an error in the Swift 6 language mode}}
     YourActor.ns = ns
   }()
@@ -81,8 +82,9 @@ class NotSendable {
 @MyActor func exhibitRace3() async {
   let ns = NotSendable()
 
-  // FIXME: Region isolation should diagnose this (https://github.com/apple/swift/issues/71533)
   await { @YourActor in
+    // expected-region-isolation-warning @+3 {{transferring 'ns' may cause a race}}
+    // expected-region-isolation-note @+2 {{global actor 'MyActor'-isolated 'ns' is captured by a global actor 'YourActor'-isolated closure. global actor 'YourActor'-isolated uses in closure may race against later global actor 'MyActor'-isolated uses}}
     // expected-complete-warning@+1 {{capture of 'ns' with non-sendable type 'NotSendable' in an isolated closure; this is an error in the Swift 6 language mode}}
     YourActor.ns = ns
   }()

test/Concurrency/transfernonsendable_global_actor.swift

@@ -22,6 +22,9 @@ func transferToNonIsolated<T>(_ t: T) async {}
 @MainActor func transferToMainActor<T>(_ t: T) async {}
 @CustomActor func transferToCustomActor<T>(_ t: T) async {}
 func useValue<T>(_ t: T) {}
+func useValueAsync<T>(_ t: T) async {}
+@MainActor func useValueMainActor<T>(_ t: T) {}
+@MainActor func mainActorFunction() {}
 
 var booleanFlag: Bool { false }
 @MainActor var mainActorIsolatedGlobal = NonSendableKlass()
@@ -221,3 +224,83 @@ struct Clock {
   // transferred value 'ns' here.
   useValue(ns)
 }
+
+@MainActor func synchronousActorIsolatedClosureError() async {
+  let closure = { @MainActor @Sendable in
+    MainActor.assertIsolated()
+  }
+
+  let erased: () -> Void = closure
+
+  await useValueAsync(erased) // expected-tns-warning {{transferring 'erased' may cause a race}}
+  // expected-tns-note @-1 {{transferring main actor-isolated 'erased' to nonisolated callee could cause races between nonisolated and main actor-isolated uses}}
+  // expected-complete-warning @-2 {{passing argument of non-sendable type '() -> Void' outside of main actor-isolated context may introduce data races}}
+  // expected-complete-note @-3 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
+}
+
+@MainActor func synchronousActorIsolatedFunctionError() async {
+  let erased: () -> Void = mainActorFunction
+
+  await useValueAsync(erased) // expected-tns-warning {{transferring 'erased' may cause a race}}
+  // expected-tns-note @-1 {{transferring main actor-isolated 'erased' to nonisolated callee could cause races between nonisolated and main actor-isolated uses}}
+  // expected-complete-warning @-2 {{passing argument of non-sendable type '() -> Void' outside of main actor-isolated context may introduce data races}}
+  // expected-complete-note @-3 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
+}
+
+@MainActor func synchronousActorIsolatedGenericFunctionError<T>(_ t: T) async {
+  let erased: (T) -> Void = useValueMainActor
+
+  await useValueAsync(erased) // expected-tns-warning {{transferring 'erased' may cause a race}}
+  // expected-tns-note @-1 {{transferring main actor-isolated 'erased' to nonisolated callee could cause races between nonisolated and main actor-isolated uses}}
+  // expected-complete-warning @-2 {{passing argument of non-sendable type '(T) -> Void' outside of main actor-isolated context may introduce data races}}
+  // expected-complete-note @-3 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
+}
+
+@MainActor func synchronousActorIsolatedClassMethodError() async {
+  @MainActor class Test {
+    func foo() {}
+  }
+
+  let t = Test()
+  let erased: () -> Void = t.foo
+
+  await useValueAsync(erased) // expected-tns-warning {{transferring 'erased' may cause a race}}
+  // expected-tns-note @-1 {{transferring main actor-isolated 'erased' to nonisolated callee could cause races between nonisolated and main actor-isolated uses}}
+  // expected-complete-warning @-2 {{passing argument of non-sendable type '() -> Void' outside of main actor-isolated context may introduce data races}}
+  // expected-complete-note @-3 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
+}
+
+@MainActor func synchronousActorIsolatedFinalClassMethodError() async {
+  @MainActor final class Test {
+    func foo() {}
+  }
+
+  let t = Test()
+  let erased: () -> Void = t.foo
+
+  await useValueAsync(erased) // expected-tns-warning {{transferring 'erased' may cause a race}}
+  // expected-tns-note @-1 {{transferring main actor-isolated 'erased' to nonisolated callee could cause races between nonisolated and main actor-isolated uses}}
+  // expected-complete-warning @-2 {{passing argument of non-sendable type '() -> Void' outside of main actor-isolated context may introduce data races}}
+  // expected-complete-note @-3 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
+}
+
+@MainActor func synchronousClosureCapturingGlobalActorIsolatedGlobal() async {
+  let closure = {
+    print(mainActorIsolatedGlobal)
+  }
+  // Regions: [{(closure), @MainActor}]
+  await transferToCustomActor(closure) // expected-tns-warning {{transferring 'closure' may cause a race}}
+  // expected-tns-note @-1 {{transferring main actor-isolated 'closure' to global actor 'CustomActor'-isolated callee could cause races between global actor 'CustomActor'-isolated and main actor-isolated uses}}
+  // expected-complete-warning @-2 {{passing argument of non-sendable type '() -> ()' into global actor 'CustomActor'-isolated context may introduce data races}}
+  // expected-complete-note @-3 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
+}
+
+@MainActor func synchronousClosureCapturingGlobalActorIsolatedFunction() async {
+  let closure = {
+    mainActorFunction()
+  }
+  await transferToCustomActor(closure) // expected-tns-warning {{transferring 'closure' may cause a race}}
+  // expected-tns-note @-1 {{transferring main actor-isolated 'closure' to global actor 'CustomActor'-isolated callee could cause races between global actor 'CustomActor'-isolated and main actor-isolated uses}}
+  // expected-complete-warning @-2 {{passing argument of non-sendable type '() -> ()' into global actor 'CustomActor'-isolated context may introduce data races}}
+  // expected-complete-note @-3 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
+}