BridgeJS: Support optional @JSClass as exported function parameters#754
Open
krodak wants to merge 1 commit into
Open
BridgeJS: Support optional @JSClass as exported function parameters#754krodak wants to merge 1 commit into
krodak wants to merge 1 commit into
Conversation
Optional jsObject was moved to the stack ABI in swiftwasm#738, but the export parameter direction in ExportSwift was never updated and still emitted direct ABI params with `Optional<T>.bridgeJSLiftParameter(isSome, value)`. For `@JSClass` types no such overload exists, so generated thunks failed to compile; for plain `JSObject?` it compiled but mismatched the JS stack-push lowering. Make `.nullable(.jsObject)` export parameters use the stack ABI (no direct ABI params, no-argument `Optional<T>.bridgeJSLiftParameter()`), and add the missing `bridgeJSLowerReturn()` for `Optional<_JSBridgedClass>` so optional `@JSClass` values round-trip as both parameters and return values. Fixes swiftwasm#751
This file contains hidden or 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
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.
Overview
Fixes #751. BridgeJS code generation failed to compile when a
@JSexported function took (or returned) an optional@JSClassparameter such asJSAbortSignal?, producingno exact matches in call to static method 'bridgeJSLiftParameter'forOptional<JSAbortSignal>.Root cause
#738 moved optional
jsObjectto the stack ABI and updated the JS glue, the ImportTS side, and the runtime lowering. However the export parameter direction inExportSwiftwas never updated:liftParameterInfo()for.nullable(.jsObject)still emitted direct ABI params and the lift callOptional<T>.bridgeJSLiftParameter(isSome, value).@JSClasstypes there is no such two-argument overload, so generated wasm thunks failed to compile.JSObject?it compiled, but silently mismatched the JS side, which now pushes the presence flag and object id onto the bridge stack. This went unnoticed because the only adjacent test had an empty body and asserteddoesNotThrowwithout reading the value back.Changes
1.
ExportSwift: optional jsObject export params use the stack ABI..nullable(.jsObject)(including@JSClass) now reportsisStackUsingParameterand produces no direct ABI parameters, so the thunk lifts via the no-argumentOptional<T>.bridgeJSLiftParameter()that pops the presence flag and object id off the bridge stack, matching the JS lowering.2. Runtime: add
Optional<_JSBridgedClass>.bridgeJSLowerReturn().@JSClasswrappers already conform to_BridgedSwiftStackType, so lifting and stack push/pop were already available via the generic extension; only the export return path needed a dedicated lowering.Before:
After:
Tests
@JSClassdirectly as an exported function parameter and return value, asserting the round-tripped value for both the present andnullcases (the previous coverage only checked import-direction members via text snapshots, which are never compiled forwasm32).JSObject?and optional@JSClass?.