Some More Additional ManagedValue APIs #6836
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…verted to objects do they have @owned ownership. This commit cleans up a few places in bridging code, where we were treating metatypes as if they had non-invalid cleanups. rdar://29791263
…ng/forwarding a cleanup for a ManagedValue. In SILGen, we often times want to transform a ManagedValue, while maintaining the underlying cleanup. The ManagedValue::transform API attempts to formalize this pattern through the usage of std::move to create a movable rvalue, but returning a different ManagedValue as a full value. This ensure that the original ManagedValue is destroyed at end of statement, will allowing the underlying cleanup to be forwarded. From an ownership perspective, I think the majority of these cases should actually recreate the underlying cleanup since in most cases they involve forwarding. For today though I am using this to just formalize this pattern at the relevant call sites. rdar://29791263
…Value. This is the first step towards eliminating: * ManagedValue::forUnmanaged(SILValue) * ManagedValue::ManagedValue(SILValue, CleanupHandle). The reason why these two methods must be eliminated is that: 1. Currently non-trivial values are "borrowed" using ManagedValue::forUnmanaged. With Semantic SIL, we are now able to represent borrows via begin_borrow and end_borrow. This will make the semantics that SILGen is trying to describe more accurate. 2. ManagedValue::ManagedValue can be used to add arbitrary cleanups. We want methods that add cleanups, but that at the same time allow the caller to specify what it expects the ownership of the given SILValue to be. This would then cause an assertion to trip giving greater clarity. rdar://29791263
|
@swift-ci Please smoke test and merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains two additional groups of new APIs for ManagedValue that allow for ownership invariants to be enforced via asserts.
ManagedValue transform(SILValue) &&;
The first patch contains a new method called transform. The key thing to notice is that it can only be called on RValue references, yet it returns a new ManagedValue. The reason for this is that transform is meant to "forward" a cleanup/transform and then destroy the original ManagedValue. This transformation happens in a bunch of places in SILGen. In most of these cases, the original cleanup is passed forward. I believe that in a SemanticARC world this will no longer be correct since in most cases where one is performing forwarding, one wants the destroy_value to occur on the newly forwarded value. In a future patch I may put the code into transform that enables the old cleanup to be destroyed and the new cleanup to be placed on the "transformed" ManagedValue.
Specific static constructor with asserts
The next patch adds new static constructors that create ManagedValues with specific semantics. There are asserts in each one of these to ensure that the relevant semantics are being followed. An example of this is the class method,
This first verifies that the given SILValue exists, that that
v.getOwnershipKind()is Owned, before finally creating the ManagedValue.rdar://29791263