Merge pull request #80863 from gottesmm/pr-83b58369a3d1ab17d10b9f0f2792801d804d6f81

[rbi] When checking for partial apply reachability of a value at a user, include the user itself in case the user is the actual partial apply

Author: gottesmm

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -1264,6 +1264,9 @@ bool PartialApplyReachabilityDataflow::isReachable(SILValue value,
 
   // We were not reachable at entry but are at our exit... walk the block and
   // see if our user is before a gen instruction.
+
+  // To do so, first find the range of pairs in valueToGenInsts that contains
+  // our base value.
   auto genStart = std::lower_bound(
       valueToGenInsts.begin(), valueToGenInsts.end(),
       std::make_pair(baseValue, nullptr),
@@ -1272,20 +1275,29 @@ bool PartialApplyReachabilityDataflow::isReachable(SILValue value,
   if (genStart == valueToGenInsts.end() || genStart->first != baseValue)
     return false;
 
+  // Then determine the end of that range.
   auto genEnd = genStart;
   while (genEnd != valueToGenInsts.end() && genEnd->first == baseValue)
     ++genEnd;
 
   // Walk forward from the beginning of the block to user. If we do not find a
   // gen instruction, then we know the gen occurs after the op.
+  //
+  // NOTE: It is important that we include the user within our range since our
+  // user might also be a gen instruction.
   return llvm::any_of(
-      user->getParent()->getRangeEndingAtInst(user), [&](SILInstruction &inst) {
+      llvm::make_range(user->getParent()->begin(),
+                       std::next(user->getIterator())), [&](SILInstruction &inst) {
+        // Our computation works by computing a lower bound and seeing if we
+        // have (baseValue, inst) within our sorted range.
         auto iter = std::lower_bound(
             genStart, genEnd, std::make_pair(baseValue, &inst),
             [](const std::pair<SILValue, SILInstruction *> &p1,
                const std::pair<SILValue, SILInstruction *> &p2) {
               return p1 < p2;
             });
+
+        // If we did not hit end and found our pair, then we are done.
         return iter != valueToGenInsts.end() && iter->first == baseValue &&
                iter->second == &inst;
       });

test/Concurrency/dispatch_inference.swift

@@ -1,11 +1,8 @@
-// RUN: %target-swift-frontend -target %target-swift-5.1-abi-triple %import-libdispatch -strict-concurrency=complete %s -emit-sil -o /dev/null -verify
 // RUN: %target-swift-frontend -target %target-swift-5.1-abi-triple %import-libdispatch -strict-concurrency=complete %s -emit-sil -o /dev/null -verify -strict-concurrency=targeted
-// RUN: %target-swift-frontend -target %target-swift-5.1-abi-triple %import-libdispatch -strict-concurrency=complete %s -emit-sil -o /dev/null -verify -strict-concurrency=complete
-// RUN: %target-swift-frontend -target %target-swift-5.1-abi-triple %import-libdispatch -strict-concurrency=complete %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -enable-upcoming-feature RegionBasedIsolation
+// RUN: %target-swift-frontend -target %target-swift-5.1-abi-triple %import-libdispatch -strict-concurrency=complete %s -emit-sil -o /dev/null -verify -verify-additional-prefix tns-
 
 // REQUIRES: concurrency
 // REQUIRES: libdispatch
-// REQUIRES: swift_feature_RegionBasedIsolation
 
 import Dispatch
 
@@ -27,8 +24,10 @@ func testUnsafeSendableInMainAsync() async {
   var x = 5
   DispatchQueue.main.async {
     x = 17 // expected-warning{{mutation of captured var 'x' in concurrently-executing code}}
+    // expected-tns-warning @-1 {{sending 'x' risks causing data races}}
+    // expected-tns-note @-2 {{'x' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later nonisolated uses}}
   }
-  print(x)
+  print(x) // expected-tns-note {{access can happen concurrently}}
 }
 
 func testUnsafeSendableInAsync(queue: DispatchQueue) async {

test/Concurrency/transfernonsendable.swift

@@ -1997,3 +1997,13 @@ func localCaptureDataRace3() async {
 
   print(x)
 }
+
+nonisolated func localCaptureDataRace4() {
+  var x = 0
+  _ = x
+
+  Task.detached { @MainActor in x = 1 } // expected-tns-warning {{sending 'x' risks causing data races}}
+  // expected-tns-note @-1 {{'x' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later nonisolated uses}}
+
+  x = 2 // expected-tns-note {{access can happen concurrently}}
+}

test/Concurrency/transfernonsendable_global_actor.swift

@@ -329,3 +329,14 @@ struct Clock {
   // expected-complete-warning @-2 {{passing argument of non-sendable type '() -> ()' into global actor 'CustomActor'-isolated context may introduce data races}}
   // expected-complete-note @-3 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
 }
+
+@MainActor
+func localCaptureDataRace5() {
+  var x = 0
+  _ = x
+
+  Task.detached { @CustomActor in x = 1 } // expected-tns-warning {{sending 'x' risks causing data races}}
+  // expected-tns-note @-1 {{'x' is captured by a global actor 'CustomActor'-isolated closure. global actor 'CustomActor'-isolated uses in closure may race against later main actor-isolated uses}}
+
+  x = 2 // expected-tns-note {{access can happen concurrently}}
+}