Skip to content

Fix arg cleanup for objc continuations #38556

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
Jul 23, 2021

Conversation

meg-gupta
Copy link
Contributor

@meg-gupta meg-gupta commented Jul 22, 2021

Current SILGen for objc continuations, can end the lifetime of args
before await_async_continuation.
This PR fixes it so that the cleanups of args for such applies can be
delayed until result plan emission which generates await_async_continuation.
Also inserts fix_lifetime cleanup so that optimizer cannot move lifetime ending operations before await_async_continuation

Fixes rdar://78982371

@meg-gupta
Copy link
Contributor Author

@swift-ci test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 171ca1acaeda1a25477929da0c406cc035c0c1cf

@meg-gupta meg-gupta force-pushed the fixobjcasyncsilgen branch from 171ca1a to 3b739e2 Compare July 22, 2021 16:15
@meg-gupta
Copy link
Contributor Author

@swift-ci test

@meg-gupta meg-gupta marked this pull request as ready for review July 22, 2021 16:17
@meg-gupta meg-gupta requested review from jckarter and rjmccall July 22, 2021 16:18
@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 3b739e2e700ffc4588f3d378494def43fa015da1

@meg-gupta meg-gupta requested a review from gottesmm July 22, 2021 19:16
@meg-gupta meg-gupta force-pushed the fixobjcasyncsilgen branch 3 times, most recently from 39aff13 to 7224888 Compare July 22, 2021 21:52
@meg-gupta
Copy link
Contributor Author

@swift-ci test

@meg-gupta meg-gupta force-pushed the fixobjcasyncsilgen branch from 7224888 to 518e9da Compare July 22, 2021 22:16
Current SILGen for objc continuations, can end the lifetime of args
before await_async_continuation.
This PR fixes it by extending the lifetime of the args until await_async_continuation
by creating copies. And then inserts manual cleanup with fix_lifetime + destroy

Fixes rdar://78982371
@meg-gupta meg-gupta force-pushed the fixobjcasyncsilgen branch from 518e9da to 3372ab7 Compare July 22, 2021 22:24
@meg-gupta
Copy link
Contributor Author

@swift-ci test

Copy link
Contributor

@gottesmm gottesmm left a comment

Choose a reason for hiding this comment

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

LGTM in terms of the SILGen. @jckarter can talk semantically if we always want to do this in this context. But this should be safe and not move scopes around.

@meg-gupta
Copy link
Contributor Author

@jckarter does this version look okay ?

@meg-gupta meg-gupta merged commit a2bd36a into swiftlang:main Jul 23, 2021
@jckarter
Copy link
Contributor

LGTM, thanks @meg-gupta !

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.

4 participants