Compatibility for Optional -> AnyHashable casts #35650
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
As part of making casting more consistent, the behavior of Optional -> AnyHashable casts
was changed in some cases. This PR provides a hook for re-enabling the old behavior
in certain contexts.
Background: Most of the time, casts from String? to AnyHashable get optimized
to just injects the String? into the AnyHashable, so the following
has long been true and remains true in Swift 5.4:
```
let s = "abc"
let o: String? = s
// Next test is true because s is promoted to Optional<String>
print(s == o)
// Next test is false: Optional<String> and String are different types
print(s as AnyHashable == o as AnyHashable)
```
But when casts ended up going through the runtime, Swift 5.3 behaved
differently, as you could see by casting a dictionary with `String?` keys (in
the generic array code, key and value casts always use the runtime logic). In
the following code, both print statements behave differently in Swift 5.4 than
before:
```
let a: [String?:String] = ["Foo":"Bar"]
let b = a as [AnyHashable:Any]
print(b["Foo"] == "Bar") // Works before Swift 5.4
print(b["Foo" as String?] == "Bar") // Works in Swift 5.4 and later
```
Old behavior: The `String?` keys would get unwrapped to `String` before being injected into AnyHashable. This allows the first to work but strangely breaks the second.
New behavior: The `String?` keys do not get unwrapped. This breaks the first but makes the second work.
TODO: The long-term goal, of course, is for `AnyHashable("Foo" as String?)` to
test equal to `AnyHashable("Foo")` (and hash the same, of course). In that
case, all of the tests above will succeed.
Resolves rdar://73301155
|
@swift-ci Please test |
|
Build failed |
|
@swift-ci Please test |
mikeash
approved these changes
Jan 29, 2021
|
Build failed |
|
Build failed |
|
@swift-ci Please test |
lorentey
approved these changes
Jan 29, 2021
|
@swift-ci Please test Windows |
This restores the earlier behavior of Optionals cast to AnyHashable, so that [String?:Any] dictionaries cast to [AnyHashable:Any] can be indexed by plain String keys. This is a little problematic because it's not consistent with the compiler-optimized casts. But the ability to index such dictionaries by plain String keys seems important to preserve. SR-9047 will expand Optional/AnyHashable interoperability so that such indexing works without this special case. An earlier proposal would link-check this, changing the behavior depending on the caller. But it's not really workable to change the behavior seen by intermediate frameworks depending on the app they're being called by.
|
After thinking on it through the weekend, I think Karoy's right. I've changed it to unconditionally preserve the previous behavior. (And updated the tests to match.) |
|
@swift-ci Please test and merge |
|
@swift-ci Please test Windows platform |
tbkka
pushed a commit
to tbkka/swift
that referenced
this pull request
Feb 1, 2021
(This is a cherry-pick of PR swiftlang#35650 to the release/5.4 branch.) The recent overhaul of the runtime dynamic casting made this particular cast consistent with the compiler optimizer and with how other similar casts behave. In particular, an `Optional` is cast to `AnyHashable` by simple injection, whereas previously the `Optional` was unwrapped (if it was non-nil) and the contents were injected. The previous behavior allowed code like the following to work: ``` let a: [String?:String] = ["Foo":"Bar"] let b = a as [AnyHashable:Any] print(b["Foo"] == "Bar") // Used to work ``` This is broken by the new cast behavior because `AnyHashable("Foo")` is not considered equal to `AnyHashable("Foo" as String?)`. Once SR-9047 is implemented, these two will in fact be considered equal. At that time, we should go back to a simple injection to make this cast behave identically to the compiler-optimized version of this case. Resolves rdar://73301155
|
Build failed |
|
Build failed |
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.
As part of making casting more consistent, the behavior of Optional -> AnyHashable casts
was changed in some cases. This PR restores the old behavior.
Background: Most of the time, casts from String? to AnyHashable get optimized
to just injects the String? into the AnyHashable, so the following
has long been true and remains true in Swift 5.4:
But when casts ended up going through the runtime, Swift 5.3 behaved
differently, as you could see by casting a dictionary with
String?keys (inthe generic array code, key and value casts always use the runtime logic). In
the following code, both print statements behave differently in current builds than
before:
Swift 5.3 and before: The
String?keys would get unwrapped toStringbefore being injected into AnyHashable. This allows the first to work but strangely breaks the second.Behavior after casting overhaul: The
String?keys do not get unwrapped. This breaks the first but makes the second work.This change restores the behavior for this specific case to the Swift 5.3 behavior.
TODO: The long-term goal, of course, is for
AnyHashable("Foo" as String?)to test equal toAnyHashable("Foo")(and hash the same, of course). See SR-9047 for details. Once that's done, we won't need to unwrap optionals in this case and can revert this change so the runtime behavior will precisely match the behavior of compiler-optimized casts.Resolves rdar://73301155