Skip to content

[SILOptimizer] Prevent devirtualization of call to distributed witnes… #80373

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 1 commit into from
Mar 31, 2025

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Mar 28, 2025

…s requirements

This is a narrow fix, we are going to work on fixing this properly and allowing both devirtualization and specialization for distributed requirement witnesses.

Anything that uses an ad-hoc serialization requirement scheme cannot be devirtualized because that would result in loss of ad-hoc conformance in new substitution map.

Resolves: #79318
Resolves: rdar://146101172

@xedin xedin requested review from ktoso and eeckstein as code owners March 28, 2025 18:03
@xedin
Copy link
Contributor Author

xedin commented Mar 28, 2025

@swift-ci please test

@xedin
Copy link
Contributor Author

xedin commented Mar 28, 2025

@swift-ci please test macOS platform

@xedin xedin force-pushed the rdar-146101172-narrow branch from 0415b40 to c99f72b Compare March 28, 2025 23:37
@xedin
Copy link
Contributor Author

xedin commented Mar 28, 2025

@swift-ci please test

@eeckstein
Copy link
Contributor

@swift-ci benchmark

…s requirements

This is a narrow fix, we are going to work on fixing this properly
and allowing both devirtualization and specialization for distributed
requirement witnesses.

Anything that uses an ad-hoc serialization requirement scheme cannot
be devirtualized because that would result in loss of ad-hoc conformance
in new substitution map.

Resolves: swiftlang#79318
Resolves: rdar://146101172
@xedin xedin force-pushed the rdar-146101172-narrow branch from c99f72b to 585b687 Compare March 30, 2025 02:28
@xedin
Copy link
Contributor Author

xedin commented Mar 30, 2025

@swift-ci benchmark

@xedin
Copy link
Contributor Author

xedin commented Mar 30, 2025

@swift-ci please test

// REQUIRES: concurrency
// REQUIRES: distributed

// REQUIRES: OS=macosx || OS=ios
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ktoso FYI, I restricted this test to macOS and iOS only for now because it fails with availability issues on watch os:

/Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/swift/test/Distributed/distributed_actor_adhoc_requirements_optimized_build.swift:56:17: error: 'RemoteCallArgument' is only available in watchOS 9.0 or newer
 50 | 
 51 | // NOT final on purpose
 52 | public class FakeInvocationEncoder : DistributedTargetInvocationEncoder {
    |              `- note: add @available attribute to enclosing class
 53 |   public typealias SerializationRequirement = Transferable
 54 | 
 55 |   public func recordArgument<Value: SerializationRequirement>(
    |               `- note: add @available attribute to enclosing instance method
 56 |     _ argument: RemoteCallArgument<Value>) throws {
    |                 `- error: 'RemoteCallArgument' is only available in watchOS 9.0 or newer
 57 |   }
 58 | 

/Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/swift/test/Distributed/distributed_actor_adhoc_requirements_optimized_build.swift:83:31: error: 'DistributedActor' is only available in watchOS 9.0 or newer
 71 | 
 72 | // NOT final on purpose
 73 | public class NotFinalActorSystemForAdHocRequirementTest: DistributedActorSystem, @unchecked Sendable {
    |              `- note: add @available attribute to enclosing class
 74 |   public typealias ActorID = String
 75 |   public typealias InvocationEncoder = FakeInvocationEncoder
    :
 80 |   public init() {}
 81 | 
 82 |   public func resolve<Act>(id: ActorID, as actorType: Act.Type)
    |               `- note: add @available attribute to enclosing instance method
 83 |     throws -> Act? where Act: DistributedActor {
    |                               `- error: 'DistributedActor' is only available in watchOS 9.0 or newer
 84 |     fatalError()
 85 |   }

@xedin
Copy link
Contributor Author

xedin commented Mar 30, 2025

@swift-ci please test macOS platform

1 similar comment
@xedin
Copy link
Contributor Author

xedin commented Mar 30, 2025

@swift-ci please test macOS platform

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assertion failure swift::ProtocolConformanceRef::subst with 6.1 in release build
3 participants