[region-isolation] When inferring isolation for an argument, handle non-self isolated parameters as well as self parameters that are actor isolated.

As part of this I went through how we handled inference and rather than using a
grab-bag getActorIsolation that was confusing to use, I created split APIs for
specific use cases (actor instance, global actor, just an apply expr crossing)
that makes it clearer inside the SILIsolationInfo::get* APIs what we are
actually trying to model. I found a few issues as a result and fixed most of
them if they were small. I also fixed one bigger one around computed property
initializers in the next commit. There is a larger change I didn't fix around allowing function
ref/partial_apply with isolated self parameters have a delayed flow sensitive
actor isolation... this will be fixed in a subsequent commit.

This also fixes a bunch of cases where we were printing actor-isolated instead
of 'self' isolated.

rdar://127295657

Author: gottesmm

include/swift/SILOptimizer/Analysis/RegionAnalysis.h

@@ -31,27 +31,6 @@ using TransferringOperandSetFactory = Partition::TransferringOperandSetFactory;
 using Element = PartitionPrimitives::Element;
 using Region = PartitionPrimitives::Region;
 
-/// Check if the passed in type is NonSendable.
-///
-/// NOTE: We special case RawPointer and NativeObject to ensure they are
-/// treated as non-Sendable and strict checking is applied to it.
-inline bool isNonSendableType(SILType type, SILFunction *fn) {
-  // Treat Builtin.NativeObject and Builtin.RawPointer as non-Sendable.
-  if (type.getASTType()->is<BuiltinNativeObjectType>() ||
-      type.getASTType()->is<BuiltinRawPointerType>()) {
-    return true;
-  }
-
-  // Treat Builtin.SILToken as Sendable. It cannot escape from the current
-  // function. We should change isSendable to hardwire this.
-  if (type.getASTType()->is<SILTokenType>()) {
-    return false;
-  }
-
-  // Otherwise, delegate to seeing if type conforms to the Sendable protocol.
-  return !type.isSendable(fn);
-}
-
 /// Return the ApplyIsolationCrossing for a specific \p inst if it
 /// exists. Returns std::nullopt otherwise.
 std::optional<ApplyIsolationCrossing>

include/swift/SILOptimizer/Utils/PartitionUtils.h

