[region-isolation] Do not ignore non-trivial results that are Sendable to be more permissive in the face of lazy typechecker issues.

We have found certain cases due to the requestified typechecker, a type is
initially Sendable and then is later non-Sendable. This can be seen by the
attached test case where the first time one calls isNonSendableType on the test
value, one would get that it is Sendable and then the second time one would get
it was non-Sendable. The result of this is that the pass gets into an
inconsistent state.

This patch is a small patch that makes the pass more permissive in the face of
such an error by making it so that we do not ignore Sendable results of
instructions (that is we make sure to track a value for them), so we do not
break invariants.

The longer term better fix is to make it so that we have a cache in the pass for
this query that way we just always use the first answer returned from the
typechecker and cache that. If the typechecker has such a bug, we may get bogus
results, but we at least do not break invariants.

As an example of this type of behavior, in the test case in this patch, we first
find the Sendable conformance of MySubClass and then the typechecker after doing
some more type checking while performing that query, the second time finds the
inherited non-Sendable conformance of MyParentClass causing MySubClass to be
considered to be non-Sendable.

rdar://132347404
(cherry picked from commit 8604480)

Author: gottesmm

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -2888,9 +2888,6 @@ CONSTANT_TRANSLATION(VectorInst, Asserting)
 
 #define IGNORE_IF_SENDABLE_RESULT_ASSIGN_OTHERWISE(INST)                       \
   TranslationSemantics PartitionOpTranslator::visit##INST(INST *inst) {        \
-    if (!isNonSendableType(inst->getType())) {                                 \
-      return TranslationSemantics::Ignored;                                    \
-    }                                                                          \
     return TranslationSemantics::Assign;                                       \
   }
 
@@ -2919,10 +2916,18 @@ IGNORE_IF_SENDABLE_RESULT_ASSIGN_OTHERWISE(StructExtractInst)
       return TranslationSemantics::Require;                                    \
     }                                                                          \
                                                                                \
-    if (isResultNonSendable)                                                   \
-      return TranslationSemantics::AssignFresh;                                \
-                                                                               \
-    return TranslationSemantics::Ignored;                                      \
+    /* We always use assign fresh here regardless of whether or not a */       \
+    /* type is sendable.                                              */       \
+    /*                                                                */       \
+    /* DISCUSSION: Since the typechecker is lazy, if there is a bug   */       \
+    /* that causes us to not look up enough type information it is    */       \
+    /* possible for a type to move from being Sendable to being       */       \
+    /* non-Sendable. For example, we could call isNonSendableType and */       \
+    /* get that the type is non-Sendable, but as a result of calling  */       \
+    /* that API, the type checker could get more information that the */       \
+    /* next time we call isNonSendableType, we will get that the type */       \
+    /* is non-Sendable.                                               */       \
+    return TranslationSemantics::AssignFresh;                                  \
   }
 
 LOOKTHROUGH_IF_NONSENDABLE_RESULT_AND_OPERAND(UncheckedTrivialBitCastInst)

test/Concurrency/transfernonsendable.swift

