Merge pull request #75114 from gottesmm/pr-f98009de45079043f0a356d020d8110306fb159f

[region-isolation] Be more aggressive about not looking through Sendable values when getting underlying objects.

Author: gottesmm

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -378,6 +378,11 @@ static SILValue getUnderlyingTrackedObjectValue(SILValue value) {
 }
 
 static UnderlyingTrackedValueInfo getUnderlyingTrackedValue(SILValue value) {
+  // Before a check if the value we are attempting to access is Sendable. In
+  // such a case, just return early.
+  if (!SILIsolationInfo::isNonSendableType(value))
+    return UnderlyingTrackedValueInfo(value);
+
   // Look through a project_box, so that we process it like its operand object.
   if (auto *pbi = dyn_cast<ProjectBoxInst>(value)) {
     value = pbi->getOperand();
@@ -386,6 +391,7 @@ static UnderlyingTrackedValueInfo getUnderlyingTrackedValue(SILValue value) {
   if (!value->getType().isAddress()) {
     SILValue underlyingValue = getUnderlyingTrackedObjectValue(value);
 
+    // If we do not have a load inst, just return the value.
     if (!isa<LoadInst, LoadBorrowInst>(underlyingValue)) {
       return UnderlyingTrackedValueInfo(underlyingValue);
     }

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

@@ -1131,9 +1131,11 @@ void SILIsolationInfo::printForOneLineLogging(llvm::raw_ostream &os) const {
 // NOTE: We special case RawPointer and NativeObject to ensure they are
 // treated as non-Sendable and strict checking is applied to it.
 bool SILIsolationInfo::isNonSendableType(SILType type, SILFunction *fn) {
-  // Treat Builtin.NativeObject and Builtin.RawPointer as non-Sendable.
+  // Treat Builtin.NativeObject, Builtin.RawPointer, and Builtin.BridgeObject as
+  // non-Sendable.
   if (type.getASTType()->is<BuiltinNativeObjectType>() ||
-      type.getASTType()->is<BuiltinRawPointerType>()) {
+      type.getASTType()->is<BuiltinRawPointerType>() ||
+      type.getASTType()->is<BuiltinBridgeObjectType>()) {
     return true;
   }
 

test/Concurrency/async_task_groups.swift

@@ -201,7 +201,6 @@ extension Collection where Self: Sendable, Element: Sendable, Self.Index: Sendab
         // TODO: When we have isolation history, isolation history will be able
         // to tell us what is going on.
         group.addTask { [submitted,i] in // expected-error {{escaping closure captures non-escaping parameter 'transform'}}
-          // expected-tns-warning @-1 {{task-isolated value of type '() async throws -> SendableTuple2<Int, T>' passed as a strongly transferred parameter}}
           let _ = try await transform(self[i]) // expected-note {{captured here}}
           let value: T? = nil
           return SendableTuple2(submitted, value!)

test/Concurrency/transfernonsendable.swift

@@ -18,6 +18,7 @@ class NonSendableKlass { // expected-complete-note 53{{}}
   // expected-typechecker-only-note @-1 3{{}}
   // expected-tns-note @-2 {{}}
   var field: NonSendableKlass? = nil
+  var boolean: Bool = false
 
   init() {}
   init(_ x: NonSendableKlass) {
@@ -1331,9 +1332,9 @@ func varSendableNonTrivialLetTupleFieldTest() 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 '(Int, SendableKlass, NonSendableKlass)' into main actor-isolated context may introduce data races}}
-  let z = test.1 // expected-tns-note {{access can happen concurrently}}
+  let z = test.1
   useValue(z)
-  useValue(test)
+  useValue(test) // expected-tns-note {{access can happen concurrently}}
 }
 
 func varNonSendableNonTrivialLetTupleFieldTest() async {
@@ -1823,3 +1824,14 @@ func testThatGlobalActorTakesPrecedenceOverActorIsolationOnMethods() async {
   // Meaning we would get an error here.
   Task { @MainActor in print(ns) }
 }
+
+// Shouldn't get any errors from x.
+//
+// We used to look through the access to x.boolean and think that the closure
+// was capturing x instead of y.
+func testBooleanCapture(_ x: inout NonSendableKlass) {
+  let y = x.boolean
+  Task.detached { @MainActor [z = y] in
+    print(z)
+  }
+}