-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[5.5] [SILGen] Used formal type when bridging completion handler arguments. #38633
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
nate-chandler
merged 3 commits into
swiftlang:release/5.5
from
nate-chandler:cherrypick/rdar79383990/release/5.5
Aug 10, 2021
Merged
[5.5] [SILGen] Used formal type when bridging completion handler arguments. #38633
nate-chandler
merged 3 commits into
swiftlang:release/5.5
from
nate-chandler:cherrypick/rdar79383990/release/5.5
Aug 10, 2021
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
Address a FIXME where lowered types rather than formal types were used when converting from objc to native types which resulted in a failure to convert block types.
Previously, AbstractionPattern::getOpaque() was used for async continuations. That was problematic for functions like ```objc - (void)performVoid2VoidWithCompletion:(void (^ _Nonnull)(void (^ _Nonnull)(void)))completion; ``` whose completion takes a closure. Doing so resulted in attempting to build a block to func thunk where one of the functions had an out parameter. Instead, use the AbstractionPattern(ty). rdar://79383990
@swift-ci please test |
@swift-ci please nominate |
@swift-ci please test |
@swift-ci please clean test windows platform |
bcab190
to
49d2269
Compare
Build failed |
49d2269
to
32a6cd2
Compare
@swift-ci please test |
Build failed |
In the synthesized completion handler that is passed to ObjC methods that are called as async, the formal type of each argument is the corresponding parameter of the formal type of the block. The non-error, non-index arguments need to be prepared so that they can be used to fulfill the continuation; the lambda which does that preparation for each such argument takes the formal type of that argument. As such, the call to that lambda needs to pass the type of the corresponding parameter of the formal type of the block to that lambda. Doing so entails skipping over the error and flag parameters if they appear before some of the non-error, non-index arguments. Previously, no parameters were skipped over. Consequently, when an error or flag argument preceded one of the non-error, non-index arguments, the wrong formal type was passed to the preparation lambda. Here, that is fixed by passing the correct index. The to-be-used indices for the formal block parameters are the same as the to-be-used indices for the lowered block parameters minus one reflecting the fact that the lowered block always has an initial block_storage parameter as the first argument which the formal type never has. rdar://81617749
32a6cd2
to
70dca91
Compare
@swift-ci please test |
@swift-ci please clean test windows platform |
DougGregor
approved these changes
Aug 10, 2021
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.
Explanation: When calling ObjC methods from Swift as async methods, a completion handler to be passed to the ObjC method is synthesized by SILGen. Being called from ObjC, that completion handler receives ObjC types such as ObjCBool and NSString. On the other hand, the Swift code that is calling-as-async that ObjC method expects native types like Bool and String. Consequently, part of the completion handler's implementation is to bridge those bridged values to native values.
The utility code for doing this bridging requires the formal type be passed in. Previously, that foreign type was being retrieved from the lowered type. For many things (e.g. ObjCBool and NSString), that worked fine. However, for block types, it did not; the "formal" type that can be obtained from a SILFunctionType is still a SILFunctionType. And the utility bridging code requires AnyFunctionTypes in order to work.
Here, the formal original native method type is passed down, bridged back to a formal ObjC type, and passed through to the bridging utility code. Additionally, the abstraction pattern used for unsafe continuations is changed to a type reference from opaque, allowing block arguments not to have bound generic arguments or require an extra thunk.
Issue: rdar://79383990
Original PR: #38370 , #38785
Testing: New regression tests, Swift CI
Reviewed by: @jckarter
Risk: Low.
Scope: Limited to Swift import of ObjC methods as async functions.