[region-isolation] Eliminate the last non-SIL test case usage of SILIsolationInfo::getWithIsolationCrossing().

The reason that I am changing this code is that getWithIsolationCrossing is a
bad API that was being used to infer actor isolation straight from an ApplyExpr
without adding an actor instance. This can cause us to reject programs
unnecessarily if we in other parts of the code correctly infer the SILValue
actor instance for the isolation.

Rather than allow for that, I am removing this code and I improved the rest of
the pattern matching here to ensure that we handled that with the normal actor
instance inferring code. This will prevent this type of mismerge from happening
by mistake. I fixed up the changes in the test cases.

The only usage of this left is for ApplyIsolationCrossings parsed straight from
SIL that we use only when testing. This is safe since if a test writer is using
the parsed SIL in this manner, they can make sure that mismerges do not happen.

Author: gottesmm

include/swift/SILOptimizer/Utils/SILIsolationInfo.h

@@ -346,18 +346,6 @@ class SILIsolationInfo {
             Flag::UnappliedIsolatedAnyParameter};
   }
 
-  /// 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) {
@@ -463,6 +451,20 @@ class SILIsolationInfo {
 
 private:
   void printOptions(llvm::raw_ostream &os) const;
+
+  /// This is used only to let through apply isolation crossings that we define
+  /// in SIL just for testing. Do not use this in any other contexts!
+  static SILIsolationInfo
+  getWithIsolationCrossing(ApplyIsolationCrossing crossing) {
+    if (!crossing.getCalleeIsolation().isActorIsolated())
+      return {};
+
+    // SIL level, just let it through without an actor instance. We assume since
+    // we are using this for SIL tests that we do not need to worry about having
+    // a null actor instance.
+    return SILIsolationInfo(SILValue(), SILValue(),
+                            crossing.getCalleeIsolation());
+  }
 };
 
 /// A SILIsolationInfo that has gone through merging and represents the dynamic

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

@@ -358,6 +358,11 @@ inferIsolationInfoForTempAllocStack(AllocStackInst *asi) {
 
 SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
   if (auto fas = FullApplySite::isa(inst)) {
+    // Check if we have SIL based "faked" isolation crossings that we use for
+    // testing purposes.
+    //
+    // NOTE: Please do not use getWithIsolationCrossing in more places! We only
+    // want to use it here!
     if (auto crossing = fas.getIsolationCrossing()) {
       if (auto info = SILIsolationInfo::getWithIsolationCrossing(*crossing))
         return info;
@@ -754,19 +759,6 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
     }
   }
 
-  // Try to infer using SIL first since we might be able to get the source name
-  // of the actor.
-  if (ApplyExpr *apply = inst->getLoc().getAsASTNode<ApplyExpr>()) {
-    if (auto crossing = apply->getIsolationCrossing()) {
-      if (auto info = SILIsolationInfo::getWithIsolationCrossing(*crossing))
-        return info;
-
-      if (crossing->getCalleeIsolation().isNonisolated()) {
-        return SILIsolationInfo::getDisconnected(false /*nonisolated(unsafe)*/);
-      }
-    }
-  }
-
   if (auto *asi = dyn_cast<AllocStackInst>(inst)) {
     if (asi->isFromVarDecl()) {
       if (auto *varDecl = asi->getLoc().getAsASTNode<VarDecl>()) {
@@ -801,6 +793,23 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
     }
   }
 
+  // Check if we have an ApplyInst with nonisolated.
+  //
+  // NOTE: We purposely avoid using other isolation info from an ApplyExpr since
+  // when we use the isolation crossing on the ApplyExpr at this point,w e are
+  // unable to find out what the appropriate instance is (since we would have
+  // found it earlier if we could). This ensures that we can eliminate a case
+  // where we get a SILIsolationInfo with actor isolation and without a SILValue
+  // actor instance. This prevents a class of bad SILIsolationInfo merge errors
+  // caused by the actor instances not matching.
+  if (ApplyExpr *apply = inst->getLoc().getAsASTNode<ApplyExpr>()) {
+    if (auto crossing = apply->getIsolationCrossing()) {
+      if (crossing->getCalleeIsolation().isNonisolated()) {
+        return SILIsolationInfo::getDisconnected(false /*nonisolated(unsafe)*/);
+      }
+    }
+  }
+
   return SILIsolationInfo();
 }