Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions include/swift/AST/DistributedDecl.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,17 @@ Type getAssociatedTypeOfDistributedSystemOfActor(DeclContext *actorOrExtension,
/// Find the concrete invocation decoder associated with the given actor.
NominalTypeDecl *getDistributedActorInvocationDecoder(NominalTypeDecl *);

/// Determine if this distributed actor can synthesize a `Codable` conformance.
/// This is based on the actor's `ID` being `Codable`.
///
/// It is possible for the `ID` to be `Codable` but the
/// `SerializationRequirement` used by the actor (and its actor system to not
/// be `Codable`). In such situation the conformance is synthesized, however
/// the user may need to provide an explicit conformance to the
/// `SerializationRequirement` if they wanted to pass the actor to distributed
/// methods.
bool canSynthesizeDistributedActorCodableConformance(NominalTypeDecl *actor);

/// Find `decodeNextArgument<T>(type: T.Type) -> T` method associated with
/// invocation decoder of the given distributed actor.
FuncDecl *getDistributedActorArgumentDecodingMethod(NominalTypeDecl *);
Expand Down
18 changes: 18 additions & 0 deletions include/swift/AST/TypeCheckRequests.h
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,24 @@ class IsDistributedActorRequest :
bool isCached() const { return true; }
};

/// Determine whether the given class is a distributed actor.
class CanSynthesizeDistributedActorCodableConformanceRequest :
public SimpleRequest<CanSynthesizeDistributedActorCodableConformanceRequest,
bool(NominalTypeDecl *),
RequestFlags::Cached> {
public:
using SimpleRequest::SimpleRequest;

private:
friend SimpleRequest;

bool evaluate(Evaluator &evaluator, NominalTypeDecl *nominal) const;

public:
// Caching
bool isCached() const { return true; }
};

