Sema: Fix an issue with NonisolatedNonsendingByDefault migration for closures

See the inline comments for more details. Depending on the closure's
type signature, sometimes adding the attribute will break code. Fix this
by also adding inferred effects to the signature in these cases.

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