Merge pull request #81633 from AnthonyLatsis/acer-campestri-2

Sema: Fix an issue with `NonisolatedNonsendingByDefault` migration fo…

Author: AnthonyLatsis

lib/Sema/NonisolatedNonsendingByDefaultMigration.cpp

@@ -21,6 +21,7 @@
 #include "swift/AST/Decl.h"
 #include "swift/AST/DiagnosticsSema.h"
 #include "swift/AST/Expr.h"
+#include "swift/AST/ParameterList.h"
 #include "swift/AST/TypeRepr.h"
 #include "swift/Basic/Assertions.h"
 #include "swift/Basic/Feature.h"
@@ -171,19 +172,81 @@ void NonisolatedNonsendingByDefaultMigrationTarget::diagnose() const {
                   featureName, functionDecl)
         .fixItInsertAttribute(
             decl->getAttributeInsertionLoc(/*forModifier=*/false), &attr);
-  } else if (closure) {
-    ctx.Diags
-        .diagnose(closure->getLoc(),
-                  diag::attr_execution_nonisolated_behavior_will_change_closure,
-                  featureName)
-        .fixItAddAttribute(&attr, closure);
-  } else {
+  } else if (functionRepr) {
     ctx.Diags
         .diagnose(
             functionRepr->getStartLoc(),
             diag::attr_execution_nonisolated_behavior_will_change_typerepr,
             featureName)
         .fixItInsertAttribute(functionRepr->getStartLoc(), &attr);
+  } else {
+    auto diag = ctx.Diags.diagnose(
+        closure->getLoc(),
+        diag::attr_execution_nonisolated_behavior_will_change_closure,
+        featureName);
+    diag.fixItAddAttribute(&attr, closure);
+
+    // The following cases fail to compile together with `@concurrent` in
+    // Swift 5 or Swift 6 mode due to parser and type checker behaviors:
+    // 1. - Explicit parameter list
+    //    - Explicit result type
+    //    - No explicit `async` effect
+    // 2. - Explicit parenthesized parameter list
+    //    - No capture list
+    //    - No explicit result type
+    //    - No explicit effect
+    //
+    // Work around these issues by adding inferred effects together with the
+    // attribute.
+
+    // If there's an explicit `async` effect, we're good.
+    if (closure->getAsyncLoc().isValid()) {
+      return;
+    }
+
+    auto *params = closure->getParameters();
+    // FIXME: We need a better way to distinguish an implicit parameter list.
+    bool hasExplicitParenthesizedParamList =
+        params->getLParenLoc().isValid() &&
+        params->getLParenLoc() != closure->getStartLoc();
+
+    // If the parameter list is implicit, we're good.
+    if (!hasExplicitParenthesizedParamList) {
+      if (params->size() == 0) {
+        return;
+      } else if ((*params)[0]->isImplicit()) {
+        return;
+      }
+    }
+
+    // At this point we must proceed if there is an explicit result type.
+    // If there is both no explicit result type and the second case does not
+    // apply for any other reason, we're good.
+    if (!closure->hasExplicitResultType() &&
+        (!hasExplicitParenthesizedParamList ||
+         closure->getBracketRange().isValid() ||
+         closure->getThrowsLoc().isValid())) {
+      return;
+    }
+
+    // Compute the insertion location.
+    SourceLoc effectsInsertionLoc = closure->getThrowsLoc();
+    if (effectsInsertionLoc.isInvalid() && closure->hasExplicitResultType()) {
+      effectsInsertionLoc = closure->getArrowLoc();
+    }
+
+    if (effectsInsertionLoc.isInvalid()) {
+      effectsInsertionLoc = closure->getInLoc();
+    }
+
+    ASSERT(effectsInsertionLoc);
+
+    std::string fixIt = "async ";
+    if (closure->getThrowsLoc().isInvalid() && closure->isBodyThrowing()) {
+      fixIt += "throws ";
+    }
+
+    diag.fixItInsert(effectsInsertionLoc, fixIt);
   }
 }
 

test/Concurrency/attr_execution/adoption_mode.swift

