[SDK] Get the preferred @encode string for MapKit types dynamically #19691
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
|
@swift-ci Please test |
mikeash
approved these changes
Oct 3, 2018
|
Build failed |
On older OSs, CLLocationCoordinate2D was an anonymous struct with a
typedef, which meant its Objective-C type encoding string was "{?=dd}"
instead of "{CLLocationCoordinate2D=dd}". This incompatibility led to
us rejecting casts from NSValues created with CLLocationCoordinate2Ds
in Objective-C on older OSs because they didn't match the type
encoding provided. Instead, we can try to create an NSValue the way
the frameworks do, and see what /its/ type encoding is. This is what
SceneKit already does.
There's an extra wrinkle here because the convenience methods for
encoding CLLocationCoordinate2Ds are actually added in MapKit rather
than CoreLocation. That means that if someone's app just uses
CoreLocation, we can't rely on those convenience methods to get the
correct type encoding. In this case, the best thing we can do is just
give up and use the static encoding.
This whole thing is indicative of a bigger problem, namely that
NSValue normally doesn't try to validate types at all. However, Swift
bridge casting really does want to distinguish, say, a
CLLocationCoordinate2D from a CGPoint, and checking the NSValue encode
string was a way to do that. But this shows that it's still not safe
to assume that's consistent against everything in a process even if
they're all using @encode on the real struct (or the Swift
equivalent), because the different parts of the process may have been
compiled against different SDKs.
This change does not attempt to solve that problem.
Finishes rdar://problem/44866579
jrose-apple
force-pushed
the
a-stray-question-mark-leads-us-astray
branch
from
0f4b699 to
e813138
Compare
|
@swift-ci Please test |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
modelorganism
pushed a commit
to modelorganism/swift
that referenced
this pull request
Oct 11, 2018
…wiftlang#19691) On older OSs, CLLocationCoordinate2D was an anonymous struct with a typedef, which meant its Objective-C type encoding string was "{?=dd}" instead of "{CLLocationCoordinate2D=dd}". This incompatibility led to us rejecting casts from NSValues created with CLLocationCoordinate2Ds in Objective-C on older OSs because they didn't match the type encoding provided. Instead, we can try to create an NSValue the way the frameworks do, and see what /its/ type encoding is. This is what SceneKit already does. There's an extra wrinkle here because the convenience methods for encoding CLLocationCoordinate2Ds are actually added in MapKit rather than CoreLocation. That means that if someone's app just uses CoreLocation, we can't rely on those convenience methods to get the correct type encoding. In this case, the best thing we can do is just give up and use the static encoding. This whole thing is indicative of a bigger problem, namely that NSValue normally doesn't try to validate types at all. However, Swift bridge casting really does want to distinguish, say, a CLLocationCoordinate2D from a CGPoint, and checking the NSValue encode string was a way to do that. But this shows that it's still not safe to assume that's consistent against everything in a process even if they're all using @encode on the real struct (or the Swift equivalent), because the different parts of the process may have been compiled against different SDKs. This change does not attempt to solve that problem. Finishes rdar://problem/44866579
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.
On older OSs, CLLocationCoordinate2D was an anonymous struct with a typedef, which meant its Objective-C type encoding string was
"{?=dd}"instead of"{CLLocationCoordinate2D=dd}". This incompatibility led to us rejecting casts from NSValues created with CLLocationCoordinate2Ds in Objective-C on older OSs because they didn't match the type encoding provided. Instead, we can try to create an NSValue the way the frameworks do, and see what its type encoding is. This is what SceneKit already does.There's an extra wrinkle here because the convenience methods for encoding CLLocationCoordinate2Ds are actually added in MapKit rather than CoreLocation. That means that if someone's app just uses CoreLocation, we can't rely on those convenience methods to get the correct type encoding. In this case, the best thing we can do is just give up and use the static encoding.
This whole thing is indicative of a bigger problem, namely that NSValue normally doesn't try to validate types at all. However, Swift bridge casting really does want to distinguish, say, a CLLocationCoordinate2D from a CGPoint, and checking the NSValue encode string was a way to do that. But this shows that it's still not safe to assume that's consistent against everything in a process even if they're all using
@encodeon the real struct (or the Swift equivalent), because the different parts of the process may have been compiled against different SDKs.This change does not attempt to solve that problem.
Finishes rdar://problem/44866579. Follow-up to #19634.