[region-isolation] Treat async let as a isolation inference boundary closure and fix diagnostics due to change.

Otherwise, we will assume that an async let autoclosure infers isolation from
its DeclContext... which we do not want. An async let autoclosure should always
be nonisolated + sending.

The diagnostic change that I mentioned in the header is that we were emitting
unfortunate "sending task or actor isolated could result in races" error. I
eliminated this by adding a new diagnostic for transfer non transferrable errors
happening in autoclosures. So now we emit this:

```swift
  func asyncLetInferAsNonIsolated<T : Actor>(
    isolation actor: isolated T
  ) async throws {
    async let subTask: Void = {
      await useValueAsyncNoReturnWithInstance(self, actor)
      // expected-warning @-1:47 {{sending 'self' risks causing data races}}
      // expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
    }()
    await subTask
```

I also noticed that we did not have enough test cases for autoclosures in
general so I also added a bunch of tests just so we can see what the current
behavior is. I think there are a few issues therein (I believe some may have
been reported due to '??').

rdar://130151318
(cherry picked from commit 03b26fd)

Author: gottesmm

include/swift/AST/DiagnosticsSIL.def

@@ -1022,6 +1022,10 @@ NOTE(regionbasedisolation_named_isolated_closure_yields_race, none,
       "%0%1 is captured by a %2 closure. %2 uses in closure may race against later %3 uses",
       (StringRef, Identifier, ActorIsolation, ActorIsolation))
 
+NOTE(regionbasedisolation_named_transfer_nt_asynclet_capture, none,
+      "sending %1 %0 into async let risks causing data races between nonisolated and %1 uses",
+      (Identifier, StringRef))
+
 // Misc Error.
 ERROR(regionbasedisolation_task_or_actor_isolated_transferred, none,
       "task or actor isolated value cannot be sent", ())

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

@@ -1216,6 +1216,10 @@ class TransferNonTransferrableDiagnosticEmitter {
     return getTransferringApplyCalleeInfo(info.transferredOperand->getUser());
   }
 
