Skip to content

[5.5] [SIL] Fixed return index for call-as-async (BOOL, Error, Value) case. #38863

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 Aug 12, 2021

5.5 Summary: Fix compiler crash during call-as-async of ObjC method whose completion handler looks like (BOOL, NSError *, Value).


Explanation: When converting an ObjC method type which is being called as async to a Swift function type, some of the values passed to the ObjC method's completion handler are converted to return values of the Swift function. The flag and error parameters, however, if present, are ignored.

When abstracting the result type for the Swift method, the formal type of the corresponding parameter in the ObjC method's completion handler is used. Digging out that parameter entails indexing into the parameters of the completion handler. Previously, the indexing logic relied on the error appearing before the flag if both appeared before the value of interest. Here, the indexing is tweaked to check both special indices for each possible index until the first that matches neither is found.

Issue: rdar://81625544

Original PR: #38825

Testing: New regression tests, Swift CI

Reviewed by: Joe Groff

Risk: Low.

Scope: Limited to Swift import of ObjC methods as async functions.

@nate-chandler nate-chandler requested a review from a team as a code owner August 12, 2021 21:18
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please nominate

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 009c874972578b3a1e38fe29c4c977de86fcf442

@nate-chandler nate-chandler force-pushed the cherrypick/rdar81625544/release/5.5 branch from 009c874 to 8898fcd Compare August 13, 2021 00:18
@nate-chandler
Copy link
Contributor Author

Needed to be based on top of #38862 .

@nate-chandler
Copy link
Contributor Author

@swift-ci please test

Previously, the function emitCBridgedToNativeValue handled three
situations around optionals:
- Bridged?, Native?
- Bridged, Native?
- Bridged, Native

Here, handling for the fourth case
- Bridged?, Native
is added.

To enable this, the number of Optional wrappings that the bridged type
has that the native type does not is passed in to the function.  Then,
in the portions of the function where actual transformations are done,
the values are unwrapped an appropriate number of times.  Mostly that
means force unwrapping N times before doing the transformation.  In the
case of types that conform to _ObjectiveCBridgeable, however, it means
force unwrapping the value N-1 times after doing the transformation
because _ObjectiveCBridgeable._unconditionallyBridgeFromObjectiveC
performs one layer of unwrapping itself.

rdar://81590807
The underlying problem was fixed by the change for rdar://81590807 .
Add tests for the specific case that was originally reported.

rdar://80704382
When converting an ObjC method type which is being called as async to a
Swift function type, some of the values passed to the ObjC method's
completion handler are converted to return values of the Swift function.
The flag and error parameters, however, if present, are ignored.

When abstracting the result type for the Swift method, the formal type
of the corresponding parameter in the ObjC method's completion handler
is used.  Digging out that parameter entails indexing into the
parameters of the completion handler.  Previously, the indexing logic
relied on the error appearing before the flag if both appeared before
the value of interest.  Here, the indexing is tweaked to check both
special indices for each possible index until the first that matches
neither is found.

rdar://81625544
@nate-chandler nate-chandler force-pushed the cherrypick/rdar81625544/release/5.5 branch from 8898fcd to 57fd46d Compare August 17, 2021 00:38
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler nate-chandler merged commit 095f046 into swiftlang:release/5.5 Aug 17, 2021
@AnthonyLatsis AnthonyLatsis added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.5 labels Jan 8, 2023
@nate-chandler nate-chandler deleted the cherrypick/rdar81625544/release/5.5 branch July 5, 2023 23:30
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