Skip to content

[Distributed] Missing diagnostic for onReturn ad-hoc req when it is mutating witness #59162

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.

Sign up for GitHub

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 2 commits into from
May 31, 2022
Merged

[Distributed] Missing diagnostic for onReturn ad-hoc req when it is mutating witness #59162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b0b389
[Distributed] onReturn ad-hoc req must not be mutating
ktoso May 31, 2022
402501d
[Distributed] Also prevent mutating remoteCall impls
ktoso May 31, 2022
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
15 changes: 15 additions & 0 deletions lib/AST/DistributedDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,11 @@ bool AbstractFunctionDecl::isDistributedActorSystemRemoteCall(bool isVoidReturn)
return false;
}

auto *func = dyn_cast<FuncDecl>(this);
if (!func) {
return false;
}

// === Structural Checks
// -- Must be throwing
if (!hasThrows()) {
Expand All @@ -415,6 +420,11 @@ bool AbstractFunctionDecl::isDistributedActorSystemRemoteCall(bool isVoidReturn)
return false;
}

// -- Must not be mutating, use classes to implement a system instead
if (func->isMutating()) {
return false;
}

// === Check generics
if (!isGeneric()) {
return false;
Expand Down Expand Up @@ -1166,6 +1176,11 @@ AbstractFunctionDecl::isDistributedTargetInvocationResultHandlerOnReturn() const
return false;
}

// --- must not be mutating
if (func->isMutating()) {
return false;
}

// === Check generics
if (!isGeneric()) {
return false;
Expand Down
17 changes: 8 additions & 9 deletions lib/Sema/TypeCheckDistributed.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ bool swift::checkDistributedActorSystemAdHocProtocolRequirements(
decl->getDescriptiveKind(), decl->getName(), identifier);
decl->diagnose(
diag::note_distributed_actor_system_conformance_missing_adhoc_requirement,
decl->getName(), identifier,
Proto->getName(), identifier,
"func remoteCall<Act, Err, Res>(\n"
" on actor: Act,\n"
" target: RemoteCallTarget,\n"
Expand All @@ -274,7 +274,7 @@ bool swift::checkDistributedActorSystemAdHocProtocolRequirements(
decl->getDescriptiveKind(), decl->getName(), identifier);
decl->diagnose(
diag::note_distributed_actor_system_conformance_missing_adhoc_requirement,
decl->getName(), identifier,
Proto->getName(), identifier,
"func remoteCallVoid<Act, Err>(\n"
" on actor: Act,\n"
" target: RemoteCallTarget,\n"
Expand Down Expand Up @@ -303,7 +303,7 @@ bool swift::checkDistributedActorSystemAdHocProtocolRequirements(
diag::distributed_actor_system_conformance_missing_adhoc_requirement,
decl->getDescriptiveKind(), decl->getName(), identifier);
decl->diagnose(diag::note_distributed_actor_system_conformance_missing_adhoc_requirement,
decl->getName(), identifier,
Proto->getName(), identifier,
"mutating func recordArgument<Value: SerializationRequirement>(_ argument: RemoteCallArgument<Value>) throws\n");
anyMissingAdHocRequirements = true;
}
Expand All @@ -318,7 +318,7 @@ bool swift::checkDistributedActorSystemAdHocProtocolRequirements(
diag::distributed_actor_system_conformance_missing_adhoc_requirement,
decl->getDescriptiveKind(), decl->getName(), identifier);
decl->diagnose(diag::note_distributed_actor_system_conformance_missing_adhoc_requirement,
decl->getName(), identifier,
Proto->getName(), identifier,
"mutating func recordReturnType<Res: SerializationRequirement>(_ resultType: Res.Type) throws\n");
anyMissingAdHocRequirements = true;
}
Expand All @@ -339,7 +339,7 @@ bool swift::checkDistributedActorSystemAdHocProtocolRequirements(
diag::distributed_actor_system_conformance_missing_adhoc_requirement,
decl->getDescriptiveKind(), decl->getName(), identifier);
decl->diagnose(diag::note_distributed_actor_system_conformance_missing_adhoc_requirement,
decl->getName(), identifier,
Proto->getName(), identifier,
"mutating func decodeNextArgument<Argument: SerializationRequirement>() throws -> Argument\n");
anyMissingAdHocRequirements = true;
}
Expand All @@ -360,10 +360,9 @@ bool swift::checkDistributedActorSystemAdHocProtocolRequirements(
diag::distributed_actor_system_conformance_missing_adhoc_requirement,
decl->getDescriptiveKind(), decl->getName(), identifier);
decl->diagnose(
diag::
note_distributed_actor_system_conformance_missing_adhoc_requirement,
decl->getName(), identifier,
"mutating func onReturn<Success: SerializationRequirement>(value: "
diag::note_distributed_actor_system_conformance_missing_adhoc_requirement,
Proto->getName(), identifier,
"func onReturn<Success: SerializationRequirement>(value: "
"Success) async throws\n");
anyMissingAdHocRequirements = true;
}
Expand Down
110 changes: 91 additions & 19 deletions test/Distributed/distributed_actor_system_missing_adhoc_requirement_impls.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import Distributed

