Skip to content

[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

Conversation

nate-chandler
Copy link
Contributor

@nate-chandler nate-chandler commented Jul 26, 2021

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.

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
@nate-chandler nate-chandler requested a review from a team as a code owner July 26, 2021 18:37
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please nominate

@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please clean test windows platform

@nate-chandler nate-chandler force-pushed the cherrypick/rdar79383990/release/5.5 branch from bcab190 to 49d2269 Compare August 6, 2021 20:35
@swift-ci
Copy link
Contributor

swift-ci commented Aug 6, 2021

Build failed
Swift Test OS X Platform
Git Sha - bcab190c55959a338c16851e7e1c422fac334ed8

@nate-chandler nate-chandler force-pushed the cherrypick/rdar79383990/release/5.5 branch from 49d2269 to 32a6cd2 Compare August 6, 2021 21:42
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Aug 6, 2021

Build failed
Swift Test OS X Platform
Git Sha - 32a6cd248bad552691b4233ef50eaf4963591323

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
@nate-chandler nate-chandler force-pushed the cherrypick/rdar79383990/release/5.5 branch from 32a6cd2 to 70dca91 Compare August 7, 2021 00:43
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please clean test windows platform

@nate-chandler nate-chandler merged commit 108cc64 into swiftlang:release/5.5 Aug 10, 2021
@nate-chandler nate-chandler deleted the cherrypick/rdar79383990/release/5.5 branch August 10, 2021 16:54
@AnthonyLatsis AnthonyLatsis added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.5 labels Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 5.5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants