-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
Disable stack promotion for classes with isolated deinit #76995
base: main
Are you sure you want to change the base?
Disable stack promotion for classes with isolated deinit #76995
Conversation
@swift-ci please smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah, great observation, fix sounds good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Thinking about this again, a better fix would be to make this check in
Though, it's a bit more complicated because it also needs to handle structs/tuples/enums which contain a class reference. This can be done by adding a flag in |
@eeckstein @nickolas-pohilets shall we merge this to resolve the crashes and investigate the follow up alternative approach you’re suggesting, or hold off merging and do the alternative approach instead immediately — wdyt @eeckstein? |
I'm fine with merging this and doing it the right way in a follow-up. But it would be nice to first file a github issue and reference it in a to-do comment in the stack promotion |
This provides a proper fix of the issue, reverting workaround implemented in #76960