Skip to content

[Distributed] Reproducer for generics and library evolution mode #80588

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 2 commits into from
Apr 21, 2025

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented Apr 7, 2025

This corrects how we were dealing with dispatch thunks -- mostly be
removing a lot of special casing we did but doesn't seem necessary and
instead we correct and emit all the necessary information int TBD.

This builds on #74935 by further refining how we fixed that issue, and adds more regression tests. It also removes a load of special casing of distributed thunks in library evolution mode, which is great.

Resolves and adds regression test for for rdar://145292018

This is also a more proper fix to the previously resolved but in a not-great-way which caused other issues:

  • refs rdar://128284016
  • refs rdar://128310903

@ktoso ktoso marked this pull request as draft April 7, 2025 12:04
@ktoso ktoso added the distributed Feature → concurrency: distributed actor label Apr 17, 2025
@ktoso ktoso force-pushed the wip-regression-test-for-b-da-bad-access branch 2 times, most recently from e1ca212 to 4fcbfbe Compare April 18, 2025 02:19
@ktoso ktoso marked this pull request as ready for review April 18, 2025 02:19
@ktoso ktoso requested review from jckarter and rjmccall as code owners April 18, 2025 02:19
…ust work

This corrects how we were dealing with dispatch thunks -- mostly be
removing a lot of special casing we did but doesn't seem necessary and
instead we correct and emit all the necessary information int TBD.

This builds on  swiftlang#74935 by further refining how we fixed that issue, and adds more regression tests. It also removes a load of special casing of distributed thunks in library evolution mode, which is great.

Resolves and adds regression test for for rdar://145292018

This is also a more proper fix to the previously resolved but in a not-great-way which caused other issues:
- resolves rdar://128284016
- resolves rdar://128310903
@ktoso ktoso force-pushed the wip-regression-test-for-b-da-bad-access branch from 4fcbfbe to 957a95b Compare April 18, 2025 02:21
@ktoso
Copy link
Contributor Author

ktoso commented Apr 18, 2025

Figured out a solution and even fixed other TBD issues along the way, and past TBD issues in a cleaner way. Will want to verify this in a project as well to make sure we don't break anyone with this but it looks solid -- way less special casing!

@ktoso ktoso requested a review from xedin April 18, 2025 02:23
@ktoso
Copy link
Contributor Author

ktoso commented Apr 18, 2025

@swift-ci please smoke test

@ktoso
Copy link
Contributor Author

ktoso commented Apr 18, 2025

@swift-ci please test

@ktoso
Copy link
Contributor Author

ktoso commented Apr 18, 2025

@swift-ci please build toolchain macOS

@xedin
Copy link
Contributor

xedin commented Apr 18, 2025

@swift-ci please test source compatibility

Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

LGTM! I left a few comments about the test inline but nothing blocking.

@ktoso
Copy link
Contributor Author

ktoso commented Apr 21, 2025

Hmm, build issue on simulator run, let me restrict this to macos and linux

:0: warning: using sysroot for 'iPhoneSimulator' but targeting 'MacOSX'
:0: warning: using sysroot for 'iPhoneSimulator' but targeting 'MacOSX'
ld: building for 'macOS', but linking in dylib (/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator.sdk/usr/lib/libobjc.A.tbd) built for 'iOS-simulator'
:0: error: link command failed with exit code 1 (use -v to see invocation)

@ktoso
Copy link
Contributor Author

ktoso commented Apr 21, 2025

@swift-ci please test

@ktoso ktoso merged commit d788713 into swiftlang:main Apr 21, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
distributed Feature → concurrency: distributed actor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants