-
Notifications
You must be signed in to change notification settings - Fork 140
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
Unwrap optionals with dynamic cast #3580
Unwrap optionals with dynamic cast #3580
Conversation
Preserve optionals in anystruct, move value unboxing earlier in casting function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Nice to see the fix is kept simple 👌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
+1 on what @SupunS said, it's great that the implementation of this improvement is so simple.
Initially I was surprised this worked, I had assumed we would need to unbox only to the requested "level" of nesting. However, after looking at the rest of the implementation, this works because we box up the value again to the requested level again below (value = interpreter.ConvertAndBox(locationRange, value, valueSemaType, expectedType
) 👍
Related: Swift's dynamic casting is actually way more complicated then I thought https://github.com/swiftlang/swift/blob/main/docs/DynamicCasting.md 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
Closes #3183
Description
Dynamic casting now unwraps optionals following the behaviour of Swift.
master
branchFiles changed
in the Github PR explorer