[Sema] Fix and cleanup distributed `id`/`actorSystem` synthesis #85245

hamishknight · 2025-10-31T17:17:58Z

Ensure associatedtype inference never considers id and actorSystem properties for a distributed actor since doing so would either result in cycles or resolving using Identifiable's default id
Consolidate synthesis logic that was previously duplicated across witness derivation and separate requests to trigger synthesis
Always synthesize id and actorSystem for a distributed actor, allowing redeclaration checking to diagnose user-defined versions
Lazily compute the interface types for id and actorSystem, ensuring the synthesis logic doesn't kick semantic requests
Install id and actorSystem as part of synthesizeSemanticMembersIfNeeded to ensure they're always injected into name lookup results (this is the actual fix for rdar://162800185)
Remove Identifiable derivation code and ImplicitMemberAction::ResolveDistributedActor, neither of which should be necessary now

rdar://162800185

xedin · 2025-11-02T15:03:37Z

Thanks, @hamishknight! I will take a look on Monday.

lib/Sema/CodeSynthesis.cpp

ktoso · 2025-11-03T00:33:28Z

Thanks a lot Hamish! Looks promising, I'll do a proper review and also verify on some projects, thank you!

And use this in `evaluateMembersRequest`.

Avoid relying on the cycle detection logic to exclude these and enforce that we never consider them since computing their interface type relies on the type witnesses already being resolved. This also means we can no longer pick up Identifiable's default implementation `id`, which we never want for a `distributed` actor. This then lets us remove the special-cased eager inference logic.

…Request` This is checking a conformance for something defined in the Distributed module itself, so things would be very broken if it failed.

…ystemOfActor` Make sure we produce an error for a broken stdlib and produce ErrorType instead.

These can be inlined into their respective requests.

Have the witness derivation logic call into the synthesis requests.

Only fallback to name lookup for serialized modules and swift interfaces. This means we can then implement the user-declared `id`/`actorSystem` error as part of redeclaration checking.

Delay their interface type computation until they are actually type-checked. This ensures their synthesis logic doesn't produce cycles.

Now that the synthesis logic for these doesn't involve kicking semantic requests we can synthesize directly as part of name lookup. This ensures they get synthesized for secondaries and we don't try and use Identifiable's default `id`. rdar://162800185

This shouldn't be necessary anymore since we we'll no longer try to infer `ID` from Identifiable's default implmentation of `id`.

The member names that triggered this don't even match the requirements in the Distributed module anymore, and `installDistributedActorIfNecessary` was only called for structs which can't be distributed actors. Rip it out.

hamishknight · 2025-11-03T13:22:02Z

Noticed we could replace a couple more checks for id/actorSystem, pushed an extra commit for that

Use `isSpecialDistributedProperty` or the synthesis requests instead. While here, also make `isSpecialDistributedProperty` agnostic to interface and serialized module cases.

hamishknight · 2025-11-03T13:31:09Z

@swift-ci please test

hamishknight · 2025-11-03T13:31:18Z

@swift-ci please test source compatibility

xedin

Excellent, thank you!

lib/Sema/CodeSynthesisDistributedActor.cpp

ktoso

Thank you, I also confirmed in an adopter project this solves the issue!

ktoso · 2025-11-01T10:52:04Z

include/swift/AST/Decl.h

                 bool *wouldConflictInSwift5 = nullptr,
                 bool skipProtocolExtensionCheck = false);

+enum class SpecialDistributedProperty {


minor:

Suggested change

enum class SpecialDistributedProperty {

enum class SpecialDistributedActorProperty {

Just to that it's clearer that we mean specifically properties of a distributed actor; (a "distributed property" exists and is distributed var {} but that's different)

I'll do a follow up with a rename, we can merge this as is.

I went ahead and fixed this in #85309 since there were a couple of other things I also noticed

ktoso · 2025-11-03T01:23:19Z

test/decl/protocol/special/DistributedActor.swift

+  nonisolated var actorSystem: DefaultDistributedActorSystem { fatalError() }
+  // expected-error@-1 {{property 'actorSystem' cannot be defined explicitly, as it conflicts with distributed actor synthesized stored property}}
+  // expected-note@-2 {{candidate exactly matches}}
+}


nice catch, thank you

ktoso · 2025-11-03T01:35:53Z

test/Distributed/rdar162800185.swift

+import Distributed
+
+func foo<T: DistributedActor>(_: T.Type) where T.ActorSystem == LocalTestingDistributedActorSystem {}
+func bar() { foo(A.self) }


Hmm, this doesn't quite reproduce rdar://162800185;
This works fine on existing main already, so we're missing something here

Now it does, we chatted offline :-)

ktoso · 2025-11-03T01:57:38Z

lib/Sema/CodeSynthesisDistributedActor.cpp

+  // but the id synthesis will force itself to be FIRST anyway, so it works out.
+  nominal->addMember(propDecl, /*hint=*/idProperty, insertAtHead);
+  nominal->addMember(pbDecl, /*hint=*/idProperty, insertAtHead);
+  return propDecl;


Thanks, this refactor to requests is neat, and we keep the ordering guarantee nicely this way too

[Sema] A couple of minor follow-ups to #85245

