Merge pull request #70697 from hborla/nonisolated-unsafe-local

[Concurrency] Suppress diagnostics about non-`Sendable` async iterator arguments in implicit calls to `next()`.

Author: hborla

include/swift/AST/Expr.h

@@ -448,6 +448,10 @@ class alignas(8) Expr : public ASTAllocated<Expr> {
     return const_cast<Expr *>(this)->getValueProvidingExpr();
   }
 
+  /// Find the original expression value, looking through various
+  /// implicit conversions.
+  const Expr *findOriginalValue() const;
+
   /// Find the original type of a value, looking through various implicit
   /// conversions.
   Type findOriginalType() const;

lib/AST/Expr.cpp

@@ -2875,7 +2875,7 @@ FrontendStatsTracer::getTraceFormatter<const Expr *>() {
   return &TF;
 }
 
-Type Expr::findOriginalType() const {
+const Expr *Expr::findOriginalValue() const {
   auto *expr = this;
   do {
     expr = expr->getSemanticsProvidingExpr();
@@ -2898,6 +2898,11 @@ Type Expr::findOriginalType() const {
     break;
   } while (true);
 
+  return expr;
+}
+
+Type Expr::findOriginalType() const {
+  auto *expr = findOriginalValue();
   return expr->getType()->getRValueType();
 }
 

lib/Sema/CSGen.cpp

@@ -4516,6 +4516,19 @@ generateForEachStmtConstraints(ConstraintSystem &cs, DeclContext *dc,
               sequenceExpr->getStartLoc(), ctx.getIdentifier(name), dc);
   makeIteratorVar->setImplicit();
 
+  // FIXME: Apply `nonisolated(unsafe)` to async iterators.
+  //
+  // Async iterators are not `Sendable`; they're only meant to be used from
+  // the isolation domain that creates them. But the `next()` method runs on
+  // the generic executor, so calling it from an actor-isolated context passes
+  // non-`Sendable` state across the isolation boundary. `next()` should
+  // inherit the isolation of the caller, but for now, use the opt out.
+  if (isAsync) {
+    auto *nonisolated = new (ctx)
+        NonisolatedAttr(/*unsafe=*/true, /*implicit=*/true);
+    makeIteratorVar->getAttrs().add(nonisolated);
+  }
+
   // First, let's form a call from sequence to `.makeIterator()` and save
   // that in a special variable which is going to be used by SILGen.
   {

lib/Sema/TypeCheckConcurrency.cpp

@@ -1954,6 +1954,15 @@ static FuncDecl *findAnnotatableFunction(DeclContext *dc) {
   return fn;
 }
 
+static bool shouldCheckSendable(Expr *expr) {
+  if (auto *declRef = dyn_cast<DeclRefExpr>(expr->findOriginalValue())) {
+    auto isolation = getActorIsolation(declRef->getDecl());
+    return isolation != ActorIsolation::NonisolatedUnsafe;
+  }
+
+  return true;
+}
+
 bool swift::diagnoseApplyArgSendability(ApplyExpr *apply, const DeclContext *declContext) {
   auto isolationCrossing = apply->getIsolationCrossing();
   if (!isolationCrossing.has_value())
@@ -1966,7 +1975,9 @@ bool swift::diagnoseApplyArgSendability(ApplyExpr *apply, const DeclContext *dec
   // Check the 'self' argument.
   if (auto *selfApply = dyn_cast<SelfApplyExpr>(apply->getFn())) {
     auto *base = selfApply->getBase();
-    if (diagnoseNonSendableTypes(
+
+    if (shouldCheckSendable(base) &&
+        diagnoseNonSendableTypes(
             base->getType(),
             declContext, /*inDerivedConformance*/Type(),
             base->getStartLoc(),
@@ -1986,6 +1997,7 @@ bool swift::diagnoseApplyArgSendability(ApplyExpr *apply, const DeclContext *dec
     // Dig out the location of the argument.
     SourceLoc argLoc = apply->getLoc();
     Type argType;
+    bool checkSendable = true;
     if (auto argList = apply->getArgs()) {
       auto arg = argList->get(paramIdx);
       if (arg.getStartLoc().isValid())
@@ -1994,6 +2006,7 @@ bool swift::diagnoseApplyArgSendability(ApplyExpr *apply, const DeclContext *dec
       // Determine the type of the argument, ignoring any implicit
       // conversions that could have stripped sendability.
       if (Expr *argExpr = arg.getExpr()) {
+        checkSendable = shouldCheckSendable(argExpr);
         argType = argExpr->findOriginalType();
 
         // If this is a default argument expression, don't check Sendability
@@ -2008,7 +2021,8 @@ bool swift::diagnoseApplyArgSendability(ApplyExpr *apply, const DeclContext *dec
       }
     }
 
-    if (diagnoseNonSendableTypes(
+    if (checkSendable &&
+        diagnoseNonSendableTypes(
             argType ? argType : param.getParameterType(),
             declContext, /*inDerivedConformance*/Type(),
             argLoc, diag::non_sendable_call_argument,

test/Concurrency/experimental_feature_strictconcurrency.swift

@@ -1,6 +1,6 @@
 // RUN: %target-swift-frontend -disable-availability-checking -parse-as-library -enable-experimental-feature StrictConcurrency -enable-experimental-feature GlobalConcurrency -emit-sil -o /dev/null -verify %s
 // RUN: %target-swift-frontend -disable-availability-checking -parse-as-library -enable-experimental-feature StrictConcurrency=complete -enable-experimental-feature GlobalConcurrency -emit-sil -o /dev/null -verify %s
-// RUN: %target-swift-frontend -disable-availability-checking -parse-as-library -enable-experimental-feature StrictConcurrency=complete -enable-experimental-feature GlobalConcurrency -emit-sil -o /dev/null -verify -enable-experimental-feature RegionBasedIsolation %s
+// RUN: %target-swift-frontend -disable-availability-checking -parse-as-library -enable-experimental-feature StrictConcurrency=complete -enable-experimental-feature GlobalConcurrency -emit-sil -o /dev/null -verify -verify-additional-prefix region-isolation- -enable-experimental-feature RegionBasedIsolation %s
 
 // REQUIRES: concurrency
 // REQUIRES: asserts
@@ -84,3 +84,22 @@ func testLocalNonisolatedUnsafe() async {
   }
   print(await task.value)
 }
+
+@MainActor
+func iterate(stream: AsyncStream<Int>) async {
+  nonisolated(unsafe) var it = stream.makeAsyncIterator()
+  // FIXME: Region isolation should consider a value from a 'nonisolated(unsafe)'
+  // declaration to be in a disconnected region
+
+  // expected-region-isolation-warning@+2 {{passing argument of non-sendable type 'AsyncStream<Int>.Iterator' from main actor-isolated context to nonisolated context at this call site could yield a race with accesses later in this function}}
+  // expected-region-isolation-note@+1 {{access here could race}}
+  while let element = await it.next() {
+    print(element)
+  }
+
+  // expected-region-isolation-warning@+2 {{passing argument of non-sendable type 'AsyncStream<Int>.Iterator' from main actor-isolated context to nonisolated context at this call site could yield a race with accesses later in this function}}
+  // expected-region-isolation-note@+1 {{access here could race}}
+  for await x in stream {
+    print(x)
+  }
+}