@@ -240,6 +240,15 @@ do {
 
 // MARK: Closures
 do {
+  @concurrent
+  func asyncOnly(_: Int, _: Int) async {}
+  @concurrent
+  @Sendable
+  func asyncThrows(_: Int, _: Int) async throws {}
+
+  nonisolated
+  func test(_: @concurrent (Int, Int) async throws -> Void) {}
+
   nonisolated
   func nonisolatedF() {
     let x = 0
@@ -249,59 +258,97 @@ do {
     let _ = { @concurrent () async -> Void in }
     let _ = { @MainActor () async -> Void in }
 
-    // expected-warning@+1:13 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{15-15=@concurrent }}{{none}}
-    let _ = { () async -> Void in }
-    // expected-warning@+1:13 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{15-15=@concurrent }}{{none}}
-    let _ = { [x] () async -> Void in _ = x }
-
-    // expected-warning@+1:13 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{14-14= @concurrent in }}{{none}}
-    let _ = {await globalAsyncF()}
-      // expected-warning@+1:13 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{14-14=@concurrent }}{{none}}
-    let _ = {[x] in await globalAsyncF(); _ = x}
-
-    // expected-warning@+1:13 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{14-14= @concurrent in }}{{none}}
-    let _ = {
-      await globalAsyncF()
-    }
-    // expected-warning@+1:13 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{14-14= @concurrent in }}{{none}}
-    let _ = {
-      await globalAsyncF()
-      func takesInts(_: Int...) {}
-      takesInts($0, $1, $2)
-    }
-
-    // expected-warning@+1:13 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{25-25=@concurrent }}{{none}}
-    let _ = { @Sendable in
-      await globalAsyncF()
-    }
-    // expected-warning@+1:13 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{25-25=@concurrent }}{{none}}
-    let _ = { @Sendable [x] in
-      _ = x
-      await globalAsyncF()
-    }
-
-    // expected-warning@+2:18 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async function type to be treated as specified to run on the caller's actor; use '@concurrent' to preserve behavior}}{{18-18=@concurrent }}{{none}}
-    // expected-warning@+1:45 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{47-47=@concurrent }}{{none}}
-    var closure: (Int, Int) async -> Void = { a, b in
-      await globalAsyncF()
-    }
-    // expected-warning@+1:15 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{17-17=@concurrent }}{{none}}
-    closure = { [x] a, b in _ = x }
-    // expected-warning@+1:15 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{+1:7-7=@concurrent }}{{none}}
-    closure = {
-      a, b async in ()
-    }
-    // expected-warning@+1:15 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{+1:7-7=@concurrent }}{{none}}
-    closure = {
-      [x] a, b async in _ = x
-    }
-    // expected-warning@+1:15 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{17-17=@concurrent }}{{none}}
-    closure = { (a, b) in () }
-
-    // expected-warning@+1:15 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{17-17=@concurrent }}{{none}}
-    closure = { [x] (a, b) in _ = x }
-
-    let _ = closure
+    // Make sure all of these case compile with the fix-its applied.
+    //
+    // Cases where we add inferred effects to ensure it compiles with '@concurrent'.
+
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{34-34=async throws }}{{none}}
+    test { (a: consuming Int, b) in                               try await asyncThrows(a, b) }
+    test { @concurrent (a: consuming Int, b) async throws in      try await asyncThrows(a, b) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{29-29=async throws }}{{none}}
+    test { (a: Int, b: Int) in                                    try await asyncThrows(a, b) }
+    test { @concurrent (a: Int, b: Int) async throws in           try await asyncThrows(a, b) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{22-22=@concurrent }}{{39-39=async throws }}{{none}}
+    test { @Sendable (a: Int, b: Int) in                          try await asyncThrows(a, b) }
+    test { @Sendable @concurrent (a: Int, b: Int) async throws in try await asyncThrows(a, b) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{24-24=async throws }}{{none}}
+    test { (a: Int, b) in                                         try await asyncThrows(a, b) }
+    test { @concurrent (a: Int, b) async throws in                try await asyncThrows(a, b) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{19-19=async throws }}{{none}}
+    test { (a, b) in                                              try await asyncThrows(a, b) }
+    test { @concurrent (a, b) async throws in                     try await asyncThrows(a, b) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{19-19=async }}{{none}}
+    test { (a, b) throws -> Void in                               try await asyncThrows(a, b) }
+    test { @concurrent (a, b) async throws -> Void in             try await asyncThrows(a, b) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{23-23=async }}{{none}}
+    test { [x] (a, b) throws -> Void in                           try await asyncThrows(a, b + x) }
+    test { @concurrent [x] (a, b) async throws -> Void in         try await asyncThrows(a, b + x) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{19-19=async throws }}{{none}}
+    test { (a, b) -> Void in                                      try await asyncThrows(a, b) }
+    test { @concurrent (a, b) async throws -> Void in             try await asyncThrows(a, b) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{23-23=async throws }}{{none}}
+    test { [x] (a, b) -> Void in                                  try await asyncThrows(a, b + x) }
+    test { @concurrent [x] (a, b) async throws -> Void in         try await asyncThrows(a, b + x) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{17-17=async throws }}{{none}}
+    test { a, b -> Void in                                        try await asyncThrows(a, b) }
+    test { @concurrent a, b async throws -> Void in               try await asyncThrows(a, b) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{17-17=async throws }}{{none}}
+    test { _, _ -> Void in                                        try await asyncThrows(1, 2) }
+    test { @concurrent _, _ async throws -> Void in               try await asyncThrows(1, 2) }
+
+    // Cases that will compile with '@concurrent' as is.
+
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{none}}
+    test { (a: Int, b: Int) throws in                             try await asyncThrows(a, b) }
+    test { @concurrent (a: Int, b: Int) throws in                 try await asyncThrows(a, b) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{none}}
+    test { (a, b) throws in                                       try await asyncThrows(a, b) }
+    test { @concurrent (a, b) throws in                           try await asyncThrows(a, b) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{none}}
+    test { (_, _) throws in                                       try await asyncThrows(1, 2) }
+    test { @concurrent (_, _) throws in                           try await asyncThrows(1, 2) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{none}}
+    test { (a, b) async throws in                                 try await asyncThrows(a, b) }
+    test { @concurrent (a, b) async throws in                     try await asyncThrows(a, b) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{none}}
+    test { (a, b) async in                                        await asyncOnly(a, b) }
+    test { @concurrent (a, b) async in                            await asyncOnly(a, b) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{none}}
+    test { a, b async -> Void in                                  await asyncOnly(a, b) }
+    test { @concurrent a, b async -> Void in                      await asyncOnly(a, b) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{none}}
+    test { a, b in                                                try await asyncThrows(a, b) }
+    test { @concurrent a, b in                                    try await asyncThrows(a, b) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{none}}
+    test { a, b throws in                                         try await asyncThrows(a, b) }
+    test { @concurrent a, b throws in                             try await asyncThrows(a, b) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{none}}
+    test { a, b async in                                          await asyncOnly(a, b) }
+    test { @concurrent a, b async in                              await asyncOnly(a, b) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{11-11= @concurrent in }}{{none}}
+    test {                                                        try await asyncThrows($0, $1) }
+    test { @concurrent in                                         try await asyncThrows($0, $1) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{none}}
+    test { [x] in                                                 try await asyncThrows($0, $1 + x) }
+    test { @concurrent [x] in                                     try await asyncThrows($0, $1 + x) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{22-22=@concurrent }}{{none}}
+    test { @Sendable [x] in                                       try await asyncThrows($0, $1 + x) }
+    test { @Sendable @concurrent [x] in                           try await asyncThrows($0, $1 + x) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{none}}
+    test { [x] a, b in                                            try await asyncThrows(a, b + x) }
+    test { @concurrent [x] a, b in                                try await asyncThrows(a, b + x) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{none}}
+    test { [x] a, b async in                                      await asyncOnly(a, b + x) }
+    test { @concurrent [x] a, b async in                          await asyncOnly(a, b + x) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{none}}
+    test { [x] a, b throws in                                     try await asyncThrows(a, b + x) }
+    test { @concurrent [x] a, b throws in                         try await asyncThrows(a, b + x) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{none}}
+    test { [x] (a, b) in                                          try await asyncThrows(a, b + x) }
+    test { @concurrent [x] (a, b) in                              try await asyncThrows(a, b + x) }
+    // expected-warning@+1:10 {{feature 'NonisolatedNonsendingByDefault' will cause nonisolated async closure to run on the caller's actor; use '@concurrent' to preserve behavior}}{{12-12=@concurrent }}{{none}}
+    test { [x] (a: Int, _: Int) in                                try await asyncThrows(a, x) }
+    test { @concurrent [x] (a: Int, _: Int) in                    try await asyncThrows(a, x) }
   }
 
   @MainActor