+  SILLocation getLoc() const {
+    return info.transferredOperand->getUser()->getLoc();
+  }
+
   /// Return the isolation region info for \p getNonTransferrableValue().
   SILDynamicMergedIsolationInfo getIsolationRegionInfo() const {
     return info.isolationRegionInfo;
@@ -1291,6 +1295,24 @@ class TransferNonTransferrableDiagnosticEmitter {
         .limitBehaviorIf(getBehaviorLimit());
   }
 
+  void emitNamedAsyncLetCapture(SILLocation loc, Identifier name,
+                                SILIsolationInfo transferredValueIsolation) {
+    assert(!getIsolationRegionInfo().isDisconnected() &&
+           "Should never be disconnected?!");
+    emitNamedOnlyError(loc, name);
+
+    SmallString<64> descriptiveKindStr;
+    {
+      llvm::raw_svector_ostream os(descriptiveKindStr);
+      getIsolationRegionInfo().printForDiagnostics(os);
+    }
+
+    diagnoseNote(loc,
+                 diag::regionbasedisolation_named_transfer_nt_asynclet_capture,
+                 name, descriptiveKindStr)
+        .limitBehaviorIf(getBehaviorLimit());
+  }
+
   void emitNamedIsolation(SILLocation loc, Identifier name,
                           ApplyIsolationCrossing isolationCrossing) {
     emitNamedOnlyError(loc, name);
@@ -1401,6 +1423,8 @@ class TransferNonTransferrableDiagnosticEmitter {
 };
 
 class TransferNonTransferrableDiagnosticInferrer {
+  struct AutoClosureWalker;
+
   TransferNonTransferrableDiagnosticEmitter diagnosticEmitter;
 
 public:
@@ -1453,6 +1477,74 @@ bool TransferNonTransferrableDiagnosticInferrer::initForIsolatedPartialApply(
   return false;
 }
 
+/// This walker visits an AutoClosureExpr and looks for uses of a specific
+/// captured value. We want to error on the uses in the autoclosure.
+struct TransferNonTransferrableDiagnosticInferrer::AutoClosureWalker
+    : ASTWalker {
+  TransferNonTransferrableDiagnosticEmitter &foundTypeInfo;
+  ValueDecl *targetDecl;
+  SILIsolationInfo targetDeclIsolationInfo;
+  SmallPtrSet<Expr *, 8> visitedCallExprDeclRefExprs;
+  SILLocation captureLoc;
+  bool isAsyncLet;
+
+  AutoClosureWalker(TransferNonTransferrableDiagnosticEmitter &foundTypeInfo,
+                    ValueDecl *targetDecl,
+                    SILIsolationInfo targetDeclIsolationInfo,
+                    SILLocation captureLoc, bool isAsyncLet)
+      : foundTypeInfo(foundTypeInfo), targetDecl(targetDecl),
+        targetDeclIsolationInfo(targetDeclIsolationInfo),
+        captureLoc(captureLoc), isAsyncLet(isAsyncLet) {}
+
+  Expr *lookThroughArgExpr(Expr *expr) {
+    while (true) {
+      if (auto *memberRefExpr = dyn_cast<MemberRefExpr>(expr)) {
+        expr = memberRefExpr->getBase();
+        continue;
+      }
+
+      if (auto *cvt = dyn_cast<ImplicitConversionExpr>(expr)) {
+        expr = cvt->getSubExpr();
+        continue;
+      }
+
+      if (auto *e = dyn_cast<ForceValueExpr>(expr)) {
+        expr = e->getSubExpr();
+        continue;
+      }
+
+      if (auto *t = dyn_cast<TupleElementExpr>(expr)) {
+        expr = t->getBase();
+        continue;
+      }
+
+      return expr;
+    }
+  }
+
+  PreWalkResult<Expr *> walkToExprPre(Expr *expr) override {
+    if (auto *declRef = dyn_cast<DeclRefExpr>(expr)) {
+      // If this decl ref expr was not visited as part of a callExpr and is our
+      // target decl... emit a simple async let error.
+      //
+      // This occurs if we do:
+      //
+      // ```
+      // let x = ...
+      // async let y = x
+      // ```
+      if (declRef->getDecl() == targetDecl) {
+        foundTypeInfo.emitNamedAsyncLetCapture(captureLoc,
+                                               targetDecl->getBaseIdentifier(),
+                                               targetDeclIsolationInfo);
+        return Action::Continue(expr);
+      }
+    }
+
+    return Action::Continue(expr);
+  }
+};
+
 bool TransferNonTransferrableDiagnosticInferrer::run() {
   // We need to find the isolation info.
   auto *op = diagnosticEmitter.getOperand();
@@ -1569,6 +1661,26 @@ bool TransferNonTransferrableDiagnosticInferrer::run() {
     }
   }
 
+  // If we are failing due to an autoclosure... see if we can find the captured
+  // value that is causing the issue.
+  if (auto *autoClosureExpr = loc.getAsASTNode<AutoClosureExpr>()) {
+    // To split up this work, we only do this for async let for now.
+    if (autoClosureExpr->getThunkKind() == AutoClosureExpr::Kind::AsyncLet) {
+      auto *i = op->getUser();
+      auto pai = ApplySite::isa(i);
+      unsigned captureIndex = pai.getAppliedArgIndex(*op);
+      auto captureInfo =
+          autoClosureExpr->getCaptureInfo().getCaptures()[captureIndex];
+      auto loc = RegularLocation(captureInfo.getLoc(), false /*implicit*/);
+      AutoClosureWalker walker(
+          diagnosticEmitter, captureInfo.getDecl(),
+          diagnosticEmitter.getIsolationRegionInfo().getIsolationInfo(), loc,
+          autoClosureExpr->getThunkKind() == AutoClosureExpr::Kind::AsyncLet);
+      autoClosureExpr->walk(walker);
+      return true;
+    }
+  }
+
   diagnosticEmitter.emitUnknownUse(loc);
   return true;
 }

lib/Sema/TypeCheckConcurrency.cpp

@@ -741,6 +741,13 @@ isIsolationInferenceBoundaryClosure(const AbstractClosureExpr *closure,
     }
   }
 
+  // An autoclosure for an async let acts as a boundary. It is non-Sendable
+  // regardless of its context.
+  if (auto *autoclosure = dyn_cast<AutoClosureExpr>(closure)) {
+    if (autoclosure->getThunkKind() == AutoClosureExpr::Kind::AsyncLet)
+      return true;
+  }
+
   return isSendableClosure(closure, forActorIsolation);
 }
 

test/Concurrency/global_actor_function_types.swift

@@ -327,9 +327,12 @@ func stripActor(_ expr: @Sendable @autoclosure () -> (() -> ())) async {
   return await stripActor(mainActorFn) // expected-warning {{converting function value of type '@MainActor () -> ()' to '() -> ()' loses global actor 'MainActor'}}
 }
 
-// NOTE: this warning is correct, but is only being caught by TypeCheckConcurrency's extra check.
+// We used to not emit an error here with strict-concurrency enabled since we
+// were inferring the async let to main actor isolated (which was incorrect). We
+// now always treat async let as non-isolated, so we get the same error in all
+// contexts.
 @MainActor func exampleWhereConstraintSolverHasWrongDeclContext_v2() async -> Int {
-  async let a: () = noActor(mainActorFn) // expected-without-transferring-warning {{converting function value of type '@MainActor () -> ()' to '() -> ()' loses global actor 'MainActor'}}
+  async let a: () = noActor(mainActorFn) // expected-warning {{converting function value of type '@MainActor () -> ()' to '() -> ()' loses global actor 'MainActor'}}
   await a
 }
 

test/Concurrency/issue-57376.swift

@@ -26,47 +26,53 @@ func testAsyncSequence1Sendable<Seq: AsyncSequence>(_ seq: Seq) async throws whe
 
 func testAsyncSequenceTypedPattern<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{54-54=, Sendable}}
   // expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{54-54=, Sendable}}
-  async let result: Int = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
-  // expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
-  // expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
+  async let result: Int = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
+  // expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
+  // expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
+  // expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
   let _ = try! await result
 }
 
 func testAsyncSequenceTypedPattern1<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{55-55=, Sendable}}
   // expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{55-55=, Sendable}}
