Embedded: fix arguments mismatch in ResultTypeInfo
#80862
Conversation
Mismatched number of arguments traps on Wasm. Second argument of `destroy` in VW seems unused, so let's pass `nullptr` as a workaround.
MaxDesiatov
added
runtime
MaxDesiatov
requested review from
kubamracek,
al45tair,
kateinoigakukun and
eeckstein
| @@ -259,7 +259,7 @@ struct ResultTypeInfo { | |||
| storeEnumTagSinglePayload(v, whichCase, emptyCases); | |||
| } | |||
| void vw_destroy(OpaqueValue *v) { | |||
| destroy(v); | |||
| destroy(v, nullptr); | |||
There was a problem hiding this comment.
Suggestions for how to test this with lit would be appreciated. I reckon we need some mock SerialExecutor implementation and Task invocation in such test? I've reproduced it with JavaScriptKit's JavaScriptEventLoop: SerialExecutor in the first place with this code.
There was a problem hiding this comment.
Ideally I would love to see runtime tests covering this. Can we / do we have runtime tests running wasm?
There was a problem hiding this comment.
@MaxDesiatov I think this issue could be reproduced with default cooperative executor.
There was a problem hiding this comment.
We have an existing non-embedded test suite for WASI. The best course of action would be enable that for embedded where possible, WDYT?
|
@swift-ci smoke test |
|
This looks about right, but I think it would be good to also fix the other witness functions, which have the same problem; and also it looks to me that even the non-embedded-swift version of this code has the same problem? |
@kubamracek In non-embedded path, the call receiver, |
Can you link the code in question? This, https://github.com/swiftlang/swift/pull/80862/files#diff-a08b11d44e9648d1e8a3f8b0d21488f95f88f7b48e13ff29219d663e25633ee3R235 says the opposite. |
|
@kubamracek I mean, the call is just a forwarding call generated here, and the underlying destroy function is called with args + this. |
|
Linux build failed with |
|
@swift-ci Please smoke test Linux platform |
Mismatched number of arguments between a call (including indirect calls) and declarations traps on Wasm.
destroyis declared with two arguments, but was called with one argument in embedded concurrency runtime. Second argument ofdestroyin VW seems unused, so let's passnullptras a workaround.