[SILGen] Fix a crash when a closure is converted to a pointer to a function returning a non-trivial C++ type #73561
Conversation
|
@swift-ci please test |
|
@swift-ci please test |
|
@swift-ci please test macOS |
lib/SILGen/SILGenBridging.cpp
Outdated
| assert((SGF.F.getRepresentation() == | ||
| SILFunctionType::Representation::CFunctionPointer || | ||
| objcFnTy->getNumIndirectFormalResults() == 0) && | ||
| "Objective-C methods cannot have indirect results"); |
There was a problem hiding this comment.
This assertion is just wrong now that we're importing types that are formally treated as indirect in SIL. The test case here is just an ObjC++ method that returns a non-trivial C++ type, or even an ObjC method that returns a struct that contains a __weak reference (although we may not support that properly). We should just remove this assertion. Please check whether that test case passes without any other changes.
There was a problem hiding this comment.
I tried compiling the following code, where S is a non-trivial C++ type, but the compiler rejects it:
@objc
class C0 : NSObject {
// error: method cannot be marked @objc because its result type cannot be represented in Objective-C
// note: non-trivial C++ classes cannot be represented in Objective-C
@objc
func m1() -> S {
return S()
}
}
The diagnostic was added in #69343.
Is there another way to trigger the assertion?
There was a problem hiding this comment.
That seems quite wrong. I mean, we can have this restriction if we need to subset things, but it is absolutely not the case that non-trivial C++ classes cannot be represented in Objective-C. We can import methods like this from Objective-C++ headers even if we restrict them in Swift for some reason.
There was a problem hiding this comment.
At any rate, please write the code assuming that this is required to work, even if there's currently a restriction about it.
There was a problem hiding this comment.
Yes, I removed the assertion altogether.
lib/SILGen/SILGenBridging.cpp
Outdated
| for (auto result : substConv.getResults()) { | ||
| if (!substConv.isSILIndirect(result)) { | ||
| continue; | ||
| if (F.getRepresentation() == |
There was a problem hiding this comment.
This condition should not be looking at function type representations and should instead be looking at, I guess, whether there's a direct/indirect mismatch in the result.
lib/SILGen/SILGenBridging.cpp
Outdated
| if (substConv.hasIndirectSILResults()) { | ||
| assert(substTy->getNumResults() == 1); | ||
| result = args[0]; | ||
| if (F.getRepresentation() != SILFunctionType::Representation::CFunctionPointer) { |
There was a problem hiding this comment.
Same thing here as above. This should be based intelligently on looking at the differences between the result types and whether they're returned directly, not the function type representation.
function returning a non-trivial C++ type rdar://124501345
Remove assertion that is no longer true.
ahatanaka
force-pushed
the
closure-to-fp-indirect-return
branch
from
01add11 to
9ee583e
Compare
|
@swift-ci please test |
…nction returning a non-trivial C++ type (swiftlang#73561) When emitting a native-to-foreign thunk, pass the thunk's result address parameter to the native function if both the thunk and the native function return their results indirectly and the thunk is not for an async function. Also, remove an outdated assertion. rdar://124501345
…nction returning a non-trivial C++ type (#73561) When emitting a native-to-foreign thunk, pass the thunk's result address parameter to the native function if both the thunk and the native function return their results indirectly and the thunk is not for an async function. Also, remove an outdated assertion. rdar://124501345 (cherry picked from commit 1b0df4e)
When emitting a native-to-foreign thunk, pass the thunk's result address parameter to the native function if both the thunk and the native function return their results indirectly and the thunk is not for an async function.
Also, remove an outdated assertion.
rdar://124501345