[ObjC Bridging] Consistently bridge block types verbatim #77496
Merged
Conversation
This file contains 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
A `@convention(block)` closure in Swift is completely compatible with Objective-C and does not need to be wrapped in a `__SwiftValue` box for use. Previously, it was bridged verbatim when appearing by itself, but could end up boxed when it went through array bridging. The test verifies that: * Objective-C does not see a `__SwiftValue` box * Swift `type(of:)` does not see a `__SwiftValue` box * Objective-C can actually call the closure Resolves rdar://138132321
tbkka
commented
Nov 8, 2024
| // Dig through existential types. | ||
| if (auto srcExistentialTy = dyn_cast<ExistentialTypeMetadata>(srcType)) { | ||
| else if (auto srcExistentialTy = dyn_cast<ExistentialTypeMetadata>(srcType)) { |
There was a problem hiding this comment.
This function has always been a long if-else chain; I've reformatted it some to make that a bit more explicit.
I've also adjusted the comment layout to hopefully make it clearer:
// Explain first if
if (...) {
...
}
// Explain second if
else if (....) {
...
}
// Explain third if
else if (...) {
...
}
| // Bridge the source value to an NSError. | ||
| auto flags = consume ? DynamicCastFlags::TakeOnSuccess | ||
| : DynamicCastFlags::Default; | ||
| return dynamicCastValueToNSError(src, srcType, srcErrorWitness, flags); | ||
| } | ||
| // Handle functions: "Block" types can be bridged literally | ||
| else if (auto fn = dyn_cast<FunctionTypeMetadata>(srcType)) { |
There was a problem hiding this comment.
This is the new case: If this is a function type with a block convention, then bridge it verbatim.
| CastsTests.test("block closures in array should bridge without __SwiftValue") | ||
| { | ||
| let f: @convention(block) () -> Void = {} | ||
| let x = ([f] as NSArray)[0] |
There was a problem hiding this comment.
This test does NSArray bridging explicitly here, but the bug also manifests when a Swift array is implicitly bridged to NSArray when calling Obj-C code.
|
|
||
| #import "Cast_Blocks.h" | ||
|
|
||
| BOOL ObjCThinksObjectIsSwiftValue(id obj) { |
There was a problem hiding this comment.
It's important to verify that Objective-C really does see what we expect.
|
@swift-ci Please test |
mikeash
approved these changes
Nov 8, 2024
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.
A
@convention(block)closure in Swift is completely compatible with Objective-C and does not need to be wrapped in a__SwiftValuebox for use.Previously, it was bridged verbatim when appearing by itself, but could end up boxed when it went through array bridging.
The test verifies that:
__SwiftValueboxtype(of:)does not see a__SwiftValueboxResolves rdar://138132321