struct MissingRemoteCall: DistributedActorSystem {
// expected-error@-1{{struct 'MissingRemoteCall' is missing witness for protocol requirement 'remoteCall'}}
// expected-note@-2{{protocol 'MissingRemoteCall' requires function 'remoteCall' with signature:}}
// expected-note@-2{{protocol 'DistributedActorSystem' requires function 'remoteCall' with signature:}}

// expected-error@-4{{struct 'MissingRemoteCall' is missing witness for protocol requirement 'remoteCallVoid'}}
// expected-note@-5{{protocol 'MissingRemoteCall' requires function 'remoteCallVoid' with signature:}}
// expected-note@-5{{protocol 'DistributedActorSystem' requires function 'remoteCallVoid' with signature:}}

typealias ActorID = ActorAddress
typealias InvocationDecoder = FakeInvocationDecoder
Expand Down Expand Up @@ -41,12 +41,73 @@ struct MissingRemoteCall: DistributedActorSystem {
}
}

struct RemoteCallMutating: DistributedActorSystem {
// expected-error@-1{{struct 'RemoteCallMutating' is missing witness for protocol requirement 'remoteCall'}}
// expected-note@-2{{protocol 'DistributedActorSystem' requires function 'remoteCall' with signature:}}

// expected-error@-4{{struct 'RemoteCallMutating' is missing witness for protocol requirement 'remoteCallVoid'}}
// expected-note@-5{{protocol 'DistributedActorSystem' requires function 'remoteCallVoid' with signature:}}

typealias ActorID = ActorAddress
typealias InvocationDecoder = FakeInvocationDecoder
typealias InvocationEncoder = FakeInvocationEncoder
typealias SerializationRequirement = Codable
typealias ResultHandler = FakeResultHandler

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

func assignID<Act>(_ actorType: Act.Type) -> ActorID
where Act: DistributedActor {
ActorAddress(parse: "fake://123")
}

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

func resignID(_ id: ActorID) {
}

mutating 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 {
fatalError("NOT IMPLEMENTED \(#function)")
}

mutating func remoteCallVoid<Act, Err>(
on actor: Act,
target: RemoteCallTarget,
invocation: inout InvocationEncoder,
throwing: Err.Type
) async throws
where Act: DistributedActor,
Act.ID == ActorID,
Err: Error {
fatalError("NOT IMPLEMENTED \(#function)")
}

func makeInvocationEncoder() -> InvocationEncoder {
}
}

