[SR-5289] Teach Mirror how to handle unowned/unmanaged references #28823
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
Test that we can reflect weak, unowned, and unmanaged references to both Swift and Obj-C types (including existential references to such types) that occur in both Swift class objects and in Swift structs. This includes the specific reported case (unowned reference to an Obj-C object).
TypeReferenceOwnership here is a bitmap indicating which of the various ownership modifiers (weak, unowned, unmanaged) were present on the field declaration. Making this a bitmap ensures that we can accurately represent the data that may have been provided but we have to be careful when we interpret the result. This changes the `isWeak`, `isUnowned`, `isUnmanged` checks to only return true if exactly one bit is set. Combinations of bits will not be accepted. Also, add `isStrong` to check for the default ownership with no modifiers.
This involves refactoring how we handle reference ownership when reflecting fields of struct and class objects: * Move FieldType into ReflectionMirror.mm FieldType is really just an internal implementation detail of this one source file, so it does not belong in an ABI header. * Use TypeReferenceOwnership directly to track field ownership This avoids bitwise copying of properties and localizes some of the knowledge about reference ownership * Generate a top-level "copyFieldContents" from ReferenceStorage.def Adding new ownership types to ReferenceStorage.def will now automatically produce calls to `copy*FieldContents` - failure to provide a suitable implementation will fail the build. * Implement `copyUnownedFieldContents` and `copyUnmanagedFieldContents`
…alIn` Balanced calls here help ensure we use consistent memory management.
…n occasional failure
I had to fix this in order to test the Mirror support changes. Weak properties are optionals, and reflection packages things in Any containers, so it's hard to test reflecting weak properties without being able to cast the result back to the original type.
In particular, a null type at this point indicates that the cast has already been checked and is known to be valid.
If this works, I should be able to factor out the common unwrap logic here.
This reverts commit 0e86b31. Trying to fix the casting inconsistencies is turning out to be a lot more complex than I thought. Let's roll back to just the Mirror fix so that can be merged before I take off on a lengthy tangent.
This reverts commit 0e86b31.
… references (swiftlang#28368)"" This reverts commit ca693ee.
|
@swift-ci Please test macOS |
|
@swift-ci Please test Linux |
shahmishal
approved these changes
Dec 17, 2019
|
Thanks! |
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 is the same as #28368 (which was reverted in #28821) except that it disables the new tests when testing against old versions of the standard library. (The functionality here is known to be broken on old standard libraries, so there's no point testing there.
Resolves SR-5289.