Merge pull request #40179 from ktoso/wip-dist-func-protos

ktoso · web-flow · commit a78d1bc47a63 · 2021-11-16T15:07:35.000+09:00
[Distributed] Improve error messages for protocol conformances
diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h
@@ -169,6 +169,7 @@ enum class DescriptiveDeclKind : uint8_t {
   Method,
   StaticMethod,
   ClassMethod,
+  DistributedMethod,
   Getter,
   Setter,
   Addressor,
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -4363,7 +4363,7 @@ NOTE(note_add_distributed_to_decl,none,
      "add 'distributed' to %0 to make this %1 witness the protocol requirement",
      (DeclName, DescriptiveDeclKind))
 NOTE(note_distributed_requirement_defined_here,none,
-     "distributed function requirement %0 declared here",
+     "distributed instance method requirement %0 declared here",
      (DeclName))
 NOTE(note_add_globalactor_to_function,none,
      "add '@%0' to make %1 %2 part of global actor %3", 
@@ -4502,13 +4502,13 @@ NOTE(note_distributed_actor_isolated_method,none,
      "distributed actor-isolated %0 %1 declared here",
      (DescriptiveDeclKind, DeclName))
 ERROR(distributed_actor_isolated_method,none,
-      "only 'distributed' functions can be called on a potentially remote distributed actor",
+      "only 'distributed' instance methods can be called on a potentially remote distributed actor",
       ())
 ERROR(distributed_actor_func_param_not_codable,none,
-      "distributed function parameter '%0' of type %1 does not conform to 'Codable'",
+      "distributed instance method parameter '%0' of type %1 does not conform to 'Codable'",
       (StringRef, Type))
 ERROR(distributed_actor_func_result_not_codable,none,
-      "distributed function result type %0 does not conform to 'Codable'",
+      "distributed instance method result type %0 does not conform to 'Codable'",
       (Type))
 ERROR(distributed_actor_remote_func_implemented_manually,none,
       "distributed function's %0 remote counterpart %1 cannot not be implemented manually.",
@@ -4519,6 +4519,9 @@ ERROR(nonisolated_distributed_actor_storage,none,
 ERROR(distributed_actor_func_nonisolated, none,
       "function %0 cannot be both 'nonisolated' and 'distributed'",
       (DeclName))
+ERROR(distributed_actor_func_private, none,
+      "%0 %1 cannot be 'private'",
+      (DescriptiveDeclKind, DeclName))
 ERROR(distributed_actor_remote_func_is_not_static,none,
       "remote function %0 must be static.",
       (DeclName))
@@ -4628,7 +4631,7 @@ ERROR(actor_instance_property_wrapper,none,
       (Identifier, Identifier))
 
 ERROR(distributed_actor_func_defined_outside_of_distributed_actor,none,
-      "distributed function %0 is declared outside of an distributed actor",
+      "distributed instance method %0 is declared outside of an distributed actor",
       (DeclName))
 ERROR(distributed_actor_local_var,none,
       "'distributed' can not be applied to local variables",
@@ -4648,7 +4651,7 @@ ERROR(distributed_actor_not_actor_func,none,
       "'distributed' can only be applied to distributed actor async functions",
       ())
 ERROR(distributed_actor_func_static,none,
-      "'distributed' functions cannot be 'static'",
+      "'distributed' method cannot be 'static'",
       ())
 ERROR(distributed_actor_func_not_in_distributed_actor,none,
       "'distributed' function can only be declared within 'distributed actor'",
diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h
@@ -4367,7 +4367,7 @@ class ApplyExpr : public Expr {
   /// Is this application _implicitly_ required to be a throwing call?
   /// This can happen if the function is actually a proxy function invocation,
   /// which may throw, regardless of the target function throwing, e.g.
-  /// a distributed function call on a 'remote' actor, may throw due to network
+  /// a distributed instance method call on a 'remote' actor, may throw due to network
   /// issues reported by the transport, regardless if the actual target function
   /// can throw.
   bool implicitlyThrows() const {
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -258,7 +258,11 @@ DescriptiveDeclKind Decl::getDescriptiveKind() const {
      // We have a method.
      switch (func->getCorrectStaticSpelling()) {
      case StaticSpellingKind::None:
-       return DescriptiveDeclKind::Method;
+       if (func->isDistributed()) {
+         return DescriptiveDeclKind::DistributedMethod;
+       } else {
+         return DescriptiveDeclKind::Method;
+       }
      case StaticSpellingKind::KeywordStatic:
        return DescriptiveDeclKind::StaticMethod;
      case StaticSpellingKind::KeywordClass:
@@ -320,6 +324,7 @@ StringRef Decl::getDescriptiveKindName(DescriptiveDeclKind K) {
   ENTRY(Method, "instance method");
   ENTRY(StaticMethod, "static method");
   ENTRY(ClassMethod, "class method");
+  ENTRY(DistributedMethod, "distributed instance method");
   ENTRY(Getter, "getter");
   ENTRY(Setter, "setter");
   ENTRY(WillSet, "willSet observer");
diff --git a/lib/SILGen/SILGenDistributed.cpp b/lib/SILGen/SILGenDistributed.cpp
@@ -599,7 +599,7 @@ void SILGenFunction::emitDistributedThunk(SILDeclRef thunk) {
     B.emitBlock(isRemoteBB);
 
     auto *selfTyDecl = FunctionDC->getParent()->getSelfNominalTypeDecl();
-    assert(selfTyDecl && "distributed function declared outside of actor");
+    assert(selfTyDecl && "distributed instance method declared outside of actor");
 
     auto remoteFnDecl = selfTyDecl->lookupDirectRemoteFunc(fd);
     assert(remoteFnDecl && "Could not find _remote_<dist_func_name> function");
diff --git a/lib/Sema/CodeSynthesisDistributedActor.cpp b/lib/Sema/CodeSynthesisDistributedActor.cpp
@@ -179,7 +179,7 @@ static AbstractFunctionDecl *addImplicitDistributedActorRemoteFunction(
       new (C) NonisolatedAttr(/*IsImplicit=*/true));
 
   // users should never have to access this function directly;
-  // it is only invoked from our distributed function thunk if the actor is remote.
+  // it is only invoked from our distributed instance method thunk if the actor is remote.
   remoteFuncDecl->setUserAccessible(false);
   remoteFuncDecl->setSynthesized();
 
diff --git a/lib/Sema/TypeCheckDistributed.cpp b/lib/Sema/TypeCheckDistributed.cpp
@@ -107,6 +107,13 @@ bool swift::checkDistributedFunction(FuncDecl *func, bool diagnose) {
 
   auto module = func->getParentModule();
 
+  if (func->getEffectiveAccess() == AccessLevel::FilePrivate) {
+    func->diagnose(diag::distributed_actor_func_private,
+                   func->getDescriptiveKind(), func->getName());
+    // TODO(distributed): fixit remove the 'private' from the declaration
+    return true;
+  }
+
   // --- Check parameters for 'Codable' conformance
   for (auto param : *func->getParameters()) {
     auto paramTy = func->mapTypeIntoContext(param->getInterfaceType());
@@ -142,7 +149,7 @@ bool swift::checkDistributedFunction(FuncDecl *func, bool diagnose) {
   auto actorDecl = func->getParent()->getSelfNominalTypeDecl();
   assert(actorDecl && actorDecl->isDistributedActor());
 
-  // _remote function for a distributed function must not be implemented by end-users,
+  // _remote function for a distributed instance method must not be implemented by end-users,
   // it must be the specific implementation synthesized by the compiler.
   auto remoteFuncDecl = actorDecl->lookupDirectRemoteFunc(func);
   if (remoteFuncDecl && !remoteFuncDecl->isSynthesized()) {
diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp
@@ -2873,7 +2873,7 @@ bool ConformanceChecker::checkActorIsolation(
     auto nominal = dyn_cast<NominalTypeDecl>(witness->getDeclContext());
     auto witnessClass = dyn_cast<ClassDecl>(witness->getDeclContext());
     if (auto extension = dyn_cast<ExtensionDecl>(witness->getDeclContext())) {
-      // We can witness a distributed function in an extension, as long as
+      // We can witness a distributed instance method in an extension, as long as
       // that extension itself is on a DistributedActor type (including
       // protocols that inherit from DistributedActor, even if the protocol
       // requirement was not expressed in terms of distributed actors).
@@ -2887,7 +2887,7 @@ bool ConformanceChecker::checkActorIsolation(
       // requirement with a distributed function, because those are always
       // cross-actor.
       //
-      // If the distributed function is well-formed (passed checks) then it can
+      // If the distributed instance method is well-formed (passed checks) then it can
       // witness this requirement. I.e. since checks to the distributed function
       // passed, it can be called through this protocol.
       if (witnessFunc && witnessFunc->isDistributed()) {
@@ -3008,7 +3008,7 @@ bool ConformanceChecker::checkActorIsolation(
     auto nominal = dyn_cast<NominalTypeDecl>(witness->getDeclContext());
     auto witnessClass = dyn_cast<ClassDecl>(witness->getDeclContext());
     if (auto extension = dyn_cast<ExtensionDecl>(witness->getDeclContext())) {
-      // We can witness a distributed function in an extension, as long as
+      // We can witness a distributed instance method in an extension, as long as
       // that extension itself is on a DistributedActor type (including
       // protocols that inherit from DistributedActor, even if the protocol
       // requirement was not expressed in terms of distributed actors).
@@ -3021,7 +3021,7 @@ bool ConformanceChecker::checkActorIsolation(
       // requirement with a distributed function, because those are always
       // cross-actor.
       //
-      // If the distributed function is well-formed (passed checks) then it can
+      // If the distributed instance method is well-formed (passed checks) then it can
       // witness this requirement. I.e. since checks to the distributed function
       // passed, it can be called through this protocol.
       if (witnessFunc && witnessFunc->isDistributed()) {
diff --git a/test/Distributed/distributed_actor_cannot_be_downcast_to_actor.swift b/test/Distributed/distributed_actor_cannot_be_downcast_to_actor.swift
@@ -23,7 +23,7 @@ distributed actor MA {
 
 @available(SwiftStdlib 5.6, *)
 func h(ma: MA) async {
-    // this would have been a bug, a non distributed function might have been called here,
+    // this would have been a bug, a non distributed instance method might have been called here,
     // so we must not allow for it, because if the actor was remote calling a non-distributed func
     // would result in a hard crash (as there is no local actor to safely call the function on).
     await g(a: ma) // expected-error{{global function 'g(a:)' requires that 'MA' conform to 'Actor'}}
diff --git a/test/Distributed/distributed_actor_func_implicitly_async_throws.swift b/test/Distributed/distributed_actor_func_implicitly_async_throws.swift
@@ -22,12 +22,12 @@ distributed actor D {
 
 @available(SwiftStdlib 5.6, *)
 func test_not_distributed_funcs(distributed: D) async {
-  distributed.hello() // expected-error{{only 'distributed' functions can be called on a potentially remote distributed actor}}
-  distributed.helloAsync() // expected-error{{only 'distributed' functions can be called on a potentially remote distributed actor}}
+  distributed.hello() // expected-error{{only 'distributed' instance methods can be called on a potentially remote distributed actor}}
+  distributed.helloAsync() // expected-error{{only 'distributed' instance methods can be called on a potentially remote distributed actor}}
   // expected-error@-1{{expression is 'async' but is not marked with 'await'}}
   // expected-note@-2{{call is 'async'}}
   // {{expression is 'async' but is not marked with 'await'}}{{7-7=await }}
-  distributed.helloAsyncThrows() // expected-error{{only 'distributed' functions can be called on a potentially remote distributed actor}}
+  distributed.helloAsyncThrows() // expected-error{{only 'distributed' instance methods can be called on a potentially remote distributed actor}}
   // expected-error@-1{{expression is 'async' but is not marked with 'await'}} // TODO: no need to diagnose this, it is impossible to call anyway
   // expected-note@-2{{call is 'async'}}
   // expected-error@-3{{call can throw, but it is not marked with 'try' and the error is not handled}} // TODO: no need to diagnose this, it is impossible to call anyway
@@ -37,12 +37,12 @@ func test_not_distributed_funcs(distributed: D) async {
 func test_outside(distributed: D) async throws {
   distributed.distHello() // expected-error{{expression is 'async' but is not marked with 'await'}}
   // expected-error@-1{{call can throw but is not marked with 'try'}}
-  // expected-note@-2{{calls to instance method 'distHello()' from outside of its actor context are implicitly asynchronous}}
+  // expected-note@-2{{calls to distributed instance method 'distHello()' from outside of its actor context are implicitly asynchronous}}
   // expected-note@-3{{did you mean to use 'try'?}}
   // expected-note@-4{{did you mean to disable error propagation?}}
   // expected-note@-5{{did you mean to handle error as optional value?}}
   try distributed.distHello() // expected-error{{expression is 'async' but is not marked with 'await'}}
-  // expected-note@-1{{calls to instance method 'distHello()' from outside of its actor context are implicitly asynchronous}}
+  // expected-note@-1{{calls to distributed instance method 'distHello()' from outside of its actor context are implicitly asynchronous}}
   await distributed.distHello() // expected-error{{call can throw but is not marked with 'try'}}
   // expected-note@-1{{did you mean to use 'try'?}}
   // expected-note@-2{{did you mean to disable error propagation?}}
@@ -65,12 +65,12 @@ func test_outside(distributed: D) async throws {
 
   distributed.distHelloThrows() // expected-error{{expression is 'async' but is not marked with 'await'}}
   // expected-error@-1{{call can throw but is not marked with 'try'}}
-  // expected-note@-2{{calls to instance method 'distHelloThrows()' from outside of its actor context are implicitly asynchronous}}
+  // expected-note@-2{{calls to distributed instance method 'distHelloThrows()' from outside of its actor context are implicitly asynchronous}}
   // expected-note@-3{{did you mean to use 'try'?}}
   // expected-note@-4{{did you mean to disable error propagation?}}
   // expected-note@-5{{did you mean to handle error as optional value?}}
   try distributed.distHelloThrows() // expected-error{{expression is 'async' but is not marked with 'await'}}
-  // expected-note@-1{{calls to instance method 'distHelloThrows()' from outside of its actor context are implicitly asynchronous}}
+  // expected-note@-1{{calls to distributed instance method 'distHelloThrows()' from outside of its actor context are implicitly asynchronous}}
   await distributed.distHelloThrows() // expected-error{{call can throw but is not marked with 'try'}}
   // expected-note@-1{{did you mean to use 'try'?}}
   // expected-note@-2{{did you mean to disable error propagation?}}
diff --git a/test/Distributed/distributed_actor_inference.swift b/test/Distributed/distributed_actor_inference.swift
@@ -104,6 +104,6 @@ distributed actor BadValuesDistributedActor_7 {
   distributed subscript(nope: Int) -> Int { nope * 2 } // expected-error{{'distributed' modifier cannot be applied to this declaration}}
   distributed static let staticLetNope: Int = 13 // expected-error{{'distributed' modifier cannot be applied to this declaration}}
   distributed static var staticVarNope: Int { 13 } // expected-error{{'distributed' modifier cannot be applied to this declaration}}
-  distributed static func staticNope() async throws -> Int { 13 } // expected-error{{'distributed' functions cannot be 'static'}}
+  distributed static func staticNope() async throws -> Int { 13 } // expected-error{{'distributed' method cannot be 'static'}}
 }
 
diff --git a/test/Distributed/distributed_actor_isolation.swift b/test/Distributed/distributed_actor_isolation.swift
@@ -47,7 +47,12 @@ distributed actor DistributedActor_1 {
     ""
   }
 
-  distributed static func distributedStatic() {} // expected-error{{'distributed' functions cannot be 'static'}}
+  distributed static func distributedStatic() {} // expected-error{{'distributed' method cannot be 'static'}}
+  distributed class func distributedClass() {}
+  // expected-error@-1{{class methods are only allowed within classes; use 'static' to declare a static method}}
+  // expected-error@-2{{'distributed' method cannot be 'static'}} // TODO(distributed): should call out 'class' instead?
+
+  distributed private func distributedPrivate() {} //expected-error{{distributed instance method 'distributedPrivate()' cannot be 'private'}}
 
   func hello() {} // expected-note{{distributed actor-isolated instance method 'hello()' declared here}}
   func helloAsync() async {} // expected-note{{distributed actor-isolated instance method 'helloAsync()' declared here}}
@@ -63,10 +68,10 @@ 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 function parameter 'notCodable' of type 'NotCodableValue' does not conform to 'Codable'}}
+    // expected-error@-1 {{distributed instance method parameter 'notCodable' of type 'NotCodableValue' does not conform to 'Codable'}}
   }
   distributed func distBadReturn(int: Int) async throws -> NotCodableValue {
-    // expected-error@-1 {{distributed function result type 'NotCodableValue' does not conform to 'Codable'}}
+    // expected-error@-1 {{distributed instance method result type 'NotCodableValue' does not conform to 'Codable'}}
     fatalError()
   }
 
@@ -77,7 +82,7 @@ distributed actor DistributedActor_1 {
     fatalError()
   }
   distributed func distBadReturnGeneric<T: Sendable>(int: Int) async throws -> T {
-    // expected-error@-1 {{distributed function result type 'T' does not conform to 'Codable'}}
+    // expected-error@-1 {{distributed instance method result type 'T' does not conform to 'Codable'}}
     fatalError()
   }
 
@@ -88,7 +93,7 @@ distributed actor DistributedActor_1 {
     value
   }
   distributed func distBadGenericParam<T: Sendable>(int: T) async throws {
-    // expected-error@-1 {{distributed function parameter 'int' of type 'T' does not conform to 'Codable'}}
+    // expected-error@-1 {{distributed instance method parameter 'int' of type 'T' does not conform to 'Codable'}}
     fatalError()
   }
 
@@ -98,7 +103,7 @@ distributed actor DistributedActor_1 {
   static func staticMainActorFunc() -> String { "" } // ok
 
   static distributed func staticDistributedFunc() -> String {
-    // expected-error@-1{{'distributed' functions cannot be 'static'}}{10-21=}
+    // expected-error@-1{{'distributed' method cannot be 'static'}}{10-21=}
     fatalError()
   }
 
@@ -146,9 +151,9 @@ func test_outside(
   _ = DistributedActor_1.staticFunc()
 
   // ==== non-distributed functions
-  distributed.hello() // expected-error{{only 'distributed' functions can be called on a potentially remote distributed actor}}
-  _ = await distributed.helloAsync() // expected-error{{only 'distributed' functions can be called on a potentially remote distributed actor}}
-  _ = try await distributed.helloAsyncThrows() // expected-error{{only 'distributed' functions can be called on a potentially remote distributed actor}}
+  distributed.hello() // expected-error{{only 'distributed' instance methods can be called on a potentially remote distributed actor}}
+  _ = await distributed.helloAsync() // expected-error{{only 'distributed' instance methods can be called on a potentially remote distributed actor}}
+  _ = try await distributed.helloAsyncThrows() // expected-error{{only 'distributed' instance methods can be called on a potentially remote distributed actor}}
 }
 
 // ==== Protocols and static (non isolated functions)
diff --git a/test/Distributed/distributed_actor_protocol_isolation.swift b/test/Distributed/distributed_actor_protocol_isolation.swift
diff --git a/test/Distributed/distributed_protocol_isolation.swift b/test/Distributed/distributed_protocol_isolation.swift
diff --git a/test/decl/protocol/special/DistributedActor.swift b/test/decl/protocol/special/DistributedActor.swift
