Merge pull request #84495 from xedin/dar-157663660-6.2

[6.2][Concurrency] Prevent some extraneous function conversion sendability checking

Author: xedin

lib/Sema/TypeCheckConcurrency.cpp

@@ -2683,7 +2683,8 @@ namespace {
 
     /// Some function conversions synthesized by the constraint solver may not
     /// be correct AND the solver doesn't know, so we must emit a diagnostic.
-    void checkFunctionConversion(Expr *funcConv, Type fromType, Type toType) {
+    void checkFunctionConversion(ImplicitConversionExpr *funcConv,
+                                 Type fromType, Type toType) {
       auto diagnoseNonSendableParametersAndResult =
           [&](FunctionType *fnType,
               std::optional<unsigned> warnUntilSwiftMode = std::nullopt) {
@@ -2788,6 +2789,18 @@ namespace {
               if (!fromFnType->isAsync())
                 break;
 
+              // Applying `nonisolated(nonsending)` to an interface type
+              // of a declaration.
+              if (auto *declRef =
+                      dyn_cast<DeclRefExpr>(funcConv->getSubExpr())) {
+                auto *decl = declRef->getDecl();
+                if (auto *nonisolatedAttr =
+                        decl->getAttrs().getAttribute<NonisolatedAttr>()) {
+                  if (nonisolatedAttr->isNonSending())
+                    return;
+                }
+              }
+
               // @concurrent -> nonisolated(nonsending)
               // crosses an isolation boundary.
               LLVM_FALLTHROUGH;
@@ -2797,16 +2810,13 @@ namespace {
               diagnoseNonSendableParametersAndResult(toFnType);
               break;
 
-            case FunctionTypeIsolation::Kind::Parameter:
-              llvm_unreachable("invalid conversion");
-
+            // @Sendable nonisolated(nonsending) -> nonisolated(nonsending)
+            // doesn't require Sendable checking.
             case FunctionTypeIsolation::Kind::NonIsolatedCaller:
-              // Non isolated caller is always async. This can only occur if we
-              // are converting from an `@Sendable` representation to something
-              // else. So we need to just check that we diagnose non sendable
-              // parameters and results.
-              diagnoseNonSendableParametersAndResult(toFnType);
               break;
+
+            case FunctionTypeIsolation::Kind::Parameter:
+              llvm_unreachable("invalid conversion");
             }
             break;
           }
@@ -3477,19 +3487,6 @@ namespace {
         }
       }
 
-      // The constraint solver may not have chosen legal casts.
-      if (auto funcConv = dyn_cast<FunctionConversionExpr>(expr)) {
-        checkFunctionConversion(funcConv,
-                                funcConv->getSubExpr()->getType(),
-                                funcConv->getType());
-      }
-
-      if (auto *isolationErasure = dyn_cast<ActorIsolationErasureExpr>(expr)) {
-        checkFunctionConversion(isolationErasure,
-                                isolationErasure->getSubExpr()->getType(),
-                                isolationErasure->getType());
-      }
-
       if (auto *defaultArg = dyn_cast<DefaultArgumentExpr>(expr)) {
         checkDefaultArgument(defaultArg);
       }
@@ -3581,6 +3578,19 @@ namespace {
         }
       }
 
+      // The constraint solver may not have chosen legal casts.
+      if (auto funcConv = dyn_cast<FunctionConversionExpr>(expr)) {
+        checkFunctionConversion(funcConv,
+                                funcConv->getSubExpr()->getType(),
+                                funcConv->getType());
+      }
+
+      if (auto *isolationErasure = dyn_cast<ActorIsolationErasureExpr>(expr)) {
+        checkFunctionConversion(isolationErasure,
+                                isolationErasure->getSubExpr()->getType(),
+                                isolationErasure->getType());
+      }
+
       return Action::Continue(expr);
     }
 
@@ -5007,16 +5017,6 @@ ActorIsolation ActorIsolationChecker::determineClosureIsolation(
         return ActorIsolation::forActorInstanceCapture(param);
     }
 
-    // If we have a closure that acts as an isolation inference boundary, then
-    // we return that it is non-isolated.
-    //
-    // NOTE: Since we already checked for global actor isolated things, we
-    // know that all Sendable closures must be nonisolated. That is why it is
-    // safe to rely on this path to handle Sendable closures.
-    if (isIsolationInferenceBoundaryClosure(closure,
-                                            /*canInheritActorContext=*/true))
-      return ActorIsolation::forNonisolated(/*unsafe=*/false);
-
     // A non-Sendable closure gets its isolation from its context.
     auto parentIsolation = getActorIsolationOfContext(
         closure->getParent(), getClosureActorIsolation);
@@ -5048,6 +5048,16 @@ ActorIsolation ActorIsolationChecker::determineClosureIsolation(
       }
     }
 
