Merge pull request #70750 from hborla/5.10-suppress-async-iterator-warning

[5.10][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

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

lib/AST/TypeCheckRequests.cpp

@@ -1624,6 +1624,7 @@ bool ActorIsolation::requiresSubstitution() const {
   switch (kind) {
   case ActorInstance:
   case Nonisolated:
+  case NonisolatedUnsafe:
   case Unspecified:
     return false;
 
@@ -1638,6 +1639,7 @@ ActorIsolation ActorIsolation::subst(SubstitutionMap subs) const {
   switch (kind) {
   case ActorInstance:
   case Nonisolated:
+  case NonisolatedUnsafe:
   case Unspecified:
     return *this;
 

lib/ASTGen/Sources/ASTGen/SourceFile.swift

@@ -36,7 +36,6 @@ extension Parser.ExperimentalFeatures {
       }
     }
     mapFeature(.ThenStatements, to: .thenStatements)
-    mapFeature(.GlobalConcurrency, to: .globalConcurrency)
   }
 }
 

lib/Parse/ParseDecl.cpp

@@ -5344,8 +5344,7 @@ bool Parser::isStartOfSwiftDecl(bool allowPoundIfAttributes,
   }
 
   // If this is 'nonisolated', check to see if it is valid.
-  if (Context.LangOpts.hasFeature(Feature::GlobalConcurrency) &&
-      Tok.isContextualKeyword("nonisolated") && Tok2.is(tok::l_paren) &&
+  if (Tok.isContextualKeyword("nonisolated") && Tok2.is(tok::l_paren) &&
       isParenthesizedNonisolated(*this)) {
     BacktrackingScope backtrack(*this);
     consumeToken(tok::identifier);

lib/Sema/CSGen.cpp

@@ -4413,6 +4413,19 @@ generateForEachStmtConstraints(ConstraintSystem &cs,
               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/TypeCheckAttr.cpp

@@ -6596,10 +6596,9 @@ void AttributeChecker::visitNonisolatedAttr(NonisolatedAttr *attr) {
   if (auto var = dyn_cast<VarDecl>(D)) {
     // stored properties have limitations as to when they can be nonisolated.
     if (var->hasStorage()) {
-      const bool isUnsafeGlobal = attr->isUnsafe() && var->isGlobalStorage();
-
-      // 'nonisolated' can not be applied to mutable stored properties.
-      if (var->supportsMutation() && !isUnsafeGlobal) {
+      // 'nonisolated' can not be applied to mutable stored properties unless
+      // qualified as 'unsafe'.
+      if (var->supportsMutation() && !attr->isUnsafe()) {
         diagnoseAndRemoveAttr(attr, diag::nonisolated_mutable_storage);
         return;
       }
@@ -6641,8 +6640,9 @@ void AttributeChecker::visitNonisolatedAttr(NonisolatedAttr *attr) {
       return;
     }
 
-    // nonisolated can not be applied to local properties.
-    if (dc->isLocalContext()) {
+    // nonisolated can not be applied to local properties unless qualified as
+    // 'unsafe'.
+    if (dc->isLocalContext() && !attr->isUnsafe()) {
       diagnoseAndRemoveAttr(attr, diag::nonisolated_local_var);
       return;
     }

lib/Sema/TypeCheckConcurrency.cpp

@@ -1947,6 +1947,15 @@ static void noteGlobalActorOnContext(DeclContext *dc, Type globalActor) {
   }
 }
 
+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())
@@ -1959,7 +1968,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, base->getStartLoc(),
             diag::non_sendable_call_argument,
@@ -1979,6 +1990,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())
@@ -1987,11 +1999,13 @@ 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()) {
-          argType = argExpr->findOriginalType();
+        checkSendable = shouldCheckSendable(argExpr);
+        argType = argExpr->findOriginalType();
       }
     }
 
-    if (diagnoseNonSendableTypes(
+    if (checkSendable &&
+        diagnoseNonSendableTypes(
             argType ? argType : param.getParameterType(),
             declContext, argLoc, diag::non_sendable_call_argument,
             isolationCrossing.value().exitsIsolation(),
@@ -2812,6 +2826,11 @@ namespace {
       if (getActorIsolation(value).isActorIsolated())
         return false;
 
+      if (auto attr = value->getAttrs().getAttribute<NonisolatedAttr>();
+          attr && attr->isUnsafe()) {
+        return false;
+      }
+
       ctx.Diags.diagnose(loc, diag::shared_mutable_state_access, value);
       value->diagnose(diag::kind_declared_here, value->getDescriptiveKind());
       return true;
@@ -3275,6 +3294,11 @@ namespace {
           }
         }
 
+        if (auto attr = var->getAttrs().getAttribute<NonisolatedAttr>();
+            attr && attr->isUnsafe()) {
+          return false;
+        }
+
         // Otherwise, we have concurrent access. Complain.
         bool preconcurrencyContext =
             getActorIsolationOfContext(

test/Concurrency/experimental_feature_strictconcurrency.swift

@@ -1,6 +1,5 @@
 // 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 SendNonSendable %s
 
 // REQUIRES: concurrency
 // REQUIRES: asserts
@@ -75,3 +74,24 @@ func f() {
   print(TestStatics.immutableInferredSendable)
   print(TestStatics.mutable) // expected-warning{{reference to static property 'mutable' is not concurrency-safe because it involves shared mutable state}}
 }
+
+func testLocalNonisolatedUnsafe() async {
+  nonisolated(unsafe) var value = 1
+  let task = Task {
+    value = 2
+    return value
+  }
+  print(await task.value)
+}
+
+@MainActor
+func iterate(stream: AsyncStream<Int>) async {
+  nonisolated(unsafe) var it = stream.makeAsyncIterator()
+  while let element = await it.next() {
+    print(element)
+  }
+
+  for await x in stream {
+    print(x)
+  }
+}