struct MissingRemoteCall_missingInout_on_encoder: DistributedActorSystem {
// expected-error@-1{{struct 'MissingRemoteCall_missingInout_on_encoder' is missing witness for protocol requirement 'remoteCall'}}
// expected-note@-2{{protocol 'MissingRemoteCall_missingInout_on_encoder' requires function 'remoteCall' with signature:}}
// expected-note@-2{{protocol 'DistributedActorSystem' requires function 'remoteCall' with signature:}}

// expected-error@-4{{struct 'MissingRemoteCall_missingInout_on_encoder' is missing witness for protocol requirement 'remoteCallVoid'}}
// expected-note@-5{{protocol 'MissingRemoteCall_missingInout_on_encoder' requires function 'remoteCallVoid' with signature:}}
// expected-note@-5{{protocol 'DistributedActorSystem' requires function 'remoteCallVoid' with signature:}}

typealias ActorID = ActorAddress
typealias InvocationDecoder = FakeInvocationDecoder
Expand Down Expand Up @@ -160,10 +221,10 @@ struct MissingRemoteCall_missing_makeInvocationEncoder: DistributedActorSystem {

struct Error_wrongReturn: DistributedActorSystem {
// expected-error@-1{{struct 'Error_wrongReturn' is missing witness for protocol requirement 'remoteCall'}}
// expected-note@-2{{protocol 'Error_wrongReturn' requires function 'remoteCall' with signature:}}
// expected-note@-2{{protocol 'DistributedActorSystem' requires function 'remoteCall' with signature:}}

// expected-error@-4{{struct 'Error_wrongReturn' is missing witness for protocol requirement 'remoteCallVoid'}}
// expected-note@-5{{protocol 'Error_wrongReturn' requires function 'remoteCallVoid' with signature:}}
// expected-note@-5{{protocol 'DistributedActorSystem' requires function 'remoteCallVoid' with signature:}}

typealias ActorID = ActorAddress
typealias InvocationDecoder = FakeInvocationDecoder
Expand Down Expand Up @@ -235,10 +296,10 @@ struct Error_wrongReturn: DistributedActorSystem {

struct BadRemoteCall_param: DistributedActorSystem {
// expected-error@-1{{struct 'BadRemoteCall_param' is missing witness for protocol requirement 'remoteCall'}}
// expected-note@-2{{protocol 'BadRemoteCall_param' requires function 'remoteCall' with signature:}}
// expected-note@-2{{protocol 'DistributedActorSystem' requires function 'remoteCall' with signature:}}

// expected-error@-4{{struct 'BadRemoteCall_param' is missing witness for protocol requirement 'remoteCallVoid'}}
// expected-note@-5{{protocol 'BadRemoteCall_param' requires function 'remoteCallVoid' with signature:}}
// expected-note@-5{{protocol 'DistributedActorSystem' requires function 'remoteCallVoid' with signature:}}

typealias ActorID = ActorAddress
typealias InvocationDecoder = FakeInvocationDecoder
Expand Down Expand Up @@ -345,7 +406,7 @@ public struct BadRemoteCall_notPublic: DistributedActorSystem {

public struct BadRemoteCall_badResultConformance: DistributedActorSystem {
// expected-error@-1{{struct 'BadRemoteCall_badResultConformance' is missing witness for protocol requirement 'remoteCall'}}
// expected-note@-2{{protocol 'BadRemoteCall_badResultConformance' requires function 'remoteCall' with signature:}}
// expected-note@-2{{protocol 'DistributedActorSystem' requires function 'remoteCall' with signature:}}

public typealias ActorID = ActorAddress
public typealias InvocationDecoder = PublicFakeInvocationDecoder
Expand Down Expand Up @@ -452,7 +513,7 @@ struct BadRemoteCall_largeSerializationRequirement: DistributedActorSystem {

struct BadRemoteCall_largeSerializationRequirementSlightlyOffInDefinition: DistributedActorSystem {
// expected-error@-1{{struct 'BadRemoteCall_largeSerializationRequirementSlightlyOffInDefinition' is missing witness for protocol requirement 'remoteCall'}}
// expected-note@-2{{protocol 'BadRemoteCall_largeSerializationRequirementSlightlyOffInDefinition' requires function 'remoteCall' with signature:}}
// expected-note@-2{{protocol 'DistributedActorSystem' requires function 'remoteCall' with signature:}}

typealias ActorID = ActorAddress
typealias InvocationDecoder = LargeSerializationReqFakeInvocationDecoder
Expand Down Expand Up @@ -608,7 +669,7 @@ public struct PublicFakeInvocationEncoder: DistributedTargetInvocationEncoder {

struct FakeInvocationEncoder_missing_recordArgument: DistributedTargetInvocationEncoder {
//expected-error@-1{{struct 'FakeInvocationEncoder_missing_recordArgument' is missing witness for protocol requirement 'recordArgument'}}
//expected-note@-2{{protocol 'FakeInvocationEncoder_missing_recordArgument' requires function 'recordArgument' with signature:}}
//expected-note@-2{{protocol 'DistributedTargetInvocationEncoder' requires function 'recordArgument' with signature:}}
typealias SerializationRequirement = Codable

mutating func recordGenericSubstitution<T>(_ type: T.Type) throws {}
Expand All @@ -620,7 +681,7 @@ struct FakeInvocationEncoder_missing_recordArgument: DistributedTargetInvocation

struct FakeInvocationEncoder_missing_recordArgument2: DistributedTargetInvocationEncoder {
//expected-error@-1{{struct 'FakeInvocationEncoder_missing_recordArgument2' is missing witness for protocol requirement 'recordArgument'}}
//expected-note@-2{{protocol 'FakeInvocationEncoder_missing_recordArgument2' requires function 'recordArgument' with signature:}}
//expected-note@-2{{protocol 'DistributedTargetInvocationEncoder' requires function 'recordArgument' with signature:}}
typealias SerializationRequirement = Codable

mutating func recordGenericSubstitution<T>(_ type: T.Type) throws {}
Expand All @@ -632,7 +693,7 @@ struct FakeInvocationEncoder_missing_recordArgument2: DistributedTargetInvocatio

struct FakeInvocationEncoder_missing_recordReturnType: DistributedTargetInvocationEncoder {
//expected-error@-1{{struct 'FakeInvocationEncoder_missing_recordReturnType' is missing witness for protocol requirement 'recordReturnType'}}
//expected-note@-2{{protocol 'FakeInvocationEncoder_missing_recordReturnType' requires function 'recordReturnType' with signature:}}
//expected-note@-2{{protocol 'DistributedTargetInvocationEncoder' requires function 'recordReturnType' with signature:}}
typealias SerializationRequirement = Codable

mutating func recordGenericSubstitution<T>(_ type: T.Type) throws {}
Expand All @@ -655,7 +716,7 @@ struct FakeInvocationEncoder_missing_recordErrorType: DistributedTargetInvocatio

struct FakeInvocationEncoder_recordArgument_wrongType: DistributedTargetInvocationEncoder {
//expected-error@-1{{struct 'FakeInvocationEncoder_recordArgument_wrongType' is missing witness for protocol requirement 'recordArgument'}}
//expected-note@-2{{protocol 'FakeInvocationEncoder_recordArgument_wrongType' requires function 'recordArgument' with signature:}}
//expected-note@-2{{protocol 'DistributedTargetInvocationEncoder' requires function 'recordArgument' with signature:}}
typealias SerializationRequirement = Codable

mutating func recordGenericSubstitution<T>(_ type: T.Type) throws {}
Expand All @@ -668,7 +729,7 @@ struct FakeInvocationEncoder_recordArgument_wrongType: DistributedTargetInvocati
}
struct FakeInvocationEncoder_recordArgument_missingMutating: DistributedTargetInvocationEncoder {
//expected-error@-1{{struct 'FakeInvocationEncoder_recordArgument_missingMutating' is missing witness for protocol requirement 'recordArgument'}}
//expected-note@-2{{protocol 'FakeInvocationEncoder_recordArgument_missingMutating' requires function 'recordArgument' with signature:}}
//expected-note@-2{{protocol 'DistributedTargetInvocationEncoder' requires function 'recordArgument' with signature:}}
typealias SerializationRequirement = Codable

mutating func recordGenericSubstitution<T>(_ type: T.Type) throws {}
Expand All @@ -680,7 +741,7 @@ struct FakeInvocationEncoder_recordArgument_missingMutating: DistributedTargetIn

struct FakeInvocationEncoder_recordResultType_wrongType: DistributedTargetInvocationEncoder {
//expected-error@-1{{struct 'FakeInvocationEncoder_recordResultType_wrongType' is missing witness for protocol requirement 'recordReturnType'}}
//expected-note@-2{{protocol 'FakeInvocationEncoder_recordResultType_wrongType' requires function 'recordReturnType' with signature:}}
//expected-note@-2{{protocol 'DistributedTargetInvocationEncoder' requires function 'recordReturnType' with signature:}}
typealias SerializationRequirement = Codable

mutating func recordGenericSubstitution<T>(_ type: T.Type) throws {}
Expand Down Expand Up @@ -757,7 +818,7 @@ public final class PublicFakeInvocationDecoder_badNotPublic: DistributedTargetIn

final class PublicFakeInvocationDecoder_badBadProtoRequirement: DistributedTargetInvocationDecoder {
// expected-error@-1{{class 'PublicFakeInvocationDecoder_badBadProtoRequirement' is missing witness for protocol requirement 'decodeNextArgument'}}
// expected-note@-2{{protocol 'PublicFakeInvocationDecoder_badBadProtoRequirement' requires function 'decodeNextArgument' with signature:}}
// expected-note@-2{{protocol 'DistributedTargetInvocationDecoder' requires function 'decodeNextArgument' with signature:}}
typealias SerializationRequirement = Codable

func decodeGenericSubstitutions() throws -> [Any.Type] { [] }
Expand Down Expand Up @@ -809,23 +870,34 @@ struct LargeSerializationReqFakeInvocationResultHandler: DistributedTargetInvoca

struct BadResultHandler_missingOnReturn: DistributedTargetInvocationResultHandler {
// expected-error@-1{{struct 'BadResultHandler_missingOnReturn' is missing witness for protocol requirement 'onReturn'}}
// expected-note@-2{{protocol 'BadResultHandler_missingOnReturn' requires function 'onReturn' with signature:}}
// expected-note@-2{{protocol 'DistributedTargetInvocationResultHandler' requires function 'onReturn' with signature:}}
typealias SerializationRequirement = Codable

// func onReturn<Res: SerializationRequirement>(value: Res) async throws {} // MISSING
func onReturnVoid() async throws {}
func onThrow<Err: Error>(error: Err) async throws {}
}

struct BadResultHandler_missingRequirement: DistributedTargetInvocationResultHandler {
// expected-error@-1{{struct 'BadResultHandler_missingRequirement' is missing witness for protocol requirement 'onReturn'}}
// expected-note@-2{{protocol 'BadResultHandler_missingRequirement' requires function 'onReturn' with signature:}}
// expected-note@-2{{protocol 'DistributedTargetInvocationResultHandler' requires function 'onReturn' with signature:}}
typealias SerializationRequirement = Codable

func onReturn<Success>(value: Success) async throws {} // MISSING : Codable
func onReturnVoid() async throws {}
func onThrow<Err: Error>(error: Err) async throws {}
}

struct BadResultHandler_mutatingButShouldNotBe: DistributedTargetInvocationResultHandler {
// expected-error@-1{{struct 'BadResultHandler_mutatingButShouldNotBe' is missing witness for protocol requirement 'onReturn'}}
// expected-note@-2{{protocol 'DistributedTargetInvocationResultHandler' requires function 'onReturn' with signature:}}
typealias SerializationRequirement = Codable

mutating func onReturn<Success: Codable>(value: Success) async throws {} // WRONG: can't be mutating
func onReturnVoid() async throws {}
func onThrow<Err: Error>(error: Err) async throws {}
}

public struct PublicFakeResultHandler: DistributedTargetInvocationResultHandler {
public typealias SerializationRequirement = Codable

Expand Down
Loading