Merge pull request #75541 from gottesmm/pr-7133549c90f98b7c9be71f80b74a97d088aa62f2

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

Author: gottesmm

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -2891,9 +2891,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;                                       \
   }
 
@@ -2922,10 +2919,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

@@ -878,8 +878,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 {
@@ -947,20 +947,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)"
+    }
+  }
+}