Skip to content

[5.9] Arrange for closure bodies promoted by AllocBoxToStack to have their originals removed by MoveOnlyChecker. #67220

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

Conversation

jckarter
Copy link
Contributor

@jckarter jckarter commented Jul 11, 2023

Issue: rdar://110675352
• Explanation: Removes a source of potential compiler crashes when noncopyable types are captured in nonescaping closures. This is an improvement on the approach taken in #67035 that avoids breaking SIL pass manager invariants and possibly leading to other compiler crashes.
• Scope of Issue: Crash-on-valid.
• Origination: Noncopyable types feature work.
• Risk: Low. The patch deletes SIL that is unreachable from the rest of the program so should have no runtime effect.
• Reviewed By: @atrick, @eeckstein
• Automated Testing: Swift CI
• Dependencies: None
• Builder Impact: Not applicable
• Directions for QE: None

jckarter added 2 commits July 11, 2023 08:43
…originals removed by MoveOnlyChecker.

This is an improvement of swiftlang#67031 which avoids deleting the closure function
body during AllocBoxToStack, which still breaks pass invariants by modifying
functions other than the currently-analyzed function. As a function pass,
AllocBoxToStack also doesn't really know with certainty whether the original
closure function is unused after stack promotion or not. We still want to
eliminate the original when it may contain invalid SIL for move-only values
that rely on the escape analysis for correct semantics, so rather than mark the
original function to be *ignored* during move-only checking, mark it to be
*deleted* by move-only checking if the function is in fact unused at that
point.

If the marked function is still used, we let it pass through move-only
checking normally, which may cause redundant diagnostics but is the right
thing to do since code is still potentially using the closure with escaping
semantics. We should rearrange things to make this situation impossible in
the future.

rdar://110675352
@jckarter jckarter requested a review from a team as a code owner July 11, 2023 15:45
@jckarter
Copy link
Contributor Author

@swift-ci Please test

@jckarter jckarter merged commit 869c5bf into swiftlang:release/5.9 Jul 11, 2023
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.

2 participants