swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.5k stars 10.35k forks source link

Distributed Actor signatures conformance not working after swift format transformation #61999

Open hassila opened 1 year ago

hassila commented 1 year ago

Describe the bug

Hello everyone!

I’m looking at distributed actors for my project. While building some PoC i found next issue, which i’m not sure is it an issue for compiler or it’s better to file an issue for https://github.com/nicklockwood/SwiftFormat. When I apply swiftformat for my project i get next change:

-    public func remoteCall<Actor, Failure, Success>(
+    public func remoteCall<Actor, Success>(
         on _: Actor,
         target _: RemoteCallTarget,
         invocation _: inout RemoteCallEncoder,
-        throwing _: Failure.Type,
+        throwing _: (some Error).Type,
         returning _: Success.Type
     ) async throws -> Success
         where Actor: DistributedActor,
         Actor.ID == ActorID,
-        Failure: Error,
-        Success: SerializationRequirement
-    {
+        Success: SerializationRequirement {
         <some implementation>
     }

-    public func remoteCallVoid<Actor, Failure>(
+    public func remoteCallVoid<Actor>(
         on actor: Actor,
         target: RemoteCallTarget,
         invocation: inout InvocationEncoder,
-        throwing _: Failure.Type
+        throwing _: (some Error).Type
     ) async throws
         where Actor: DistributedActor,
         Actor.ID == ActorID,
-        Failure: Error
-    {
+        {
       <some implementation>

while the applicable syntax seems to be correct from language point of view it breaks the conformance to DistributedActorSystem, so i get an error:

error: class 'DistributedSystem' is missing witness for protocol requirement 'remoteCall'
public class DistributedSystem: DistributedActorSystem, @unchecked Sendable {
             ^
DistributedSystem.swift:11:14: note: protocol 'DistributedActorSystem' requires function 'remoteCall' with signature:
func remoteCall<Act, Err, Res>(
    on actor: Act,
    target: RemoteCallTarget,
    invocation: inout InvocationEncoder,
    throwing: Err.Type,
    returning: Res.Type
) async throws -> Res
  where Act: DistributedActor,
        Act.ID == ActorID,
        Err: Error,
        Res: SerializationRequirement

public class DistributedSystem: DistributedActorSystem, @unchecked Sendable {
             ^
DistributedSystem.swift:163:29: error: expected type
        Actor.ID == ActorID,
                            ^

So, basically it doesn’t like the way how swiftformat changed method signature. If i rollback this particular piece - all work good

Do you think it’s an issue which should be addressed in compiler or should I file a bug for swiftformat that it shouldn’t touch those methods?

Thanks for advice!

Environment Swift 5.7+ .

ktoso commented 1 year ago

Thanks for the report! Yeah I guess we need to handle this better; the witness matching here is implemented "manually" and it since it's an ad-hoc requirement (sadly). So we didn't account for just some Error etc, which changed the count of formal generic parameters to the func.

To doublecheck @hborla @xedin I guess we should support this as it must be possible to implement requirements with a some type I guess, right? I confirmed this should work:

  1> protocol P {
  2.     func nein<A: Error>(a: A)
  3. }
  4> struct K: P {
  5.     func nein(a: some Error) {}
  6. }

Separately from that though I think SwiftFormat aggressively rewriting signatures like that is a bit too aggressive... but that's a separate discussion.

ktoso commented 1 year ago

Radar to track this rdar://102126743

ktoso commented 1 year ago

I thought about this more and agree we must support this. Thanks for reporting!