Skip to content

fix SILGen bug for cross-module actor let accesses #40097

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
Nov 9, 2021

Conversation

kavon
Copy link
Member

@kavon kavon commented Nov 8, 2021

When trying to access a let-bound property of an actor across
a moudule boundary, the operation should be treated as async.
But, upon reaching SILGen, the compiler would crash when trying
to emit the needed hop, because the base of the element reference
was missing. This commit fixes that and adds regression coverage.

resolves rdar://81812013

@kavon kavon added the concurrency Feature: umbrella label for concurrency language features label Nov 8, 2021
@kavon kavon self-assigned this Nov 8, 2021
When trying to access a let-bound property of an actor across
a moudule boundary, the operation should be treated as async.
But, upon reaching SILGen, the compiler would crash when trying
to emit the needed hop, because the base of the element reference
was missing. This commit fixes that and adds regression coverage.

resolves rdar://81812013
@kavon kavon force-pushed the cross-module-actor-let-silgen branch from 4227108 to 9bff2b4 Compare November 8, 2021 22:32
@kavon
Copy link
Member Author

kavon commented Nov 8, 2021

@swift-ci please smoke test and merge

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

LGTM. This code would crash the compiler before, and the value passed in is otherwise not queried, so this looks Very Safe.

@kavon
Copy link
Member Author

kavon commented Nov 9, 2021

I wanted to add a bit more regression test coverage in the test by adding a case where the access happens in an optional chain.

@kavon
Copy link
Member Author

kavon commented Nov 9, 2021

@swift-ci please smoke test and merge

@swift-ci swift-ci merged commit 7b7443e into swiftlang:main Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
concurrency Feature: umbrella label for concurrency language features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants