Merge pull request #79651 from xedin/sendability-checks-for-function-conversions

[Concurrency] Implement sendability checking for `@Sendable` function…

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -8363,6 +8363,13 @@ ERROR(attr_execution_type_attr_incompatible_with_isolated_any,none,
       "cannot use '@execution' together with @isolated(any)",
       ())
 
+ERROR(invalid_function_conversion_with_non_sendable,none,
+      "cannot convert %0 to %1 because crossing of an isolation boundary "
+      "requires parameter and result types to conform to 'Sendable' protocol",
+      (Type, Type))
+NOTE(type_does_not_conform_to_Sendable,none,
+     "type %0 does not conform to 'Sendable' protocol", (Type))
+
 
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"

lib/Sema/TypeCheckConcurrency.cpp

@@ -2654,12 +2654,58 @@ 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) {
+      auto diagnoseNonSendableParametersAndResult =
+          [&](FunctionType *fnType, bool downgradeToWarning = false) {
+            auto *dc = getDeclContext();
+            llvm::SmallPtrSet<Type, 2> nonSendableTypes;
+
+            SendableCheckContext context(dc);
+            for (auto &param : fnType->getParams()) {
+              diagnoseNonSendableTypes(
+                  param.getPlainType(), context,
+                  /*inDerivedConformance=*/Type(), funcConv->getLoc(),
+                  [&](Type type, DiagnosticBehavior behavior) {
+                    nonSendableTypes.insert(type);
+                    return true;
+                  });
+            }
+
+            diagnoseNonSendableTypes(
+                fnType->getResult(), context,
+                /*inDerivedConformance=*/Type(), funcConv->getLoc(),
+                [&](Type type, DiagnosticBehavior behavior) {
+                  nonSendableTypes.insert(type);
+                  return true;
+                });
+
+            if (!nonSendableTypes.empty()) {
+              auto &ctx = dc->getASTContext();
+              {
+                auto diag = ctx.Diags.diagnose(
+                    funcConv->getLoc(),
+                    diag::invalid_function_conversion_with_non_sendable,
+                    fromType, toType);
+
+                if (downgradeToWarning)
+                  diag.warnUntilSwiftVersion(7);
+              }
+
+              for (auto type : nonSendableTypes) {
+                ctx.Diags.diagnose(funcConv->getLoc(),
+                                   diag::type_does_not_conform_to_Sendable,
+                                   type);
+              }
+            }
+          };
+
       if (auto fromFnType = fromType->getAs<FunctionType>()) {
-        if (auto fromActor = fromFnType->getGlobalActor()) {
-          if (auto toFnType = toType->getAs<FunctionType>()) {
+        if (auto toFnType = toType->getAs<FunctionType>()) {
+          auto fromIsolation = fromFnType->getIsolation();
+          auto toIsolation = toFnType->getIsolation();
 
-            // ignore some kinds of casts, as they're diagnosed elsewhere.
-            if (toFnType->hasGlobalActor() || toFnType->isAsync())
+          if (auto fromActor = fromFnType->getGlobalActor()) {
+            if (toFnType->hasGlobalActor() ||
+                (toFnType->isAsync() && !toIsolation.isNonIsolatedCaller()))
               return;
 
             auto dc = const_cast<DeclContext*>(getDeclContext());
@@ -2672,7 +2718,115 @@ namespace {
                             diag::converting_func_loses_global_actor, fromType,
                             toType, fromActor)
                   .warnUntilSwiftVersion(6);
+              return;
+            }
+          }
+
+          // Conversions from non-Sendable types are handled by
+          // region-based isolation.
+          // Function conversions are used to inject concurrency attributes
+          // into interface types until that changes we won't be able to
+          // diagnose all of the cases here.
+          if (!fromFnType->isSendable())
+            return;
+
+          switch (toIsolation.getKind()) {
+          // Converting to `@execution(caller)` function type
+          case FunctionTypeIsolation::Kind::NonIsolatedCaller: {
+            switch (fromIsolation.getKind()) {
+            case FunctionTypeIsolation::Kind::NonIsolated: {
+              // nonisolated -> @execution(caller) doesn't cross
+              // an isolation boundary.
+              if (!fromFnType->isAsync())
+                break;
+
+              // @execution(concurrent) -> @execution(caller)
+              // crosses an isolation boundary.
+              LLVM_FALLTHROUGH;
+            }
+            case FunctionTypeIsolation::Kind::GlobalActor:
+            case FunctionTypeIsolation::Kind::Erased:
+              diagnoseNonSendableParametersAndResult(toFnType);
+              break;
+
+            case FunctionTypeIsolation::Kind::NonIsolatedCaller:
+            case FunctionTypeIsolation::Kind::Parameter:
+              llvm_unreachable("invalid conversion");
+            }
+            break;
+          }
+
+          // Converting to nonisolated synchronous or @execution(concurrent)
+          // asynchronous function type could require crossing an isolation
+          // boundary.
+          case FunctionTypeIsolation::Kind::NonIsolated: {
+            switch (fromIsolation.getKind()) {
+            case FunctionTypeIsolation::Kind::Parameter:
+            case FunctionTypeIsolation::Kind::NonIsolatedCaller:
+            case FunctionTypeIsolation::Kind::Erased:
+              diagnoseNonSendableParametersAndResult(
+                  toFnType, /*downgradeToWarning=*/true);
+              break;
+
+            case FunctionTypeIsolation::Kind::GlobalActor: {
+              // Handled above by `safeToDropGlobalActor` check.
+              break;
             }
+
+            case FunctionTypeIsolation::Kind::NonIsolated: {
+              // nonisolated synchronous <-> @execution(concurrent)
+              if (fromFnType->isAsync() != toFnType->isAsync()) {
+                diagnoseNonSendableParametersAndResult(
+                    toFnType, /*downgradeToWarning=*/true);
+              }
+              break;
+            }
+            }
+            break;
+          }
+
+          // Converting to an actor-isolated function always
+          // requires crossing an isolation boundary.
+          case FunctionTypeIsolation::Kind::GlobalActor: {
+            switch (fromIsolation.getKind()) {
+            case FunctionTypeIsolation::Kind::Parameter:
+            case FunctionTypeIsolation::Kind::Erased:
+              diagnoseNonSendableParametersAndResult(
+                  toFnType, /*downgradeToWarning=*/true);
+              break;
+
+            case FunctionTypeIsolation::Kind::NonIsolated: {
+              // Since @execution(concurrent) as an asynchronous
+              // function it would mean that without Sendable
+              // check it would be possible for non-Sendable state
+              // to escape from actor isolation.
+              if (fromFnType->isAsync()) {
+                diagnoseNonSendableParametersAndResult(
+                  toFnType, /*downgradeToWarning=*/true);
+                break;
+              }
+              // Runs on the actor.
+              break;
+            }
+
+            // Runs on the actor.
+            case FunctionTypeIsolation::Kind::NonIsolatedCaller:
+              break;
+
+            case FunctionTypeIsolation::Kind::GlobalActor:
+              llvm_unreachable("invalid conversion");
+            }
+            break;
+          }
+
+          // Converting to @isolated(any) doesn't cross an isolation
+          // boundary.
+          case FunctionTypeIsolation::Kind::Erased:
+            break;
+
+          // TODO: Figure out what exactly needs to happen here.
+          case FunctionTypeIsolation::Kind::Parameter:
+            break;
           }
         }
       }

test/Concurrency/attr_execution_conversions.swift

@@ -4,6 +4,11 @@
 // REQUIRES: concurrency
 // REQUIRES: swift_feature_ExecutionAttribute
 
+@globalActor
+actor MyActor {
+  static let shared = MyActor()
+}
+
 @execution(concurrent)
 func concurrentTest() async {
 }
@@ -77,3 +82,64 @@ do {
   // expected-error@-1 {{cannot convert value of type '(@execution(caller) () async -> ()).Type' to expected argument type '(@isolated(any) () async -> Void).Type'}}
 }
 
+
+// Converting to `@execution(caller)` function
+class NonSendable {}
+
+func testNonSendableDiagnostics(
+  globalActor1: @escaping @Sendable @MainActor (NonSendable) async -> Void,
+  globalActor2: @escaping @Sendable @MainActor () async -> NonSendable,
+  erased1: @escaping @Sendable @isolated(any) (NonSendable) async -> Void,
+  erased2: @escaping @Sendable @isolated(any) () async -> NonSendable,
+  nonIsolated1: @escaping @Sendable (NonSendable) -> Void,
+  nonIsolated2: @escaping @Sendable @execution(concurrent) (NonSendable) async -> Void,
+  nonIsolated3: @escaping @Sendable () -> NonSendable,
+  nonIsolated4: @escaping @Sendable @execution(concurrent) () async -> NonSendable,
+  caller1: @escaping @Sendable @execution(caller) (NonSendable) async -> Void,
+  caller2: @escaping @Sendable @execution(caller) () async -> NonSendable
+) {
+  let _: @execution(caller) (NonSendable) async -> Void = globalActor1 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}}
+  // expected-error@-1 {{cannot convert '@MainActor @Sendable (NonSendable) async -> Void' to '@execution(caller) (NonSendable) async -> Void' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}}
+  let _: @execution(caller) () async -> NonSendable = globalActor2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}}
+  // expected-error@-1 {{cannot convert '@MainActor @Sendable () async -> NonSendable' to '@execution(caller) () async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}}
+
+  let _: @execution(caller) (NonSendable) async -> Void = erased1 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}}
+  // expected-error@-1 {{cannot convert '@isolated(any) @Sendable (NonSendable) async -> Void' to '@execution(caller) (NonSendable) async -> Void' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}}
+  let _: @execution(caller) () async -> NonSendable = erased2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}}
+  // expected-error@-1 {{cannot convert '@isolated(any) @Sendable () async -> NonSendable' to '@execution(caller) () async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}}
+
+  let _: @execution(caller) (NonSendable) async -> Void = nonIsolated1 // Ok
+  let _: @execution(caller) (NonSendable) async -> Void = nonIsolated2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}}
+  // expected-error@-1 {{cannot convert '@Sendable (NonSendable) async -> Void' to '@execution(caller) (NonSendable) async -> Void' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}}
+
+  let _: @execution(caller) () async -> NonSendable = nonIsolated3 // Ok
+  let _: @execution(caller) () async -> NonSendable = nonIsolated4 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}}
+  // expected-error@-1 {{cannot convert '@Sendable () async -> NonSendable' to '@execution(caller) () async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}}
+
+  let _: @execution(concurrent) (NonSendable) async -> Void = erased1 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}}
+  // expected-warning@-1 {{cannot convert '@isolated(any) @Sendable (NonSendable) async -> Void' to '(NonSendable) async -> Void' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}}
+  let _: @execution(concurrent) () async -> NonSendable = erased2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}}
+  // expected-warning@-1 {{cannot convert '@isolated(any) @Sendable () async -> NonSendable' to '() async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}}
+
+
+  let _: @execution(concurrent) (NonSendable) async -> Void = caller1 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}}
+  // expected-warning@-1 {{cannot convert '@execution(caller) @Sendable (NonSendable) async -> Void' to '(NonSendable) async -> Void' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}}
+  let _: @execution(concurrent) () async -> NonSendable = caller2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}}
+  // expected-warning@-1 {{cannot convert '@execution(caller) @Sendable () async -> NonSendable' to '() async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}}
+
+  let _: @MainActor (NonSendable) async -> Void = nonIsolated1 // Ok
+  let _: @MainActor (NonSendable) async -> Void = nonIsolated2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}}
+  // expected-warning@-1 {{cannot convert '@Sendable (NonSendable) async -> Void' to '@MainActor (NonSendable) async -> Void' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}}
+
+  let _: @MainActor () async -> NonSendable = nonIsolated3 // Ok
+  let _: @MainActor () async -> NonSendable = nonIsolated4 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}}
+  // expected-warning@-1 {{cannot convert '@Sendable () async -> NonSendable' to '@MainActor () async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}}
+
+  let _: @MainActor (NonSendable) async -> Void = caller1 // Ok
+  let _: @MainActor () async -> NonSendable = caller2 // Ok
+
+  let _: @MyActor (NonSendable) async -> Void = globalActor1
+  // expected-error@-1 {{cannot convert value actor-isolated to 'MainActor' to specified type actor-isolated to 'MyActor'}}
+  let _: @MyActor () async -> NonSendable = globalActor2
+  // expected-error@-1 {{cannot convert value actor-isolated to 'MainActor' to specified type actor-isolated to 'MyActor'}}
+}