[5.7🍒] Fix crash when emitting force-unwrap-and-call expression for optional ObjC methods #60401
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 |
|
@swift-ci please test |
…c method Due to some changes in how the AST is constructed by CSApply to handle global actors from @preconcurrency declarations, the AST for a force-call to an optional ObjC method that is part of a global-actor qualified protocol, such as: ``` import Foundation @mainactor @objc protocol P { @objc optional func generateMaybe() async } extension P { func test() async -> Void { await self.generateMaybe!() } } ``` now contains a function conversion in between the forced-value operation and the dynamic-ref: ``` (force_value_expr type='@mainactor () async -> ()' (optional_evaluation_expr implicit type='(@mainactor () async -> ())?' (inject_into_optional implicit type='(@mainactor () async -> ())?' (function_conversion_expr implicit type='@mainactor () async -> ()' <<<<< new! (bind_optional_expr implicit type='() async -> ()' (dynamic_member_ref_expr type='(() async -> ())?' ... ))))) ``` For compatibility with how ObjC does message sends to these optional methods, in Swift there is a difference between how something like `a.method!()` and the following are compiled: ``` let meth = a.method! meth() ``` The former will directly call the optional method and let the "unknown selector" error be emitted if it's not implemented. But the latter will query the dynamic method and inject that result into an `Optional<...>` to then be forced, yielding a "force-unwrap nil" error. It's a subtle difference but those two scenarios lead to different code paths in SILGen. Now, this patch fixes the issue by enhancing the recognition of these direct call scenarios so that it can look past these function conversions. Because that recognition already tries to ignore implicit conversions, this is not something new. The problem was that we weren't considering implicit conversions between the optional-evaluation and bind-optional expressions. This patch is intentionally precise about what kind of function conversions can be skipped, because I do not know if all of them are OK to blindly skip or not. We might need to expand that in the future. Resolves rdar://97646309
kavon
force-pushed
the
5.7-fix-97646309
branch
from
02d8acc to
726775f
Compare
|
@swift-ci please test |
DougGregor
approved these changes
Aug 11, 2022
AnthonyLatsis
added
🍒 release cherry pick
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.
Cherry-pick of #60399
Resolves rdar://97646309