Merge pull request #40197 from ktoso/wip-more-func-restrictions

[Distributed] more restrictions on distributed funcs based on serialization findings

Author: ktoso

include/swift/AST/DiagnosticsSema.def

@@ -4505,25 +4505,34 @@ ERROR(distributed_actor_isolated_method,none,
       "only 'distributed' instance methods can be called on a potentially remote distributed actor",
       ())
 ERROR(distributed_actor_func_param_not_codable,none,
-      "distributed instance method parameter '%0' of type %1 does not conform to 'Codable'",
-      (StringRef, Type))
+      "parameter '%0' of type %1 in %2 does not conform to '%3'",
+      (StringRef, Type, DescriptiveDeclKind, StringRef))
 ERROR(distributed_actor_func_result_not_codable,none,
-      "distributed instance method result type %0 does not conform to 'Codable'",
-      (Type))
+      "result type %0 of %1 does not conform to '%2'",
+      (Type, DescriptiveDeclKind, StringRef))
 ERROR(distributed_actor_remote_func_implemented_manually,none,
-      "distributed function's %0 remote counterpart %1 cannot not be implemented manually.",
+      "distributed instance method's %0 remote counterpart %1 cannot not be implemented manually.",
       (Identifier, Identifier))
 ERROR(nonisolated_distributed_actor_storage,none,
       "'nonisolated' can not be applied to distributed actor stored properties",
       ())
 ERROR(distributed_actor_func_nonisolated, none,
-      "function %0 cannot be both 'nonisolated' and 'distributed'",
+      "cannot declare method %0 as both 'nonisolated' and 'distributed'",
       (DeclName))
 ERROR(distributed_actor_func_private, none,
       "%0 %1 cannot be 'private'",
       (DescriptiveDeclKind, DeclName))