/// Retrieve the implicit conformance for the given distributed actor type to
/// the Codable protocol protocol.
///
Expand Down
3 changes: 3 additions & 0 deletions include/swift/AST/TypeCheckerTypeIDZone.def
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ SWIFT_REQUEST(TypeChecker, IsDistributedActorRequest, bool(NominalTypeDecl *),
SWIFT_REQUEST(TypeChecker, GetDistributedActorImplicitCodableRequest,
NormalProtocolConformance *(NominalTypeDecl *, KnownProtocolKind),
Cached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, CanSynthesizeDistributedActorCodableConformanceRequest,
bool (NominalTypeDecl *),
Cached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, GetDistributedActorSystemRemoteCallFunctionRequest,
AbstractFunctionDecl *(NominalTypeDecl *, bool),
Cached, NoLocationInfo)
Expand Down
4 changes: 3 additions & 1 deletion lib/AST/ConformanceLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "swift/AST/Module.h"
#include "swift/AST/ASTContext.h"
#include "swift/AST/Builtins.h"
#include "swift/AST/DistributedDecl.h"
#include "swift/AST/DiagnosticsSema.h"
#include "swift/AST/ExistentialLayout.h"
#include "swift/AST/GenericEnvironment.h"
Expand Down Expand Up @@ -660,7 +661,8 @@ LookupConformanceInModuleRequest::evaluate(
}
} else if (protocol->isSpecificProtocol(KnownProtocolKind::Encodable) ||
protocol->isSpecificProtocol(KnownProtocolKind::Decodable)) {
if (nominal->isDistributedActor()) {
// if (nominal->isDistributedActor()) {
if (canSynthesizeDistributedActorCodableConformance(nominal)) {
auto protoKind =
protocol->isSpecificProtocol(KnownProtocolKind::Encodable)
? KnownProtocolKind::Encodable
Expand Down
24 changes: 22 additions & 2 deletions lib/AST/DistributedDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,23 @@

using namespace swift;

/******************************************************************************/
/************* Implicit Distributed Actor Codable Conformance *****************/
/******************************************************************************/

bool swift::canSynthesizeDistributedActorCodableConformance(NominalTypeDecl *actor) {
auto &C = actor->getASTContext();

if (!actor->isDistributedActor())
return false;

return evaluateOrDefault(
C.evaluator,
CanSynthesizeDistributedActorCodableConformanceRequest{actor},
false);
}


/******************************************************************************/
/************** Distributed Actor System Associated Types *********************/
/******************************************************************************/
Expand Down Expand Up @@ -210,17 +227,20 @@ Type swift::getDistributedActorSystemInvocationDecoderType(NominalTypeDecl *syst
Type swift::getDistributedSerializationRequirementType(
NominalTypeDecl *nominal, ProtocolDecl *protocol) {
assert(nominal);
assert(protocol);
auto &ctx = nominal->getASTContext();

if (!protocol)
return Type();

// Dig out the serialization requirement type.
auto module = nominal->getParentModule();
Type selfType = nominal->getSelfInterfaceType();
auto conformance = module->lookupConformance(selfType, protocol);
if (conformance.isInvalid())
return Type();

return conformance.getTypeWitnessByName(selfType, ctx.Id_SerializationRequirement);
return conformance.getTypeWitnessByName(selfType,
ctx.Id_SerializationRequirement);
}

AbstractFunctionDecl *
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/ProtocolConformance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1296,7 +1296,7 @@ static SmallVector<ProtocolConformance *, 2> findSynthesizedConformances(
}

/// Distributed actors can synthesize Encodable/Decodable, so look for those
if (nominal->isDistributedActor()) {
if (canSynthesizeDistributedActorCodableConformance(nominal)) {
trySynthesize(KnownProtocolKind::Encodable);
trySynthesize(KnownProtocolKind::Decodable);
}
Expand Down
26 changes: 26 additions & 0 deletions lib/Sema/CodeSynthesisDistributedActor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1043,3 +1043,29 @@ NormalProtocolConformance *GetDistributedActorImplicitCodableRequest::evaluate(
return addDistributedActorCodableConformance(classDecl,
C.getProtocol(protoKind));
}

bool CanSynthesizeDistributedActorCodableConformanceRequest::evaluate(
Evaluator &evaluator, NominalTypeDecl *actor) const {

if (actor && !isa<ClassDecl>(actor))
return false;

if (!actor->isDistributedActor())
return false;

auto systemTy = getConcreteReplacementForProtocolActorSystemType(actor);
if (!systemTy)
return false;

if (!systemTy->getAnyNominal())
return false;

auto idTy = getDistributedActorSystemActorIDType(systemTy->getAnyNominal());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if (!idTy)
return false;

return TypeChecker::conformsToKnownProtocol(
idTy, KnownProtocolKind::Decodable, actor->getParentModule()) &&
TypeChecker::conformsToKnownProtocol(
idTy, KnownProtocolKind::Encodable, actor->getParentModule());
}
2 changes: 2 additions & 0 deletions test/Distributed/Inputs/FakeDistributedActorSystems.swift
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,8 @@ public struct FakeInvocationEncoder : DistributedTargetInvocationEncoder {
var returnType: Any.Type? = nil
var errorType: Any.Type? = nil

public init() {}

public mutating func recordGenericSubstitution<T>(_ type: T.Type) throws {
print(" > encode generic sub: \(String(reflecting: type))")
genericSubs.append(type)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,11 @@
import Distributed

@available(SwiftStdlib 6.0, *)
distributed actor Worker<ActorSystem> where ActorSystem: DistributedActorSystem<any Codable>, ActorSystem.ActorID: Codable {
//distributed actor Worker<ActorSystem> where ActorSystem: DistributedActorSystem<any Codable>, ActorSystem.ActorID: Codable {
distributed actor Worker<ActorSystem> where ActorSystem: DistributedActorSystem<any Codable> {
distributed func hi(name: String) {
print("Hi, \(name)!")
}

nonisolated var description: Swift.String {
"Worker(\(id))"
}
}

// ==== Execute ----------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
// RUN: %empty-directory(%t)
// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/Inputs/FakeDistributedActorSystems.swift
// RUN: %target-swift-frontend -typecheck -verify -disable-availability-checking -I %t 2>&1 %s
// REQUIRES: concurrency
// REQUIRES: distributed

import Distributed
import FakeDistributedActorSystems

distributed actor YesVeryMuchSo {
typealias ActorSystem = FakeRoundtripActorSystem
}
func test_YesVeryMuchSo(_ actor: YesVeryMuchSo) {
let _: any Codable = actor // implicit conformance, ID was Codable

// unrelated protocol
let _: any CustomSerializationProtocol = actor // expected-error{{value of type 'YesVeryMuchSo' does not conform to specified type 'CustomSerializationProtocol'}}
}

// ==== ------------------------------------------------------------------------

distributed actor SerReqNotCodable {
typealias ActorSystem = FakeCustomSerializationRoundtripActorSystem
}
func test_NotAtAll_NoImplicit(_ actor: SerReqNotCodable) {
let _: any Codable = actor // OK, the ID was Codable, even though SerializationRequirement was something else

// no implicit conformance
let _: any CustomSerializationProtocol = actor // expected-error{{value of type 'SerReqNotCodable' does not conform to specified type 'CustomSerializationProtocol'}}
}

// ==== ------------------------------------------------------------------------

distributed actor NotAtAll_NothingCodable {
typealias ActorSystem = FakeIdIsNotCodableActorSystem
}
func test_NotAtAll_NoImplicit(_ actor: NotAtAll_NothingCodable) {
let _: any Codable = actor // expected-error{{value of type 'NotAtAll_NothingCodable' does not conform to specified type 'Decodable'}}

// no implicit conformance
let _: any CustomSerializationProtocol = actor // expected-error{{value of type 'NotAtAll_NothingCodable' does not conform to specified type 'CustomSerializationProtocol'}}
}

public struct NotCodableID: Sendable, Hashable {}

@available(SwiftStdlib 5.7, *)
public struct FakeIdIsNotCodableActorSystem: DistributedActorSystem, CustomStringConvertible {
public typealias ActorID = NotCodableID
public typealias InvocationDecoder = FakeInvocationDecoder
public typealias InvocationEncoder = FakeInvocationEncoder
public typealias SerializationRequirement = Codable
public typealias ResultHandler = FakeRoundtripResultHandler

// just so that the struct does not become "trivial"
let someValue: String = ""
let someValue2: String = ""
let someValue3: String = ""
let someValue4: String = ""

public init() {
}

public func resolve<Act>(id: ActorID, as actorType: Act.Type) throws -> Act?
where Act: DistributedActor,
Act.ID == ActorID {
nil
}

public func assignID<Act>(_ actorType: Act.Type) -> ActorID
where Act: DistributedActor,
Act.ID == ActorID {
.init()
}

public func actorReady<Act>(_ actor: Act)
where Act: DistributedActor,
Act.ID == ActorID {
}

public func resignID(_ id: ActorID) {
}

public func makeInvocationEncoder() -> InvocationEncoder {
.init()
}

public func remoteCall<Act, Err, Res>(
on actor: Act,
target: RemoteCallTarget,
invocation invocationEncoder: inout InvocationEncoder,
throwing: Err.Type,
returning: Res.Type
) async throws -> Res
where Act: DistributedActor,
Act.ID == ActorID,
Err: Error,
Res: SerializationRequirement {
throw ExecuteDistributedTargetError(message: "\(#function) not implemented.")
}

public func remoteCallVoid<Act, Err>(
on actor: Act,
target: RemoteCallTarget,
invocation invocationEncoder: inout InvocationEncoder,
throwing: Err.Type
) async throws
where Act: DistributedActor,
Act.ID == ActorID,
Err: Error {
throw ExecuteDistributedTargetError(message: "\(#function) not implemented.")
}

public nonisolated var description: Swift.String {
"\(Self.self)()"
}
}
15 changes: 8 additions & 7 deletions test/Distributed/distributed_actor_protocol_isolation.swift
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// RUN: %empty-directory(%t)
// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/Inputs/FakeDistributedActorSystems.swift
// RUN: %target-swift-frontend-emit-module -I %t -emit-module-path %t/distributed_actor_protocol_isolation.swiftmodule -module-name distributed_actor_protocol_isolation -disable-availability-checking %s
// X: %target-swift-frontend -typecheck -verify -disable-availability-checking -I %t 2>&1 %s
// RUN: %target-swift-frontend -typecheck -verify -disable-availability-checking -I %t 2>&1 %s
// REQUIRES: concurrency
// REQUIRES: distributed

Expand All @@ -15,7 +14,7 @@ protocol Greeting: DistributedActor {
}

extension Greeting {
func greetLocal(name: String) {
func greetLocal(name: String) { // expected-note{{distributed actor-isolated instance method 'greetLocal(name:)' declared here}}
print("\(greeting()), \(name)!") // okay, we're on the actor
}
}
Expand All @@ -28,10 +27,12 @@ extension Greeting where SerializationRequirement == Codable {
}
}

extension Greeting {
extension Greeting where Self.SerializationRequirement == Codable {
nonisolated func greetAliceALot() async throws {
// try await greetDistributed(name: "Alice") // okay, via Codable
// let rawGreeting = try await greeting() // okay, via Self's serialization requirement
// greetLocal(name: "Alice") // expected-error{{only 'distributed' instance methods can be called on a potentially remote distributed actor}}
try await self.greetDistributed(name: "Alice") // okay, via Codable
let rawGreeting = try await greeting() // okay, via Self's serialization requirement
_ = rawGreeting

greetLocal(name: "Alice") // expected-error{{only 'distributed' instance methods can be called on a potentially remote distributed actor}}
}
}
8 changes: 1 addition & 7 deletions test/Distributed/distributed_actor_system_missing.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@ distributed actor DA {
// expected-error@-1{{distributed actor 'DA' does not declare ActorSystem it can be used with}}
// expected-error@-2 {{type 'DA' does not conform to protocol 'DistributedActor'}}

// Since synthesis would have failed due to the missing ActorSystem:
// expected-error@-5{{type 'DA' does not conform to protocol 'Encodable'}}
// expected-error@-6{{type 'DA' does not conform to protocol 'Decodable'}}

// expected-note@-8{{you can provide a module-wide default actor system by declaring:}}
// expected-note@-4{{you can provide a module-wide default actor system by declaring:}}

// Note to add the typealias is diagnosed on the protocol:
// _Distributed.DistributedActor:3:20: note: diagnostic produced elsewhere: protocol requires nested type 'ActorSystem'; do you want to add it?
Expand All @@ -27,8 +23,6 @@ distributed actor DA {
distributed actor Server { // expected-error 2 {{distributed actor 'Server' does not declare ActorSystem it can be used with}}
// expected-error@-1 {{type 'Server' does not conform to protocol 'DistributedActor'}}
// expected-note@-2{{you can provide a module-wide default actor system by declaring:}}
// expected-error@-3{{type 'Server' does not conform to protocol 'Encodable'}}
// expected-error@-4{{type 'Server' does not conform to protocol 'Decodable'}}
typealias ActorSystem = DoesNotExistDataSystem
// expected-error@-1{{cannot find type 'DoesNotExistDataSystem' in scope}}
typealias SerializationRequirement = any Codable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ distributed actor Fish {
// expected-error@-2{{distributed actor 'Fish' does not declare ActorSystem it can be used with}}
// expected-error@-3{{type 'Fish' does not conform to protocol 'DistributedActor'}}
// expected-note@-4{{you can provide a module-wide default actor system by declaring:\ntypealias DefaultDistributedActorSystem = <#ConcreteActorSystem#>}}
// expected-error@-5{{type 'Fish' does not conform to protocol 'Encodable'}}
// expected-error@-6{{type 'Fish' does not conform to protocol 'Decodable'}}

distributed func tell(_ text: String, by: Fish) {
// What would the fish say, if it could talk?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ distributed actor ProtocolWithChecksSeqReqDA_MissingSystem: ProtocolWithChecksSe
// expected-error@-4{{distributed actor 'ProtocolWithChecksSeqReqDA_MissingSystem' does not declare ActorSystem it can be used with}}
//
// expected-error@-6{{type 'ProtocolWithChecksSeqReqDA_MissingSystem' does not conform to protocol 'DistributedActor'}}
// expected-error@-7{{type 'ProtocolWithChecksSeqReqDA_MissingSystem' does not conform to protocol 'Encodable'}}
// expected-error@-8{{type 'ProtocolWithChecksSeqReqDA_MissingSystem' does not conform to protocol 'Decodable'}}

// Entire conformance is doomed, so we didn't proceed to checking the functions; that's fine
distributed func testAT() async throws -> NotCodable { .init() }
Expand Down