@@ -124,8 +124,8 @@ class SILIsolationInfo {
   /// derived isolatedValue from.
   SILValue actorInstance;
 
-  SILIsolationInfo(ActorIsolation actorIsolation, SILValue isolatedValue,
-                   SILValue actorInstance)
+  SILIsolationInfo(SILValue isolatedValue, SILValue actorInstance,
+                   ActorIsolation actorIsolation)
       : kind(Actor), actorIsolation(actorIsolation),
         isolatedValue(isolatedValue), actorInstance(actorInstance) {
     assert((!actorInstance ||
@@ -193,37 +193,78 @@ class SILIsolationInfo {
 
   [[nodiscard]] SILIsolationInfo merge(SILIsolationInfo other) const;
 
-  SILIsolationInfo withActorIsolated(SILValue isolatedValue,
-                                     SILValue actorInstance,
-                                     ActorIsolation isolation) {
-    return SILIsolationInfo::getActorIsolated(isolatedValue, actorInstance,
-                                              isolation);
-  }
-
   static SILIsolationInfo getDisconnected() { return {Kind::Disconnected}; }
 
-  static SILIsolationInfo getActorIsolated(SILValue isolatedValue,
-                                           SILValue actorInstance,
-                                           ActorIsolation actorIsolation) {
-    return {actorIsolation, isolatedValue, actorInstance};
-  }
-
-  static SILIsolationInfo getActorIsolated(SILValue isolatedValue,
-                                           SILValue actorInstance,
-                                           NominalTypeDecl *typeDecl) {
-    if (typeDecl->isAnyActor())
-      return {ActorIsolation::forActorInstanceSelf(typeDecl), isolatedValue,
-              actorInstance};
-    auto isolation = swift::getActorIsolation(typeDecl);
-    if (isolation.isGlobalActor())
-      return {isolation, isolatedValue, actorInstance};
+  /// Create an actor isolation for a value that we know is actor isolated to a
+  /// specific actor, but we do not know the specific instance yet.
+  ///
+  /// This can occur when closing over a closure with an isolated parameter or
+  /// if we are determining isolation of a function_ref that takes an isolated
+  /// parameter. In both cases, we cannot know what the actual isolation is
+  /// until we invoke the closure or function.
+  ///
+  /// TODO: This is just a stub currently until I implement the flow sensitive
+  /// part. We just treat all instances the same. There are tests that validate
+  /// this behavior.
+  static SILIsolationInfo
+  getFlowSensitiveActorIsolated(SILValue isolatedValue,
+                                ActorIsolation actorIsolation) {
+    return {isolatedValue, SILValue(), actorIsolation};
+  }
+
+  /// Only use this as a fallback if we cannot find better information.
+  static SILIsolationInfo
+  getWithIsolationCrossing(ApplyIsolationCrossing crossing) {
+    if (crossing.getCalleeIsolation().isActorIsolated()) {
+      // SIL level, just let it through
+      return SILIsolationInfo(SILValue(), SILValue(),
+                              crossing.getCalleeIsolation());
+    }
+
     return {};
   }
 
+  static SILIsolationInfo getActorInstanceIsolated(SILValue isolatedValue,
+                                                   SILValue actorInstance,
+                                                   NominalTypeDecl *typeDecl) {
+    assert(actorInstance);
+    if (!typeDecl->isAnyActor()) {
+      assert(!swift::getActorIsolation(typeDecl).isGlobalActor() &&
+             "Should have called getGlobalActorIsolated");
+      return {};
+    }
+    return {isolatedValue, actorInstance,
+            ActorIsolation::forActorInstanceSelf(typeDecl)};
+  }
+
+  /// A special actor instance isolated for partial apply cases where we do not
+  /// close over the isolated parameter and thus do not know the actual actor
+  /// instance that we are going to use.
+  static SILIsolationInfo
+  getPartialApplyActorInstanceIsolated(SILValue isolatedValue,
+                                       NominalTypeDecl *typeDecl) {
+    if (!typeDecl->isAnyActor()) {
+      assert(!swift::getActorIsolation(typeDecl).isGlobalActor() &&
+             "Should have called getGlobalActorIsolated");
+      return {};
+    }
+    return {isolatedValue, SILValue(),
+            ActorIsolation::forActorInstanceSelf(typeDecl)};
+  }
+
   static SILIsolationInfo getGlobalActorIsolated(SILValue value,
                                                  Type globalActorType) {
-    return getActorIsolated(value, SILValue() /*no actor instance*/,
-                            ActorIsolation::forGlobalActor(globalActorType));
+    return {value, SILValue() /*no actor instance*/,
+            ActorIsolation::forGlobalActor(globalActorType)};
+  }
+
+  static SILIsolationInfo getGlobalActorIsolated(SILValue value,
+                                                 ValueDecl *decl) {
+    auto isolation = swift::getActorIsolation(decl);
+    if (!isolation.isGlobalActor())
+      return {};
+    return SILIsolationInfo::getGlobalActorIsolated(value,
+                                                    isolation.getGlobalActor());
   }
 
   static SILIsolationInfo getTaskIsolated(SILValue value) {
@@ -244,6 +285,18 @@ class SILIsolationInfo {
     return {};
   }
 
+  /// A helper that is used to ensure that we treat certain builtin values as
+  /// non-Sendable that the AST level otherwise thinks are non-Sendable.
+  ///
+  /// E.x.: Builtin.RawPointer and Builtin.NativeObject
+  ///
+  /// TODO: Fix the type checker.
+  static bool isNonSendableType(SILType type, SILFunction *fn);
+
+  static bool isNonSendableType(SILValue value) {
+    return isNonSendableType(value->getType(), value->getFunction());
+  }
+
   bool hasSameIsolation(ActorIsolation actorIsolation) const;
 
   /// Returns true if \p this and \p other have the same isolation. It allows

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -124,7 +124,8 @@ struct UseDefChainVisitor
     // If this is a type case, see if the result of the cast is sendable. In
     // such a case, we do not want to look through this cast.
     if (castType == AccessStorageCast::Type &&
-        !isNonSendableType(cast->getType(), cast->getFunction()))
+        !SILIsolationInfo::isNonSendableType(cast->getType(),
+                                             cast->getFunction()))
       return SILValue();
 
     // If we do not have an identity cast, mark this as a merge.
@@ -160,7 +161,8 @@ struct UseDefChainVisitor
 
         // See if our operand type is a sendable type. In such a case, we do not
         // want to look through our operand.
-        if (!isNonSendableType(op->getType(), op->getFunction()))
+        if (!SILIsolationInfo::isNonSendableType(op->getType(),
+                                                 op->getFunction()))
           return SILValue();
 
         break;
@@ -169,7 +171,8 @@ struct UseDefChainVisitor
         // These are merges if we have multiple fields.
         auto op = cast<TupleElementAddrInst>(inst)->getOperand();
 
-        if (!isNonSendableType(op->getType(), op->getFunction()))
+        if (!SILIsolationInfo::isNonSendableType(op->getType(),
+                                                 op->getFunction()))
           return SILValue();
 
         isMerge |= op->getType().getNumTupleElements() > 1;
@@ -183,7 +186,8 @@ struct UseDefChainVisitor
         // identify the sendable type with the non-sendable operand. These we
         // are always going to ignore anyways since a sendable let/var field of
         // a struct can always be used.
-        if (!isNonSendableType(op->getType(), op->getFunction()))
+        if (!SILIsolationInfo::isNonSendableType(op->getType(),
+                                                 op->getFunction()))
           return SILValue();
 
         // These are merges if we have multiple fields.
@@ -314,21 +318,23 @@ static SILValue getUnderlyingTrackedObjectValue(SILValue value) {
       // If we have a cast and our operand and result are non-Sendable, treat it
       // as a look through.
       if (isLookThroughIfOperandAndResultNonSendable(svi)) {
-        if (isNonSendableType(svi->getType(), fn) &&
-            isNonSendableType(svi->getOperand(0)->getType(), fn)) {
+        if (SILIsolationInfo::isNonSendableType(svi->getType(), fn) &&
+            SILIsolationInfo::isNonSendableType(svi->getOperand(0)->getType(),
+                                                fn)) {
           temp = svi->getOperand(0);
         }
       }
 
       if (isLookThroughIfResultNonSendable(svi)) {
-        if (isNonSendableType(svi->getType(), fn)) {
+        if (SILIsolationInfo::isNonSendableType(svi->getType(), fn)) {
           temp = svi->getOperand(0);
         }
       }
 
       if (isLookThroughIfOperandNonSendable(svi)) {
         // If our operand is a non-Sendable type, look through this instruction.
-        if (isNonSendableType(svi->getOperand(0)->getType(), fn)) {
+        if (SILIsolationInfo::isNonSendableType(svi->getOperand(0)->getType(),
+                                                fn)) {
           temp = svi->getOperand(0);
         }
       }
@@ -1464,7 +1470,7 @@ class PartitionOpTranslator {
   /// NOTE: We special case RawPointer and NativeObject to ensure they are
   /// treated as non-Sendable and strict checking is applied to it.
   bool isNonSendableType(SILType type) const {
-    return ::isNonSendableType(type, function);
+    return SILIsolationInfo::isNonSendableType(type, function);
   }
 
   TrackableValue
@@ -2221,7 +2227,8 @@ class PartitionOpTranslator {
     case TranslationSemantics::AssertingIfNonSendable:
       // Do not error if all of our operands are sendable.
       if (llvm::none_of(inst->getOperandValues(), [&](SILValue value) {
-            return ::isNonSendableType(value->getType(), inst->getFunction());
+            return ::SILIsolationInfo::isNonSendableType(value->getType(),
+                                                         inst->getFunction());
           }))
         return;
       llvm::errs() << "BadInst: " << *inst;
@@ -3242,11 +3249,18 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
     // If we were able to find this was actor isolated from finding our
     // underlying object, use that. It is never wrong.
     if (info.actorIsolation) {
-      SILValue actorInstance =
-          info.value->getType().isAnyActor() ? info.value : SILValue();
-      iter.first->getSecond().mergeIsolationRegionInfo(
-          SILIsolationInfo::getActorIsolated(value, actorInstance,
-                                             *info.actorIsolation));
+      SILIsolationInfo isolation;
+      if (info.value->getType().isAnyActor()) {
+        isolation = SILIsolationInfo::getActorInstanceIsolated(
+            value, info.value, info.actorIsolation->getActor());
+      } else if (info.actorIsolation->isGlobalActor()) {
+        isolation = SILIsolationInfo::getGlobalActorIsolated(
+            value, info.actorIsolation->getGlobalActor());
+      }
+
+      if (isolation) {
+        iter.first->getSecond().mergeIsolationRegionInfo(isolation);
+      }
     }
 
     auto storage = AccessStorageWithBase::compute(value);
@@ -3278,7 +3292,7 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
   }
 
   // Otherwise refer to the oracle. If we have a Sendable value, just return.
-  if (!isNonSendableType(value->getType(), fn)) {
+  if (!SILIsolationInfo::isNonSendableType(value->getType(), fn)) {
     iter.first->getSecond().addFlag(TrackableValueFlag::isSendable);
     return {iter.first->first, iter.first->second};
   }
@@ -3296,8 +3310,9 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
     auto parentAddrInfo = getUnderlyingTrackedValue(svi);
     if (parentAddrInfo.actorIsolation) {
       iter.first->getSecond().mergeIsolationRegionInfo(
-          SILIsolationInfo::getActorIsolated(svi, parentAddrInfo.value,
-                                             *parentAddrInfo.actorIsolation));
+          SILIsolationInfo::getActorInstanceIsolated(
+              svi, parentAddrInfo.value,
+              parentAddrInfo.actorIsolation->getActor()));
     }
 
     auto storage = AccessStorageWithBase::compute(svi->getOperand(0));

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

@@ -1527,8 +1527,8 @@ struct DiagnosticEvaluator final
     if (auto *svi =
             dyn_cast<SingleValueInstruction>(partitionOp.getSourceInst())) {
       if (isa<TupleElementAddrInst, StructElementAddrInst>(svi) &&
-          !regionanalysisimpl::isNonSendableType(svi->getType(),
-                                                 svi->getFunction())) {
+          !SILIsolationInfo::isNonSendableType(svi->getType(),
+                                               svi->getFunction())) {
         bool isCapture = operandState.isClosureCaptured;
         if (!isCapture) {
           return;