[Distributed] Only synthesize Codable for DA where the ID is Codable

[ktoso]

This is a bug in synthesis we only noticed recently. The design is such that a distributed actor shall be Codable when the ID is Codable. But we wrongly always added the conformance and thus synthesis would fail when IDs were not Codable.

This forced adopters to make their ID types Codable even though they were not using Codable at all.

The implementations come from:

@available(SwiftStdlib 5.7, *)
extension DistributedActor /*: implicitly Decodable */ where Self.ID: Decodable { ... }
@available(SwiftStdlib 5.7, *)
extension DistributedActor /*: implicitly Encodable */ where Self.ID: Encodable { ... } 

We are not extending this implicit conformance to other types, so this is just a fix of behavior.

resolves rdar://122930345

[ktoso]

I wonder if @ser-0xff @hassila have hit this issue in https://github.com/ordo-one/package-distributed-system since it seems you use your own serialization requirement but had to make the actor and id codable anyway.

This should remove forcing people into this un-necessary conformance.

[ktoso]

@swift-ci please smoke test

[xedin]

Actor system doesn't have to be a concrete nominal type for ActorID to be available (i.e. I think it could be stated as a same-type requirement?), maybe better use getAssociatedTypeOfDistributedSystemOfActor or directly substitute ActorSystem.ActorID dependent member?...

[ktoso]

Good catch, I'll add a test like that and cover it. Same type requirement should be the case you're mentioning yeah

[ktoso]

Heh weirdly enough facing term verification crashes that I don't quite understand when trying to use the getAssociatedTypeOfDistributedSystemOfActor here... I dug around for a while but will need to ask @slavapestov for a hint I think

[ser-0xff]

In the our implementation the ID type was codable from the very beginning because we transmitted it between server and client, but recently we made some changes so do not transmit the whole ID any more. Probably will make sense to remove that conformance.
Thank you for the update.

[ktoso]

@swift-ci please smoke test

[ktoso]

I see, thanks @ser-0xff -- I thought you had run into this issue and were forced to do Codable there.
For what it's worth this should enable you to remove the Codable conformance then.

[ktoso]

@swift-ci please smoke test Linux

[ktoso]

@swift-ci please smoke test Linux

[ktoso]

@swift-ci please smoke test Windows

[ktoso]

This doesn't handle the

protocol Protocol_SerNotCodable_IdCodable: DistributedActor
  where ActorSystem == FakeCustomSerializationRoundtripActorSystem { }

yet but I'm working on it.

Seems the new code in getAssociatedTypeOfDistributedSystemOfActor:

  auto *actorType = actorOrExtension->getSelfNominalTypeDecl();
  if (isa<ProtocolDecl>(actorType))
    return memberTy->getReducedType(sig);

still isn't quite good enough:

(lldb) e sig->getReducedType(memberTy)
Term verification failed
Initial term:    τ_0_0.[DistributedActor:ActorSystem]
Erased term:     τ_0_0.ActorSystem
Simplified term: τ_0_0.[Protocol_SerNotCodable_IdCodable:ActorSystem]

Requirement machine for <τ_0_0 where τ_0_0 : Protocol_SerNotCodable_IdCodable>
Rewrite system: {
- [Protocol_SerNotCodable_IdCodable].[Protocol_SerNotCodable_IdCodable] => [Protocol_SerNotCodable_IdCodable] [permanent]
- [Protocol_SerNotCodable_IdCodable].ActorSystem => [Protocol_SerNotCodable_IdCodable:ActorSystem] [permanent]
- [Protocol_SerNotCodable_IdCodable].SerializationRequirement => [Protocol_SerNotCodable_IdCodable:SerializationRequirement] [permanent]
- [Protocol_SerNotCodable_IdCodable].ID => [Protocol_SerNotCodable_IdCodable:ID] [permanent]
- [Protocol_SerNotCodable_IdCodable].[DistributedActor] => [Protocol_SerNotCodable_IdCodable] [explicit]
- [Protocol_SerNotCodable_IdCodable].[Protocol_SerNotCodable_IdCodable:ActorSystem] => [Protocol_SerNotCodable_IdCodable:ActorSystem]
- [Protocol_SerNotCodable_IdCodable].[Protocol_SerNotCodable_IdCodable:SerializationRequirement] => [Protocol_SerNotCodable_IdCodable:SerializationRequirement]
- [Protocol_SerNotCodable_IdCodable].[Protocol_SerNotCodable_IdCodable:ID] => [Protocol_SerNotCodable_IdCodable:ID]
- τ_0_0.[Protocol_SerNotCodable_IdCodable] => τ_0_0 [explicit]
- τ_0_0.ActorSystem => τ_0_0.[Protocol_SerNotCodable_IdCodable:ActorSystem]
- τ_0_0.SerializationRequirement => τ_0_0.[Protocol_SerNotCodable_IdCodable:SerializationRequirement]
- τ_0_0.ID => τ_0_0.[Protocol_SerNotCodable_IdCodable:ID]
- τ_0_0.[DistributedActor] => τ_0_0
}
Property map: {
  [Protocol_SerNotCodable_IdCodable] => { conforms_to: [Protocol_SerNotCodable_IdCodable DistributedActor] }
  τ_0_0 => { conforms_to: [Protocol_SerNotCodable_IdCodable DistributedActor] }
}
Conformance paths: {
}

[ktoso]

@swift-ci please smoke test

[ktoso]

Turns out we actually don't want to support the inference of Codable for a protocol anyway -- and it would have been cyclic.

So we're good as soon as CI is happy from unrelated issues

[ktoso]

@swift-ci please smoke test macOS