+ERROR(distributed_actor_func_inout, none,
+      "cannot declare 'inout' argument %0 in %1 %2",
+      (DeclName, DescriptiveDeclKind, DeclName))
+ERROR(distributed_actor_func_closure, none,
+      "%0 %1 cannot declare closure arguments, as they cannot be serialized",
+      (DescriptiveDeclKind, DeclName))
+ERROR(distributed_actor_func_variadic, none,
+      "cannot declare variadic argument %0 in %1 %2",
+      (DeclName, DescriptiveDeclKind, DeclName))
 ERROR(distributed_actor_remote_func_is_not_static,none,
-      "remote function %0 must be static.",
+      "remote function %0 must be static",
       (DeclName))
 ERROR(distributed_actor_remote_func_is_not_async_throws,none,
       "remote function %0 must be 'async throws'.",
@@ -4654,7 +4663,7 @@ ERROR(distributed_actor_func_static,none,
       "'distributed' method cannot be 'static'",
       ())
 ERROR(distributed_actor_func_not_in_distributed_actor,none,
-      "'distributed' function can only be declared within 'distributed actor'",
+      "'distributed' method can only be declared within 'distributed actor'",
       ())
 ERROR(distributed_actor_designated_ctor_must_have_one_transport_param,none,
       "designated distributed actor initializer %0 must accept exactly one "

lib/Sema/TypeCheckDistributed.cpp

@@ -69,6 +69,24 @@ void swift::diagnoseDistributedFunctionInNonDistributedActorProtocol(
   }
 }
 
+
+/// Add Fix-It text for the given nominal type to adopt Codable.
+///
+/// Useful when 'Codable' is the 'SerializationRequirement' and a non-Codable
+/// function parameter or return value type is detected.
+void swift::addCodableFixIt(
+    const NominalTypeDecl *nominal, InFlightDiagnostic &diag) {
+  if (nominal->getInherited().empty()) {
+    SourceLoc fixItLoc = nominal->getBraces().Start;
+    diag.fixItInsert(fixItLoc, ": Codable");
+  } else {
+    ASTContext &ctx = nominal->getASTContext();
+    SourceLoc fixItLoc = nominal->getInherited().back().getSourceRange().End;
+    fixItLoc = Lexer::getLocForEndOfToken(ctx.SourceMgr, fixItLoc);
+    diag.fixItInsert(fixItLoc, ", Codable");
+  }
+}
+
 // ==== ------------------------------------------------------------------------
 
 bool IsDistributedActorRequest::evaluate(
@@ -119,28 +137,53 @@ bool swift::checkDistributedFunction(FuncDecl *func, bool diagnose) {
     auto paramTy = func->mapTypeIntoContext(param->getInterfaceType());
     if (TypeChecker::conformsToProtocol(paramTy, encodableType, module).isInvalid() ||
         TypeChecker::conformsToProtocol(paramTy, decodableType, module).isInvalid()) {
-      if (diagnose)
-        func->diagnose(
+      if (diagnose) {
+        auto diag = func->diagnose(
             diag::distributed_actor_func_param_not_codable,
-            param->getArgumentName().str(),
-            param->getInterfaceType()
-        );
-      // TODO: suggest a fixit to add Codable to the type?
+            param->getArgumentName().str(), param->getInterfaceType(),
+            func->getDescriptiveKind(), "Codable");
+        if (auto paramNominalTy = paramTy->getAnyNominal()) {
+          addCodableFixIt(paramNominalTy, diag);
+        } // else, no nominal type to suggest the fixit for, e.g. a closure
+      }
+      return true;
+    }
+
+    if (param->isInOut()) {
+      param->diagnose(
+          diag::distributed_actor_func_inout,
+          param->getName(),
+          func->getDescriptiveKind(), func->getName()
+      ).fixItRemove(SourceRange(param->getTypeSourceRangeForDiagnostics().Start,
+                                param->getTypeSourceRangeForDiagnostics().Start.getAdvancedLoc(1)));
+      // FIXME(distributed): the fixIt should be on param->getSpecifierLoc(), but that Loc is invalid for some reason?
       return true;
     }
+
+    if (param->isVariadic()) {
+      param->diagnose(
+          diag::distributed_actor_func_variadic,
+          param->getName(),
+          func->getDescriptiveKind(), func->getName()
+      );
+    }
   }
 
   // --- Result type must be either void or a codable type
   auto resultType = func->mapTypeIntoContext(func->getResultInterfaceType());
   if (!resultType->isVoid()) {
     if (TypeChecker::conformsToProtocol(resultType, decodableType, module).isInvalid() ||
         TypeChecker::conformsToProtocol(resultType, encodableType, module).isInvalid()) {
-      if (diagnose)
-        func->diagnose(
+      if (diagnose) {
+        auto diag = func->diagnose(
             diag::distributed_actor_func_result_not_codable,
-            func->getResultInterfaceType()
+            func->getResultInterfaceType(), func->getDescriptiveKind(),
+            "Codable" // Codable is a typealias, easier to diagnose like that
         );
-      // TODO: suggest a fixit to add Codable to the type?
+        if (auto resultNominalType = resultType->getAnyNominal()) {
+          addCodableFixIt(resultNominalType, diag);
+        }
+      }
       return true;
     }
   }

lib/Sema/TypeCheckDistributed.h

@@ -55,6 +55,9 @@ Type getDistributedActorIdentityType(NominalTypeDecl *actor);
 void diagnoseDistributedFunctionInNonDistributedActorProtocol(
   const ProtocolDecl *proto, InFlightDiagnostic &diag);
 
+/// Emit a FixIt suggesting to add Codable to the nominal type.
+void addCodableFixIt(const NominalTypeDecl *nominal, InFlightDiagnostic &diag);
+
 }
 
 

test/Distributed/distributed_actor_inference.swift

@@ -29,21 +29,21 @@ distributed enum SomeDistributedActor_3 { } // expected-error{{'distributed' mod
 
 @available(SwiftStdlib 5.6, *)
 struct SomeNotActorStruct_2 {
-  distributed func nopeAsyncThrows() async throws -> Int { 42 } // expected-error{{'distributed' function can only be declared within 'distributed actor'}}
+  distributed func nopeAsyncThrows() async throws -> Int { 42 } // expected-error{{'distributed' method can only be declared within 'distributed actor'}}
 }
 
 @available(SwiftStdlib 5.6, *)
 class SomeNotActorClass_3 {
-  distributed func nopeAsyncThrows() async throws -> Int { 42 } // expected-error{{'distributed' function can only be declared within 'distributed actor'}}
+  distributed func nopeAsyncThrows() async throws -> Int { 42 } // expected-error{{'distributed' method can only be declared within 'distributed actor'}}
 }
 
 @available(SwiftStdlib 5.6, *)
 actor SomeNotDistributedActor_4 {
-  distributed func notInDistActorAsyncThrowing() async throws -> Int { 42 } // expected-error{{'distributed' function can only be declared within 'distributed actor'}}
+  distributed func notInDistActorAsyncThrowing() async throws -> Int { 42 } // expected-error{{'distributed' method can only be declared within 'distributed actor'}}
 }
 
 protocol DP {
-  distributed func hello()  // expected-error{{'distributed' function can only be declared within 'distributed actor'}}
+  distributed func hello()  // expected-error{{'distributed' method can only be declared within 'distributed actor'}}
 }
 
 @available(SwiftStdlib 5.6, *)
@@ -58,7 +58,7 @@ protocol DPOK2: DPOK {
 
 @available(SwiftStdlib 5.6, *)
 enum SomeNotActorEnum_5 {
-  distributed func nopeAsyncThrows() async throws -> Int { 42 } // expected-error{{'distributed' function can only be declared within 'distributed actor'}}
+  distributed func nopeAsyncThrows() async throws -> Int { 42 } // expected-error{{'distributed' method can only be declared within 'distributed actor'}}
 }
 
 @available(SwiftStdlib 5.6, *)
@@ -68,9 +68,9 @@ distributed actor SomeDistributedActor_6 {
 
 @available(SwiftStdlib 5.6, *)
 distributed actor SomeDistributedActor_7 {
-  distributed func dont_1() async throws -> Int { 42 } // expected-error{{distributed function's 'dont_1' remote counterpart '_remote_dont_1' cannot not be implemented manually.}}
-  distributed func dont_2() async throws -> Int { 42 } // expected-error{{distributed function's 'dont_2' remote counterpart '_remote_dont_2' cannot not be implemented manually.}}
-  distributed func dont_3() async throws -> Int { 42 } // expected-error{{distributed function's 'dont_3' remote counterpart '_remote_dont_3' cannot not be implemented manually.}}
+  distributed func dont_1() async throws -> Int { 42 } // expected-error{{distributed instance method's 'dont_1' remote counterpart '_remote_dont_1' cannot not be implemented manually.}}
+  distributed func dont_2() async throws -> Int { 42 } // expected-error{{distributed instance method's 'dont_2' remote counterpart '_remote_dont_2' cannot not be implemented manually.}}
+  distributed func dont_3() async throws -> Int { 42 } // expected-error{{distributed instance method's 'dont_3' remote counterpart '_remote_dont_3' cannot not be implemented manually.}}
 }
 
 @available(SwiftStdlib 5.6, *)

test/Distributed/distributed_actor_is_experimental_enabled_missing_import.swift

@@ -15,8 +15,8 @@ distributed actor class DAC {}
 
 actor A {
   func normal() async {}
-  distributed func dist() {} // expected-error{{'distributed' function can only be declared within 'distributed actor'}}
-  distributed func distAsync() async {} // expected-error{{'distributed' function can only be declared within 'distributed actor'}}
+  distributed func dist() {} // expected-error{{'distributed' method can only be declared within 'distributed actor'}}
+  distributed func distAsync() async {} // expected-error{{'distributed' method can only be declared within 'distributed actor'}}
 
   distributed var neverOk: String { // expected-error{{'distributed' modifier cannot be applied to this declaration}}
     "vars are not allowed to be distributed *ever* anyway"

test/Distributed/distributed_actor_isolation.swift

@@ -22,7 +22,7 @@ actor LocalActor_1 {
   var mutable: String = ""
 
   distributed func nope() {
-    // expected-error@-1{{'distributed' function can only be declared within 'distributed actor'}}
+    // expected-error@-1{{'distributed' method can only be declared within 'distributed actor'}}
   }
 }
 
@@ -68,21 +68,33 @@ distributed actor DistributedActor_1 {
   distributed func distIntString(int: Int, two: String) async throws -> (String) { "\(int) + \(two)" } // ok
 
   distributed func dist(notCodable: NotCodableValue) async throws {
-    // expected-error@-1 {{distributed instance method parameter 'notCodable' of type 'NotCodableValue' does not conform to 'Codable'}}
+    // expected-error@-1 {{parameter 'notCodable' of type 'NotCodableValue' in distributed instance method does not conform to 'Codable'}}
   }
   distributed func distBadReturn(int: Int) async throws -> NotCodableValue {
-    // expected-error@-1 {{distributed instance method result type 'NotCodableValue' does not conform to 'Codable'}}
+    // expected-error@-1 {{result type 'NotCodableValue' of distributed instance method does not conform to 'Codable'}}
     fatalError()
   }
 
+  distributed func varargs(int: Int...) {
+    // expected-error@-1{{cannot declare variadic argument 'int' in distributed instance method 'varargs(int:)'}}
+  }
+
+  distributed func closure(close: () -> String) {
+    // expected-error@-1{{parameter 'close' of type '() -> String' in distributed instance method does not conform to 'Codable'}}
+  }
+
+  distributed func noInout(inNOut burger: inout String) {
+    // expected-error@-1{{cannot declare 'inout' argument 'burger' in distributed instance method 'noInout(inNOut:)'}}{{43-49=}}
+  }
+
   distributed func distReturnGeneric<T: Codable & Sendable>(item: T) async throws -> T { // ok
     item
   }
   distributed func distReturnGenericWhere<T: Sendable>(item: Int) async throws -> T where T: Codable { // ok
     fatalError()
   }
   distributed func distBadReturnGeneric<T: Sendable>(int: Int) async throws -> T {
-    // expected-error@-1 {{distributed instance method result type 'T' does not conform to 'Codable'}}
+    // expected-error@-1 {{result type 'T' of distributed instance method does not conform to 'Codable'}}
     fatalError()
   }
 
@@ -93,7 +105,7 @@ distributed actor DistributedActor_1 {
     value
   }
   distributed func distBadGenericParam<T: Sendable>(int: T) async throws {
-    // expected-error@-1 {{distributed instance method parameter 'int' of type 'T' does not conform to 'Codable'}}
+    // expected-error@-1 {{parameter 'int' of type 'T' in distributed instance method does not conform to 'Codable'}}
     fatalError()
   }
 

test/Distributed/distributed_actor_nonisolated.swift

@@ -38,12 +38,12 @@ distributed actor DA {
   }
 
   nonisolated distributed func nonisolatedDistributed() async {
-    // expected-error@-1{{function 'nonisolatedDistributed()' cannot be both 'nonisolated' and 'distributed'}}{{3-15=}}
+    // expected-error@-1{{cannot declare method 'nonisolatedDistributed()' as both 'nonisolated' and 'distributed'}}{{3-15=}}
     fatalError()
   }
 
   distributed nonisolated func distributedNonisolated() async {
-    // expected-error@-1{{function 'distributedNonisolated()' cannot be both 'nonisolated' and 'distributed'}}{{15-27=}}
+    // expected-error@-1{{cannot declare method 'distributedNonisolated()' as both 'nonisolated' and 'distributed'}}{{15-27=}}
     fatalError()
   }
 

test/Distributed/distributed_protocol_isolation.swift

@@ -11,7 +11,7 @@ typealias DefaultActorTransport = AnyActorTransport
 // MARK: Distributed actor protocols
 
 protocol WrongDistFuncs {
-    distributed func notDistActor() // expected-error{{'distributed' function can only be declared within 'distributed actor'}}{{5-17=}} {{25-25=: DistributedActor}}
+    distributed func notDistActor() // expected-error{{'distributed' method can only be declared within 'distributed actor'}}{{5-17=}} {{25-25=: DistributedActor}}
 }
 
 protocol DistProtocol: DistributedActor {
@@ -199,7 +199,7 @@ extension DistributedTacoMaker {
 
 extension TacoPreparation {
     distributed func makeSalsa() -> Salsa {}
-  // expected-error@-1{{'distributed' function can only be declared within 'distributed actor'}}
+  // expected-error@-1{{'distributed' method can only be declared within 'distributed actor'}}
 }
 
 distributed actor TacoWorker: DistributedTacoMaker {} // implemented in extensions