Skip to content

[Casting] Make more casts look inside __SwiftValue #68952

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

Merged

Conversation

tbkka
Copy link
Contributor

@tbkka tbkka commented Oct 3, 2023

The following sequence of casts would previously succeed

struct S {}
let s = S() as AnyObject
s as? NSObject // Should fail

The final cast here should fail, since S clearly is not a subclass of NSObject. But it would previously succeed because the as AnyObject would package the struct into an ObjC-compatible __SwiftValue class. This latter is an NSObject subclass.

This bug was fixed in the main swift_dynamicCast runtime function some time ago, but not in the swift_dynamicCastObjCClass which is chosen by IRGen to optimize casts to ObjC class types. This PR changes the latter to test for __SwiftValue and fall back to the former in that case in order to get the correct handling. Falling back to swift_dynamicCast also ensures that the contents of the __SwiftValue are correctly unwrapped/bridged/etc as necessary to fully support Swift casting semantics.

Resolves: rdar://111422725

TODO: I've left an XFAILed test here about the behavior of type(of:) with __SwiftValue boxes.

The following sequence of casts would previously succeed
```
struct S {}
let s = S() as AnyObject
s as? NSObject
```
The final cast here should fail, since `S` clearly is not a
subclass of NSObject.  But it would previously succeed because
the `as AnyObject` would package the struct into an ObjC-compatible
`__SwiftValue` class.  This latter is an NSObject subclass.

This bug was fixed in the main `swift_dynamicCast` runtime function
some time ago, but not in the `swift_dynamicCastObjCClass` which
is chosen by IRGen to optimize casts to ObjC class types.
This PR changes the latter to test for `__SwiftValue` and fall
back to the former in that case in order to get the correct
handling.  Falling back to `swift_dynamicCast` also ensures that
the contents of the `__SwiftValue` are correctly unwrapped/bridged/etc
as necessary to fully support Swift casting semantics.

Resolves: rdar://111422725

TODO: I've left an XFAILed test here about the behavior of `type(of:)`
with `__SwiftValue` boxes.
@tbkka tbkka requested a review from mikeash October 3, 2023 21:58
@tbkka
Copy link
Contributor Author

tbkka commented Oct 3, 2023

@swift-ci Please test

Copy link
Contributor

@mikeash mikeash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Hope we don't have to come back and add a bincompat mode for it.

@tbkka
Copy link
Contributor Author

tbkka commented Oct 4, 2023

@swift-ci Please test

1 similar comment
@tbkka
Copy link
Contributor Author

tbkka commented Oct 24, 2023

@swift-ci Please test

@tbkka tbkka merged commit 3bdbbb5 into swiftlang:main Oct 24, 2023
tbkka added a commit to tbkka/swift that referenced this pull request Jan 18, 2024
…alue

Turns out that our Obj-C bridging relies on this inconsistent behavior,
so it can quite likely never be actually fixed.
tbkka added a commit that referenced this pull request Feb 2, 2024
Revert #68952 [Casting] Make more casts look inside __SwiftValue
@tbkka tbkka deleted the tbkka-rdar111422725-more-SwiftValue-casting branch August 1, 2024 16:38
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