[region-isolation] Given a .none #isolation parameter, infer the isolation from the callee rather than the isolated parameter.

Otherwise, we will have differing isolation from other parameters since
the isolations will look different since one will have the .none value
as an instance and the other will not have one and instead will rely on
the AST isolation info. That is the correct behavior here since we do
not actually have an actor here.

I also removed some undefined behavior in the merging code. The way the
code should work is that we should check if the merge fails and in such
a case emit an unknown pattern error... instead of not checking
appropriately on the next iteration and hitting undefined behavior.

rdar://130396399

Author: gottesmm

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -1590,6 +1590,10 @@ class PartitionOpTranslator {
     SmallVector<SILValue, 8> assignOperands;
     SmallVector<SILValue, 8> assignResults;
 
+    // A helper we use to emit an unknown patten error if our merge is
+    // invalid. This ensures we guarantee that if we find an actor merge error,
+    // the compiler halts. Importantly this lets our users know 100% that if the
+    // compiler exits successfully, actor merge errors could not have happened.
     std::optional<SILDynamicMergedIsolationInfo> mergedInfo;
     if (resultIsolationInfoOverride) {
       mergedInfo = resultIsolationInfoOverride;
@@ -1600,12 +1604,26 @@ class PartitionOpTranslator {
     for (SILValue src : sourceValues) {
       if (auto value = tryToTrackValue(src)) {
         assignOperands.push_back(value->getRepresentative().getValue());
-        mergedInfo = mergedInfo->merge(value->getIsolationRegionInfo());
+        auto originalMergedInfo = mergedInfo;
+        (void)originalMergedInfo;
+        if (mergedInfo)
+          mergedInfo = mergedInfo->merge(value->getIsolationRegionInfo());
 
         // If we fail to merge, then we have an incompatibility in between some
         // of our arguments (consider isolated to different actors) or with the
         // isolationInfo we specified. Emit an unknown patten error.
         if (!mergedInfo) {
+          LLVM_DEBUG(
+              llvm::dbgs() << "Merge Failure!\n"
+                           << "Original Info: ";
+              if (originalMergedInfo)
+                  originalMergedInfo->printForDiagnostics(llvm::dbgs());
+              else llvm::dbgs() << "nil";
+              llvm::dbgs() << "\nValue: "
+                           << value->getRepresentative().getValue();
+              llvm::dbgs() << "Value Info: ";
+              value->getIsolationRegionInfo().printForDiagnostics(llvm::dbgs());
+              llvm::dbgs() << "\n");
           builder.addUnknownPatternError(src);
           continue;
         }
@@ -1614,7 +1632,7 @@ class PartitionOpTranslator {
 
     for (SILValue result : resultValues) {
       // If we had isolation info explicitly passed in... use our
-      // mergedInfo. Otherwise, we want to infer.
+      // resultIsolationInfoError. Otherwise, we want to infer.
       if (resultIsolationInfoOverride) {
         // We only get back result if it is non-Sendable.
         if (auto nonSendableValue =
@@ -1657,9 +1675,10 @@ class PartitionOpTranslator {
       // derived, introduce a fake element so we just propagate the actor
       // region.
       //
-      // NOTE: Here we check if we have mergedInfo rather than isolationInfo
-      // since we want to do this regardless of whether or not we passed in a
-      // specific isolation info unlike earlier when processing actual results.
+      // NOTE: Here we check if we have resultIsolationInfoOverride rather than
+      // isolationInfo since we want to do this regardless of whether or not we
+      // passed in a specific isolation info unlike earlier when processing
+      // actual results.
       if (assignOperands.size() && resultIsolationInfoOverride) {
         builder.addActorIntroducingInst(assignOperands.back(),
                                         resultIsolationInfoOverride);

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

@@ -364,21 +364,31 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
     }
 
     if (auto *isolatedOp = fas.getIsolatedArgumentOperandOrNullPtr()) {
-      // First pattern match from global actors being passed as isolated
-      // parameters. This gives us better type information. If we can pattern
-      // match... we should!
+      // First see if we have an enum inst.
       if (auto *ei = dyn_cast<EnumInst>(isolatedOp->get())) {
-        if (ei->getElement()->getParentEnum()->isOptionalDecl() &&
-            ei->hasOperand()) {
-          if (auto *ieri = dyn_cast<InitExistentialRefInst>(ei->getOperand())) {
-            CanType selfASTType = ieri->getFormalConcreteType();
-
-            if (auto *nomDecl = selfASTType->getAnyActor()) {
-              // The SILValue() parameter doesn't matter until we have isolation
-              // history.
-              if (nomDecl->isGlobalActor())
-                return SILIsolationInfo::getGlobalActorIsolated(SILValue(),
-                                                                nomDecl);
+        if (ei->getElement()->getParentEnum()->isOptionalDecl()) {
+          // Pattern match from global actors being passed as isolated
+          // parameters. This gives us better type information. If we can
+          // pattern match... we should!
+          if (ei->hasOperand()) {
+            if (auto *ieri =
+                    dyn_cast<InitExistentialRefInst>(ei->getOperand())) {
+              CanType selfASTType = ieri->getFormalConcreteType();
+
+              if (auto *nomDecl = selfASTType->getAnyActor()) {
+                // The SILValue() parameter doesn't matter until we have
+                // isolation history.
+                if (nomDecl->isGlobalActor())
+                  return SILIsolationInfo::getGlobalActorIsolated(SILValue(),
+                                                                  nomDecl);
+              }
+            }
+          } else {
+            // In this case, we have a .none so we are attempting to use the
+            // global queue. In such a case, we need to not use the enum as our
+            // value and instead need to grab the isolation of our apply.
+            if (auto isolationInfo = get(fas.getCallee())) {
+              return isolationInfo;
             }
           }
         }

test/Concurrency/transfernonsendable.swift

@@ -1767,3 +1767,30 @@ extension MyActor {
     }
   }
 }
+
+public struct TimeoutError: Error, CustomStringConvertible {
+  public var description: String { "Timed out" }
+}
+
+// We used to not merge the isolation below correctly causing us to emit a crash
+// due to undefined behavior. Make sure we do not crash or emit an unhandled
+// pattern error.
+public func doNotCrashOrEmitUnhandledPatternErrors<T: Sendable>(
+  _ duration: Duration,
+  _ body: @escaping @Sendable () async throws -> T
+) async throws -> T {
+  try await withThrowingTaskGroup(of: T.self) { taskGroup in
+    taskGroup.addTask {
+      try await Task.sleep(for: duration)
+      throw TimeoutError()
+    }
+    taskGroup.addTask {
+      return try await body()
+    }
+    for try await value in taskGroup {
+      taskGroup.cancelAll()
+      return value
+    }
+    throw CancellationError()
+  }
+}