-  async let _: Int = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
-  // expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
-  // expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
+  async let _: Int = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
+  // expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
+  // expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
+  // expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
 }
 
 func testAsyncSequence<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{42-42=, Sendable}}
   // expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{42-42=, Sendable}}
-  async let result = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
-  // expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
-  // expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
+  async let result = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
+  // expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
+  // expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
+  // expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
   let _ = try! await result
 }
 
 func testAsyncSequence1<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{43-43=, Sendable}}
   // expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{43-43=, Sendable}}
-  async let _ = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
-  // expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
-  // expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
+  async let _ = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
+  // expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
+  // expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
+  // expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
 }
 
 func testAsyncSequence3<Seq>(_ seq: Seq) async throws where Seq: AsyncSequence, Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}}
   // expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}}
-  async let result = seq // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
-  // expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
-  // expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
+  async let result = seq // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
+  // expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
+  // expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
+  // expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
   let _ = await result
 }
 
 func testAsyncSequence4<Seq>(_ seq: Seq) async throws where Seq: AsyncSequence, Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}}
   // expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}}
-  async let _ = seq // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
-  // expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
-  // expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
+  async let _ = seq // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
+  // expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
+  // expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
+  // expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
 }
 
 func search(query: String, entities: [String]) async throws -> [String] {

test/Concurrency/transfernonsendable_asynclet.swift

@@ -18,7 +18,11 @@ class NonSendableKlass {
 
 class SendableKlass : @unchecked Sendable {}
 
-actor Actor {
+struct NonSendableStruct { // expected-note {{}}
+  var ns = NonSendableKlass()
+}
+
+actor MyActor {
   var klass = NonSendableKlass()
   final var finalKlass = NonSendableKlass()
 
@@ -28,7 +32,7 @@ actor Actor {
   func useNonSendableFunction(_: () -> Void) {}
 }
 
-final actor FinalActor {
+final actor FinalMyActor {
   var klass = NonSendableKlass()
   func useKlass(_ x: NonSendableKlass) {}
 }
@@ -38,6 +42,13 @@ func useInOut<T>(_ x: inout T) {}
 func useValue<T>(_ x: T) -> T { x }
 func useValueWrapInOptional<T>(_ x: T) -> T? { x }
 
+func useValueNoReturnWithInstance<T, V : Actor>(_ x: T, _ y: V) -> () { fatalError() }
+func useValueAsyncNoReturnWithInstance<T, V : Actor>(_ x: T, _ y: V) async -> () { fatalError() }
+@MainActor
+func useMainActorValueAsyncNoReturn<T>(_ x: T) async -> () { fatalError() }
+@MainActor
+func useMainActorValueNoReturn<T>(_ x: T) -> () { fatalError() }
+
 @MainActor func returnValueFromMain<T>() async -> T { fatalError() }
 @MainActor func transferToMain<T>(_ t: T) async {}
 @MainActor func transferToMainInt<T>(_ t: T) async -> Int { 5 }
@@ -722,11 +733,50 @@ func asyncLetWithoutCapture() async {
 }
 
 func asyncLet_Let_ActorIsolated_Method() async {
-  let a = Actor()
+  let a = MyActor()
   let x = NonSendableKlass()
   async let y = a.useKlass(x) // expected-warning {{sending 'x' risks causing data races}}
   // expected-note @-1 {{sending 'x' to actor-isolated instance method 'useKlass' risks causing data races between actor-isolated and local nonisolated uses}}
 
   useValue(x) // expected-note {{access can happen concurrently}}
   let _ = await y
 }
+
+extension NonSendableStruct {
+  func asyncLetInferAsNonIsolated<T : Actor>(
+    isolation actor: isolated T
+  ) async throws {
+    async let subTask: Void = {
+      await useValueAsyncNoReturnWithInstance(self, actor)
+      // expected-warning @-1:47 {{sending 'self' risks causing data races}}
+      // expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
+    }()
+    await subTask
+
+    async let subTask2: () = await useValueAsyncNoReturnWithInstance(self, actor)
+    // expected-warning @-1 {{sending 'self' risks causing data races}}
+    // expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
+    await subTask2
+
+    async let subTask3: () = useValueNoReturnWithInstance(self, actor)
+    // expected-warning @-1 {{sending 'self' risks causing data races}}
+    // expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
+    await subTask3
+
+    async let subTask4: () = await useMainActorValueAsyncNoReturn(self)
+    // expected-warning @-1 {{sending 'self' risks causing data races}}
+    // expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
+    await subTask4
+
+    async let subTask5: () = useMainActorValueNoReturn(self)
+    // expected-warning @-1 {{sending 'self' risks causing data races}}
+    // expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
+    await subTask5
+
+    async let subTask6: NonSendableStruct = self
+    // expected-warning @-1 {{sending 'self' risks causing data races}}
+    // expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
+    // expected-warning @-3 {{non-sendable type 'NonSendableStruct' returned by implicitly asynchronous call to nonisolated function cannot cross actor boundary}}
+    _ = await subTask6
+  }
+}