hamishknight force-pushed the panic-at-the-distro branch 3 times, most recently from a2e17a7 to 8dea46d Compare November 1, 2025 10:49

hamishknight changed the title ~~[WIP] A DistributedActor fix and cleanup~~ [Sema] Fix and cleanup distributed id/actorSystem synthesis Nov 2, 2025

hamishknight force-pushed the panic-at-the-distro branch 3 times, most recently from 09b4f75 to 996f65a Compare November 2, 2025 14:39

hamishknight marked this pull request as ready for review November 2, 2025 14:40

hamishknight requested review from AnthonyLatsis, hborla, ktoso, slavapestov and xedin as code owners November 2, 2025 14:40

hamishknight force-pushed the panic-at-the-distro branch from 996f65a to 02a1d6b Compare November 2, 2025 15:32

hamishknight commented Nov 2, 2025

View reviewed changes

lib/Sema/CodeSynthesis.cpp Outdated Show resolved Hide resolved

hamishknight force-pushed the panic-at-the-distro branch from 02a1d6b to 6a905fc Compare November 2, 2025 18:48

hamishknight added 12 commits November 3, 2025 10:54

[AST] Introduce isSpecialDistributedProperty

acb4622

And use this in `evaluateMembersRequest`.

[Sema] Remove unnecessary logic in `GetDistributedActorSystemProperty…

72f4ca1

…Request` This is checking a conformance for something defined in the Distributed module itself, so things would be very broken if it failed.

[AST] NFC: Reorder headers in DistributedDecl.cpp

f0df6bd

[AST] Avoid returning null Type from `getAssociatedTypeOfDistributedS…

613d47d

…ystemOfActor` Make sure we produce an error for a broken stdlib and produce ErrorType instead.

[Sema] NFC: Inline id and actorSystem synthesis logic

48b6472

These can be inlined into their respective requests.

[Sema] Consolidate distributed actor property synthesis logic

b79b559

Have the witness derivation logic call into the synthesis requests.

[Sema] Always synthesize id and actorSystem properties

b996e0f

Only fallback to name lookup for serialized modules and swift interfaces. This means we can then implement the user-declared `id`/`actorSystem` error as part of redeclaration checking.

[Sema] Lazily compute type of id and actorSystem properties

66aaf4d

Delay their interface type computation until they are actually type-checked. This ensures their synthesis logic doesn't produce cycles.

[Sema] Remove Identifiable derivation for distributed actors

fc70a7a

This shouldn't be necessary anymore since we we'll no longer try to infer `ID` from Identifiable's default implmentation of `id`.

[Sema] Remove ImplicitMemberAction::ResolveDistributedActor

5aa0a74

The member names that triggered this don't even match the requirements in the Distributed module anymore, and `installDistributedActorIfNecessary` was only called for structs which can't be distributed actors. Rip it out.

hamishknight force-pushed the panic-at-the-distro branch from 6a905fc to 5aa0a74 Compare November 3, 2025 10:54

hamishknight requested review from jckarter and kavon as code owners November 3, 2025 13:21

hamishknight force-pushed the panic-at-the-distro branch 2 times, most recently from d5eb1b2 to 3bc7e5b Compare November 3, 2025 13:27

[AST] Replace a couple more checks for vars named id/actorSystem

965e85f

Use `isSpecialDistributedProperty` or the synthesis requests instead. While here, also make `isSpecialDistributedProperty` agnostic to interface and serialized module cases.

hamishknight force-pushed the panic-at-the-distro branch from 3bc7e5b to 965e85f Compare November 3, 2025 13:31

xedin approved these changes Nov 3, 2025

View reviewed changes

lib/Sema/CodeSynthesisDistributedActor.cpp Show resolved Hide resolved

ktoso approved these changes Nov 4, 2025

View reviewed changes

ktoso merged commit 0f58eb2 into swiftlang:main Nov 4, 2025
5 of 7 checks passed

hamishknight deleted the panic-at-the-distro branch November 4, 2025 08:45

hamishknight mentioned this pull request Nov 4, 2025

[Sema] A couple of minor follow-ups to #85245 #85309

Merged

kavon added a commit that referenced this pull request Nov 4, 2025

Merge pull request #85309 from hamishknight/follow-up

9c63a56

[Sema] A couple of minor follow-ups to #85245

	enum class SpecialDistributedProperty {
	enum class SpecialDistributedActorProperty {

[Sema] Fix and cleanup distributed id/actorSystem synthesis #85245

[Sema] Fix and cleanup distributed id/actorSystem synthesis #85245

Conversation

hamishknight commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xedin commented Nov 2, 2025

Uh oh!

Uh oh!

ktoso commented Nov 3, 2025

Uh oh!

hamishknight commented Nov 3, 2025

Uh oh!

hamishknight commented Nov 3, 2025

Uh oh!

hamishknight commented Nov 3, 2025

Uh oh!

xedin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ktoso left a comment

Choose a reason for hiding this comment

Uh oh!

ktoso Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

ktoso Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

hamishknight Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

ktoso Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

ktoso Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

ktoso Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

ktoso Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[Sema] Fix and cleanup distributed `id`/`actorSystem` synthesis #85245

[Sema] Fix and cleanup distributed `id`/`actorSystem` synthesis #85245

hamishknight commented Oct 31, 2025 •

edited

Loading