Skip to content

[AddressLowering] Omit dealloc_stack in dead ends. #62453

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

Conversation

nate-chandler
Copy link
Contributor

@nate-chandler nate-chandler commented Dec 8, 2022

It is valid for owned values not to be destroyed on paths terminating in unreachable. Similarly, it is valid for storage not to be deallocated on paths terminating in unreachable. However, it is not valid for storage to be deallocated before being deinitialized, even in paths terminating in unreachable. Consequently, when AddressLowering, dealloc_stacks must not be created in dead-end blocks: because the input OSSA may not destroy an owned value, the output may not deinitialize owned storage; so a dealloc_stack in an unreachable block could dealloc storage which was not deinitialized.

It is valid for owned values not to be destroyed on paths terminating in
unreachable.  Similarly, it is valid for storage not to be deallocated
on paths terminating in unreachable.  However, it is _not_ valid for
storage to be deallocated before being deinitialized, even in paths
terminating in unreachable.  Consequently, when AddressLowering,
dealloc_stacks must not be created in dead-end blocks: because the input
OSSA may not destroy an owned value, the output may not deinitialize
owned storage; so a dealloc_stack in an unreachable block could dealloc
storage which was not deinitialized.
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@atrick atrick left a comment

Choose a reason for hiding this comment

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

Well, this is frustrating. Another unfortunate workaround until I can enable complete OSSA lifetimes

@nate-chandler nate-chandler marked this pull request as ready for review December 8, 2022 18:36
@nate-chandler nate-chandler merged commit b782278 into swiftlang:main Dec 8, 2022
@nate-chandler nate-chandler deleted the opaque-values/2/20221207 branch December 8, 2022 18:36
@meg-gupta
Copy link
Contributor

LGTM

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.

3 participants