From 6ca5ace32de4451f36d6888a4a1a65422b40c3a2 Mon Sep 17 00:00:00 2001 From: Konrad 'ktoso' Malawski Date: Wed, 2 Apr 2025 18:41:57 +0900 Subject: [PATCH] [Distributed] Harden detecting adhoc req methods, don't crash when missing protocol decls This issue manifested in crashing when in Xcode one would do the Archive workflow, and we would be missing the Distributed module types and proceed to run into a nullpointer when faced with code like this: ``` public class TestViewModel { public init() {} public func onReturn() { print("on return executed!") } } ``` where the name matched one of the ad hoc requirements, but we'd get null for the protocol lookup since this does not import the distributed module. resolves rdar://148327936 --- lib/AST/DistributedDecl.cpp | 22 ++++- ...ed_actor_adhoc_names_in_non_da_types.swift | 82 +++++++++++++++++++ 2 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 test/Distributed/distributed_actor_adhoc_names_in_non_da_types.swift diff --git a/lib/AST/DistributedDecl.cpp b/lib/AST/DistributedDecl.cpp index 3877139eefb39..8ab8b8ed5a4fb 100644 --- a/lib/AST/DistributedDecl.cpp +++ b/lib/AST/DistributedDecl.cpp @@ -488,8 +488,10 @@ bool AbstractFunctionDecl::isDistributedActorSystemRemoteCall(bool isVoidReturn) } // === Must be declared in a 'DistributedActorSystem' conforming type - ProtocolDecl *systemProto = - C.getDistributedActorSystemDecl(); + ProtocolDecl *systemProto = C.getDistributedActorSystemDecl(); + if (!systemProto) { + return false; + } auto systemNominal = DC->getSelfNominalTypeDecl(); auto distSystemConformance = lookupConformance( @@ -794,6 +796,9 @@ AbstractFunctionDecl::isDistributedTargetInvocationEncoderRecordArgument() const // === Must be declared in a 'DistributedTargetInvocationEncoder' conforming type ProtocolDecl *encoderProto = C.getProtocol(KnownProtocolKind::DistributedTargetInvocationEncoder); + if (!encoderProto) { + return false; + } auto encoderNominal = getDeclContext()->getSelfNominalTypeDecl(); auto protocolConformance = lookupConformance( @@ -925,6 +930,9 @@ AbstractFunctionDecl::isDistributedTargetInvocationEncoderRecordReturnType() con // === Must be declared in a 'DistributedTargetInvocationEncoder' conforming type ProtocolDecl *encoderProto = C.getProtocol(KnownProtocolKind::DistributedTargetInvocationEncoder); + if (!encoderProto) { + return false; + } auto encoderNominal = getDeclContext()->getSelfNominalTypeDecl(); auto protocolConformance = lookupConformance( @@ -1051,6 +1059,9 @@ AbstractFunctionDecl::isDistributedTargetInvocationEncoderRecordErrorType() cons // === Must be declared in a 'DistributedTargetInvocationEncoder' conforming type ProtocolDecl *encoderProto = C.getProtocol(KnownProtocolKind::DistributedTargetInvocationEncoder); + if (!encoderProto) { + return false; + } auto encoderNominal = getDeclContext()->getSelfNominalTypeDecl(); auto protocolConformance = lookupConformance( @@ -1158,7 +1169,9 @@ AbstractFunctionDecl::isDistributedTargetInvocationDecoderDecodeNextArgument() c // === Must be declared in a 'DistributedTargetInvocationEncoder' conforming type ProtocolDecl *decoderProto = C.getProtocol(KnownProtocolKind::DistributedTargetInvocationDecoder); - + if (!decoderProto) { + return false; + } auto decoderNominal = getDeclContext()->getSelfNominalTypeDecl(); auto protocolConformance = lookupConformance( decoderNominal->getDeclaredInterfaceType(), decoderProto); @@ -1251,6 +1264,9 @@ AbstractFunctionDecl::isDistributedTargetInvocationResultHandlerOnReturn() const // === Must be declared in a 'DistributedTargetInvocationEncoder' conforming type ProtocolDecl *decoderProto = C.getProtocol(KnownProtocolKind::DistributedTargetInvocationResultHandler); + if (!decoderProto) { + return false; + } auto decoderNominal = getDeclContext()->getSelfNominalTypeDecl(); auto protocolConformance = lookupConformance( diff --git a/test/Distributed/distributed_actor_adhoc_names_in_non_da_types.swift b/test/Distributed/distributed_actor_adhoc_names_in_non_da_types.swift new file mode 100644 index 0000000000000..8516b1e3742df --- /dev/null +++ b/test/Distributed/distributed_actor_adhoc_names_in_non_da_types.swift @@ -0,0 +1,82 @@ +// RUN: %empty-directory(%t) +// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -target %target-swift-5.7-abi-triple %S/Inputs/FakeDistributedActorSystems.swift +// RUN: %target-swift-frontend -typecheck -verify -target %target-swift-5.7-abi-triple -I %t 2>&1 %s + +// UNSUPPORTED: back_deploy_concurrency +// REQUIRES: concurrency +// REQUIRES: distributed + +import Distributed +import FakeDistributedActorSystems + +distributed actor Philosopher { + typealias ActorSystem = FakeActorSystem + + let philosophy: String + + init(system: FakeActorSystem) { + philosophy = "Epistemology" + } + + distributed func hi1() -> String { "Hi!" } +// distributed func hi2() -> String { "Hi!" } +// distributed func hi3() -> String { "Hi!" } +// distributed func hi4() -> String { "Hi!" } +// distributed func hi5() -> String { "Hi!" } +// distributed func hi6() -> String { "Hi!" } +// distributed func hi7() -> String { "Hi!" } +// distributed func hi8() -> String { "Hi!" } +// distributed func hi9() -> String { "Hi!" } + + func test(other: Philosopher) async throws { + // self -------------------------------------------------------------------- + async let alet = self.hi1() // none + _ = await alet // Ok - `self.hi()` isn't throwing + +// Task { +// _ = self.hi1() // none +// } +// +// Task.detached { +// _ = await self.hi2() // async +// } + } + +// // other ------------------------------------------------------------------- +// +// async let otherLet = other.hi3() // hi = async throws because of `other` +// _ = try await otherLet +// +// Task { +// _ = try await other.hi4() // hi = async throws +// } +// +// Task.detached { +// _ = try await other.hi5() // hi = async throws +// } +// +// // known to be local ------------------------------------------------------- +// +// // FIXME(distributed): relies on the "secretly known to be local" hack in typechecking +// let _: String? = await other.whenLocal { __secretlyKnownToBeLocal in +// // we're able to get state of what would otherwise be distributed-isolated: +// __secretlyKnownToBeLocal.philosophy +// } +// } +// +// static func test(iso: isolated Philosopher) async throws { +// _ = iso.h6() // we're "in the actor" already, since isolated +// +// // isolated parameter ------------------------------------------------------ +// async let otherLet = iso.hi7() // async +// _ = await otherLet +// +// Task { +// _ = await iso.hi8() // none +// } +// +// Task.detached { +// _ = await iso.hi9() // async +// } +// } +}