+    // If we have a closure that acts as an isolation inference boundary, then
+    // we return that it is non-isolated.
+    //
+    // NOTE: Since we already checked for global actor isolated things, we
+    // know that all Sendable closures must be nonisolated. That is why it is
+    // safe to rely on this path to handle Sendable closures.
+    if (isIsolationInferenceBoundaryClosure(closure,
+                                            /*canInheritActorContext=*/true))
+      return ActorIsolation::forNonisolated(/*unsafe=*/false);
+
     return normalIsolation;
   }();
 

test/Concurrency/actor_inout_isolation.swift

@@ -220,7 +220,7 @@ if #available(SwiftStdlib 5.1, *) {
   let _ = Task.detached { await { (_ foo: inout Int) async in foo += 1 }(&number) }
   // expected-error @-1 {{actor-isolated var 'number' cannot be passed 'inout' to 'async' function call}}
   // expected-minimal-error @-2 {{global actor 'MyGlobalActor'-isolated var 'number' can not be used 'inout' from a nonisolated context}}
-  // expected-complete-tns-error @-3 {{main actor-isolated var 'number' can not be used 'inout' from a nonisolated context}}
+  // expected-complete-tns-warning @-3 {{main actor-isolated var 'number' can not be used 'inout' from a nonisolated context}}
 }
 
 // attempt to pass global state owned by the global actor to another async function

test/Concurrency/attr_execution/conversions_silgen.swift

@@ -613,3 +613,33 @@ func testConvertToThrowing(isolation: isolated (any Actor)? = #isolation) async
     observe()
   }
 }
+
+func testSendableClosureNonisolatedNonSendingInference() {
+  // CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen49testSendableClosureNonisolatedNonSendingInferenceyyFySiYaYbYCcfU_ : $@convention(thin) @Sendable @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, Int) -> ()
+  // CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional<any Actor>, %1 : $Int):
+  // CHECK: hop_to_executor [[EXECUTOR]]
+  // CHECK: // end sil function '$s21attr_execution_silgen49testSendableClosureNonisolatedNonSendingInferenceyyFySiYaYbYCcfU_'
+  let _: nonisolated(nonsending) @Sendable (Int) async -> Void = { _ in }
+
+  // CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen49testSendableClosureNonisolatedNonSendingInferenceyyFyS2SYaKYCcYaYbYCcfU0_ : $@convention(thin) @Sendable @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @guaranteed @async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @guaranteed String) -> (@owned String, @error any Error)) -> @error any Error
+  // CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional<any Actor>, %1 : @guaranteed $@async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @guaranteed String) -> (@owned String, @error any Error)):
+  // CHECK: hop_to_executor [[EXECUTOR]]
+  // CHECK: // end sil function '$s21attr_execution_silgen49testSendableClosureNonisolatedNonSendingInferenceyyFyS2SYaKYCcYaYbYCcfU0_'
+  let _: nonisolated(nonsending) @Sendable (
+    nonisolated(nonsending) @escaping (String) async throws -> String
+  ) async throws -> Void = { _ in }
+}
+
+// CHECK-LABEL: sil hidden [ossa] @$s21attr_execution_silgen014testSendableToE35ConversionWithNonisilatedNonsendingyyF : $@convention(thin) () -> ()
+// CHECK: [[TEST_REF:%.*]] = function_ref @$s21attr_execution_silgen014testSendableToE35ConversionWithNonisilatedNonsendingyyF0D0L_7closureyS2SYaKYCc_tYaYbKF : $@convention(thin) @Sendable @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @guaranteed @async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @guaranteed String) -> (@owned String, @error any Error)) -> @error any Error
+// CHECK: // end sil function '$s21attr_execution_silgen014testSendableToE35ConversionWithNonisilatedNonsendingyyF'
+func testSendableToSendableConversionWithNonisilatedNonsending() {
+  @Sendable nonisolated(nonsending) func test(
+    closure: nonisolated(nonsending) @escaping (String) async throws -> String
+  ) async throws {
+  }
+
+  let _: nonisolated(nonsending) @Sendable (
+    nonisolated(nonsending) @escaping (String) async throws -> String
+  ) async throws -> Void = test
+}

test/Concurrency/sendable_checking_captures_swift5.swift

@@ -92,5 +92,5 @@ do {
 
   let c: Class
   test(c)
-  // expected-complete-warning@-1:8 {{implicit capture of 'c' requires that 'Class' conforms to 'Sendable'; this is an error in the Swift 6 language mode}}
+  // expected-complete-warning@-1:8 {{implicit capture of 'c' requires that 'Class' conforms to 'Sendable'}}
 }