@@ -872,8 +872,8 @@ func letSendableTrivialLetStructFieldTest() async {
   await transferToMain(test) // expected-tns-warning {{sending 'test' risks causing data races}}
   // expected-tns-note @-1 {{sending 'test' to main actor-isolated global function 'transferToMain' risks causing data races between main actor-isolated and local nonisolated uses}}
   // expected-complete-warning @-2 {{passing argument of non-sendable type 'StructFieldTests' into main actor-isolated context may introduce data races}}
-  _ = test.letSendableTrivial
-  useValue(test) // expected-tns-note {{access can happen concurrently}}
+  _ = test.letSendableTrivial // expected-tns-note {{access can happen concurrently}}
+  useValue(test)
 }
 
 func letSendableNonTrivialLetStructFieldTest() async {
@@ -941,20 +941,20 @@ func letNonSendableNonTrivialLetStructFieldClosureTest() async {
   await transferToMain(test) // expected-tns-warning {{sending 'test' risks causing data races}}
   // expected-tns-note @-1 {{sending 'test' to main actor-isolated global function 'transferToMain' risks causing data races between main actor-isolated and local nonisolated uses}}
   // expected-complete-warning @-2 {{passing argument of non-sendable type 'StructFieldTests' into main actor-isolated context may introduce data races}}
-  let z = test.letSendableNonTrivial
+  let z = test.letSendableNonTrivial // expected-tns-note {{access can happen concurrently}}
   _ = z
   let z2 = test.varSendableNonTrivial
   _ = z2
-  useValue(test) // expected-tns-note {{access can happen concurrently}}
+  useValue(test)
 }
 
 func varSendableTrivialLetStructFieldTest() async {
   let test = StructFieldTests()
   await transferToMain(test) // expected-tns-warning {{sending 'test' risks causing data races}}
   // expected-tns-note @-1 {{sending 'test' to main actor-isolated global function 'transferToMain' risks causing data races between main actor-isolated and local nonisolated uses}}
   // expected-complete-warning @-2 {{passing argument of non-sendable type 'StructFieldTests' into main actor-isolated context may introduce data races}}
-  _ = test.varSendableTrivial
-  useValue(test) // expected-tns-note {{access can happen concurrently}}
+  _ = test.varSendableTrivial // expected-tns-note {{access can happen concurrently}}
+  useValue(test)
 }
 
 func varSendableNonTrivialLetStructFieldTest() async {

test/Concurrency/transfernonsendable_typecheckerbugs.swift

@@ -0,0 +1,84 @@
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/src)
+// RUN: %empty-directory(%t/sdk)
+// RUN: %empty-directory(%t/sdk/ObjCAPI)
+// RUN: %empty-directory(%t/sdk/SwiftAPI)
+// RUN: %empty-directory(%t/compile)
+// RUN: split-file %s %t/src
+
+// Build Objective-C lib
+// RUN: %target-clang -dynamiclib %t/src/ObjCAPI.m -I %t/src -o %t/sdk/ObjCAPI/libObjCAPI.dylib -lobjc
+// RUN: cp %t/src/ObjCAPI.modulemap %t/sdk/ObjCAPI/module.modulemap
+// RUN: cp %t/src/ObjCAPI.h %t/sdk/ObjCAPI/ObjCAPI.h
+
+// Build the swift part of API
+// RUN: %target-swiftc_driver -emit-library -emit-module-path %t/sdk/SwiftAPI/SwiftAPI.swiftmodule %t/src/SwiftAPI.swift -parse-as-library -I %t/sdk -swift-version 6 -module-name SwiftAPI -enable-library-evolution
+
+// Now compile against the API.
+// RUN: %target-swift-frontend -emit-sil -o /dev/null -I %t/sdk/SwiftAPI -I %t/sdk/ObjCAPI %t/src/main.swift -swift-version 6
+
+// REQUIRES: asserts
+// REQUIRES: concurrency
+// REQUIRES: objc_interop
+
+// Test
+
+//--- ObjCAPI.modulemap
+
+module ObjCAPI {
+  header "ObjCAPI.h"
+}
+
+//--- ObjCAPI.h
+
+#define NS_SWIFT_SENDABLE __attribute__((swift_attr("@Sendable")))
+#define NS_SWIFT_NONSENDABLE __attribute__((swift_attr("@_nonSendable")))
+
+NS_SWIFT_NONSENDABLE
+@interface MyParentClass
+@end
+
+NS_SWIFT_SENDABLE
+@interface MySubClass : MyParentClass
+@end
+
+//--- ObjCAPI.m
+
+#include "ObjCAPI.h"
+
+@implementation MyParentClass
+@end
+
+@implementation MySubClass
+@end
+
+//--- SwiftAPI.swift
+
+@_exported import ObjCAPI // Clang module
+
+public struct Measurement<T : MyParentClass> {
+  /// The unit component of the `Measurement`.
+  public let unit: T
+
+  public var count: Int { 0 }
+}
+
+extension Measurement : Sendable where T : Sendable {}
+
+//--- main.swift
+
+public import SwiftAPI
+
+public enum MarketplaceKitError : Sendable {
+  case first
+  case second(Measurement<MySubClass>)
+
+  public var description: String {
+    switch self {
+    case .first:
+      return "first"
+    case .second(let value):
+      return "\(value.count)"
+    }
+  }
+}