Skip to content

6.0: [NoncopyablePartialConsumption] Promote to feature. #73159

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

nate-chandler
Copy link
Contributor

@nate-chandler nate-chandler commented Apr 20, 2024

Explanation: Enable noncopyable partial consumption by default.

Enabling the feature exposed some bugs that needed to be fixed along the way:

  • Handling for enum deinits in the address checker.
  • Handling for deiniting enums in one place in the optimizer.
  • Handling for drop_deinits in one place in optimizer.
  • Not using illegal SILLocation when generating code to destroy fields of a noncopyable value stored in an addres.
  • Recognize destroys of on_stack partial applies don't consume guaranteed arguments their context captures.
  • Completing lifetimes in functions that use move-only address values.
    Scope: Affects noncopyable code.
    Issue: rdar://126715654
    Original PR: [NoncopyablePartialConsumption] Promote to feature. #73121
    Risk: Medium.
    Testing: Updated unit tests.
    Reviewer: @jckarter, @kavon

Remove all checks for the feature flag.  It's on all the time.
A destroy_value of an on-stack partial apply isn't actually a consuming
use, so don't treat it as one.
Previously, the enum representation was fixed to represent the different
cases payloads separately with the unchecked_take_enum_data_addr
instruction consuming all fields but that whose address is obtained.
In a few places, handling for enum deinits was left undone with an
assertion that the enum not have one.

Here, the deinit bit of the enum is shifted to the end.  And the
assertions are replaced with handling.  Finally, the logic for inserting
destroys after switch_enum_addr instructions is fixed.
It's not allowed to replace ued(enum(x)) with x if the enum in question
has a deinit.
It relies on complete lifetimes for its analysis.  So if there are any
instructions to check, complete all lifetimes in the function first.
@nate-chandler nate-chandler requested a review from a team as a code owner April 20, 2024 00:57
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please test macos platform

@nate-chandler nate-chandler requested a review from tbkka April 22, 2024 14:01
@nate-chandler nate-chandler merged commit 41308c1 into swiftlang:release/6.0 Apr 22, 2024
@nate-chandler nate-chandler deleted the cherrypick/release/6.0/rdar126715654 branch April 22, 2024 17:45
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