Skip to content

Commit 5dbbd02

Browse files
krodakkrodak
committed
BridgeJS: Support optional @jsclass as exported function parameters
Optional jsObject was moved to the stack ABI in #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 #751
1 parent d1e0f95 commit 5dbbd02

11 files changed

Lines changed: 223 additions & 2 deletions

File tree

‎Plugins/BridgeJS/Sources/BridgeJSCore/ExportSwift.swift‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1492,6 +1492,11 @@ extension BridgeType {
14921492
switch self {
14931493
case .swiftStruct, .array, .dictionary, .associatedValueEnum:
14941494
return true
1495+
case .nullable(.jsObject, _):
1496+
// Optional jsObject (incl. @JSClass) parameters travel via the bridge
1497+
// stack (isSome + object id), unlike their non-optional counterparts
1498+
// which are passed as a direct i32 parameter.
1499+
return true
14951500
case .nullable(let wrapped, _):
14961501
return wrapped.isStackUsingParameter
14971502
default:
@@ -1531,6 +1536,12 @@ extension BridgeType {
15311536
case .unsafePointer: return .unsafePointer
15321537
case .swiftProtocol: return .jsObject
15331538
case .void: return .void
1539+
case .nullable(.jsObject, _):
1540+
// Optional jsObject (incl. @JSClass) uses the stack ABI: the presence
1541+
// flag and object id are transferred through the bridge stack rather
1542+
// than as direct ABI parameters. The lift expression is the no-argument
1543+
// `Optional<T>.bridgeJSLiftParameter()` that pops them off the stack.
1544+
return LiftingIntrinsicInfo(parameters: [])
15341545
case .nullable(let wrappedType, _):
15351546
let wrappedInfo = try wrappedType.liftParameterInfo()
15361547
if wrappedInfo.parameters.isEmpty {

‎Plugins/BridgeJS/Tests/BridgeJSToolTests/Inputs/MacroSwift/Optionals.swift‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ class OptionalPropertyHolder {
3030

3131
@JS func testOptionalPropertyRoundtrip(_ holder: OptionalPropertyHolder?) -> OptionalPropertyHolder?
3232

33+
// Exported functions taking/returning an optional jsObject use the stack ABI.
34+
@JS func roundTripExportedOptionalJSObject(value: JSObject?) -> JSObject?
35+
36+
// Exported function taking/returning an optional imported @JSClass (issue #751).
37+
@JS func roundTripExportedOptionalJSClass(value: WithOptionalJSClass?) -> WithOptionalJSClass?
38+
3339
@JS
3440
func roundTripString(name: String?) -> String? {
3541
return name

‎Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/Optionals.json‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,76 @@
239239
}
240240
}
241241
},
242+
{
243+
"abiName" : "bjs_roundTripExportedOptionalJSObject",
244+
"effects" : {
245+
"isAsync" : false,
246+
"isStatic" : false,
247+
"isThrows" : false
248+
},
249+
"name" : "roundTripExportedOptionalJSObject",
250+
"parameters" : [
251+
{
252+
"label" : "value",
253+
"name" : "value",
254+
"type" : {
255+
"nullable" : {
256+
"_0" : {
257+
"jsObject" : {
258+
259+
}
260+
},
261+
"_1" : "null"
262+
}
263+
}
264+
}
265+
],
266+
"returnType" : {
267+
"nullable" : {
268+
"_0" : {
269+
"jsObject" : {
270+
271+
}
272+
},
273+
"_1" : "null"
274+
}
275+
}
276+
},
277+
{
278+
"abiName" : "bjs_roundTripExportedOptionalJSClass",
279+
"effects" : {
280+
"isAsync" : false,
281+
"isStatic" : false,
282+
"isThrows" : false
283+
},
284+
"name" : "roundTripExportedOptionalJSClass",
285+
"parameters" : [
286+
{
287+
"label" : "value",
288+
"name" : "value",
289+
"type" : {
290+
"nullable" : {
291+
"_0" : {
292+
"jsObject" : {
293+
"_0" : "WithOptionalJSClass"
294+
}
295+
},
296+
"_1" : "null"
297+
}
298+
}
299+
}
300+
],
301+
"returnType" : {
302+
"nullable" : {
303+
"_0" : {
304+
"jsObject" : {
305+
"_0" : "WithOptionalJSClass"
306+
}
307+
},
308+
"_1" : "null"
309+
}
310+
}
311+
},
242312
{
243313
"abiName" : "bjs_roundTripString",
244314
"effects" : {

‎Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/Optionals.swift‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,28 @@ public func _bjs_testOptionalPropertyRoundtrip(_ holderIsSome: Int32, _ holderVa
2020
#endif
2121
}
2222

23+
@_expose(wasm, "bjs_roundTripExportedOptionalJSObject")
24+
@_cdecl("bjs_roundTripExportedOptionalJSObject")
25+
public func _bjs_roundTripExportedOptionalJSObject() -> Void {
26+
#if arch(wasm32)
27+
let ret = roundTripExportedOptionalJSObject(value: Optional<JSObject>.bridgeJSLiftParameter())
28+
return ret.bridgeJSLowerReturn()
29+
#else
30+
fatalError("Only available on WebAssembly")
31+
#endif
32+
}
33+
34+
@_expose(wasm, "bjs_roundTripExportedOptionalJSClass")
35+
@_cdecl("bjs_roundTripExportedOptionalJSClass")
36+
public func _bjs_roundTripExportedOptionalJSClass() -> Void {
37+
#if arch(wasm32)
38+
let ret = roundTripExportedOptionalJSClass(value: Optional<WithOptionalJSClass>.bridgeJSLiftParameter())
39+
return ret.bridgeJSLowerReturn()
40+
#else
41+
fatalError("Only available on WebAssembly")
42+
#endif
43+
}
44+
2345
@_expose(wasm, "bjs_roundTripString")
2446
@_cdecl("bjs_roundTripString")
2547
public func _bjs_roundTripString(_ nameIsSome: Int32, _ nameBytes: Int32, _ nameLength: Int32) -> Void {

‎Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/Optionals.d.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ export type Exports = {
5050
}
5151
roundTripOptionalClass(value: Greeter | null): Greeter | null;
5252
testOptionalPropertyRoundtrip(holder: OptionalPropertyHolder | null): OptionalPropertyHolder | null;
53+
roundTripExportedOptionalJSObject(value: any | null): any | null;
54+
roundTripExportedOptionalJSClass(value: WithOptionalJSClass | null): WithOptionalJSClass | null;
5355
roundTripString(name: string | null): string | null;
5456
roundTripInt(value: number | null): number | null;
5557
roundTripInt8(value: number | null): number | null;

‎Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/Optionals.js‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -725,6 +725,46 @@ export async function createInstantiator(options, swift) {
725725
const optResult = pointer === null ? null : OptionalPropertyHolder.__construct(pointer);
726726
return optResult;
727727
},
728+
roundTripExportedOptionalJSObject: function bjs_roundTripExportedOptionalJSObject(value) {
729+
const isSome = value != null;
730+
if (isSome) {
731+
const objId = swift.memory.retain(value);
732+
i32Stack.push(objId);
733+
}
734+
i32Stack.push(+isSome);
735+
instance.exports.bjs_roundTripExportedOptionalJSObject();
736+
const isSome1 = i32Stack.pop();
737+
let optResult;
738+
if (isSome1) {
739+
const objId1 = i32Stack.pop();
740+
const obj = swift.memory.getObject(objId1);
741+
swift.memory.release(objId1);
742+
optResult = obj;
743+
} else {
744+
optResult = null;
745+
}
746+
return optResult;
747+
},
748+
roundTripExportedOptionalJSClass: function bjs_roundTripExportedOptionalJSClass(value) {
749+
const isSome = value != null;
750+
if (isSome) {
751+
const objId = swift.memory.retain(value);
752+
i32Stack.push(objId);
753+
}
754+
i32Stack.push(+isSome);
755+
instance.exports.bjs_roundTripExportedOptionalJSClass();
756+
const isSome1 = i32Stack.pop();
757+
let optResult;
758+
if (isSome1) {
759+
const objId1 = i32Stack.pop();
760+
const obj = swift.memory.getObject(objId1);
761+
swift.memory.release(objId1);
762+
optResult = obj;
763+
} else {
764+
optResult = null;
765+
}
766+
return optResult;
767+
},
728768
roundTripString: function bjs_roundTripString(name) {
729769
const isSome = name != null;
730770
let result, result1;

‎Sources/JavaScriptKit/BridgeJSIntrinsics.swift‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1826,6 +1826,19 @@ extension _BridgedAsOptional where Wrapped == JSObject {
18261826
}
18271827
}
18281828

1829+
extension _BridgedAsOptional where Wrapped: _JSBridgedClass {
1830+
// `@JSClass` wrappers (`_JSBridgedClass`) bridge an underlying `JSObject`, so an
1831+
// optional wrapper uses the same stack ABI as `Optional<JSObject>`: the presence
1832+
// flag and retained object id are transferred through the bridge stack.
1833+
//
1834+
// Lifting (`bridgeJSLiftParameter()` / `bridgeJSLiftReturn()`) and stack push/pop
1835+
// are already provided by the generic `Wrapped: _BridgedSwiftStackType` extension;
1836+
// only the export return path needs a dedicated lowering here.
1837+
@_spi(BridgeJS) public consuming func bridgeJSLowerReturn() -> Void {
1838+
Wrapped.bridgeJSStackPushAsOptional(asOptional)
1839+
}
1840+
}
1841+
18291842
extension _BridgedAsOptional where Wrapped: _BridgedSwiftProtocolWrapper {
18301843
@_spi(BridgeJS) public static func bridgeJSLiftParameter(_ isSome: Int32, _ objectId: Int32) -> Self {
18311844
Self(

‎Tests/BridgeJSRuntimeTests/ExportAPITests.swift‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@ func runJsWorks() -> Void
7575
return try Foo(value)
7676
}
7777

78+
@JS func roundTripOptionalImportedClass(v: Foo?) -> Foo? {
79+
return v
80+
}
81+
7882
struct TestError: Error {
7983
let message: String
8084
}

‎Tests/BridgeJSRuntimeTests/Generated/BridgeJS.swift‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5262,9 +5262,9 @@ public func _bjs_OptionalSupportExports_static_roundTripOptionalAPIOptionalResul
52625262

52635263
@_expose(wasm, "bjs_OptionalSupportExports_static_takeOptionalJSObject")
52645264
@_cdecl("bjs_OptionalSupportExports_static_takeOptionalJSObject")
5265-
public func _bjs_OptionalSupportExports_static_takeOptionalJSObject(_ valueIsSome: Int32, _ valueValue: Int32) -> Void {
5265+
public func _bjs_OptionalSupportExports_static_takeOptionalJSObject() -> Void {
52665266
#if arch(wasm32)
5267-
OptionalSupportExports.takeOptionalJSObject(_: Optional<JSObject>.bridgeJSLiftParameter(valueIsSome, valueValue))
5267+
OptionalSupportExports.takeOptionalJSObject(_: Optional<JSObject>.bridgeJSLiftParameter())
52685268
#else
52695269
fatalError("Only available on WebAssembly")
52705270
#endif
@@ -6940,6 +6940,17 @@ public func _bjs_makeImportedFoo(_ valueBytes: Int32, _ valueLength: Int32) -> I
69406940
#endif
69416941
}
69426942

6943+
@_expose(wasm, "bjs_roundTripOptionalImportedClass")
6944+
@_cdecl("bjs_roundTripOptionalImportedClass")
6945+
public func _bjs_roundTripOptionalImportedClass() -> Void {
6946+
#if arch(wasm32)
6947+
let ret = roundTripOptionalImportedClass(v: Optional<Foo>.bridgeJSLiftParameter())
6948+
return ret.bridgeJSLowerReturn()
6949+
#else
6950+
fatalError("Only available on WebAssembly")
6951+
#endif
6952+
}
6953+
69436954
@_expose(wasm, "bjs_throwsSwiftError")
69446955
@_cdecl("bjs_throwsSwiftError")
69456956
public func _bjs_throwsSwiftError(_ shouldThrow: Int32) -> Void {

‎Tests/BridgeJSRuntimeTests/Generated/JavaScript/BridgeJS.json‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11917,6 +11917,41 @@
1191711917
}
1191811918
}
1191911919
},
11920+
{
11921+
"abiName" : "bjs_roundTripOptionalImportedClass",
11922+
"effects" : {
11923+
"isAsync" : false,
11924+
"isStatic" : false,
11925+
"isThrows" : false
11926+
},
11927+
"name" : "roundTripOptionalImportedClass",
11928+
"parameters" : [
11929+
{
11930+
"label" : "v",
11931+
"name" : "v",
11932+
"type" : {
11933+
"nullable" : {
11934+
"_0" : {
11935+
"jsObject" : {
11936+
"_0" : "Foo"
11937+
}
11938+
},
11939+
"_1" : "null"
11940+
}
11941+
}
11942+
}
11943+
],
11944+
"returnType" : {
11945+
"nullable" : {
11946+
"_0" : {
11947+
"jsObject" : {
11948+
"_0" : "Foo"
11949+
}
11950+
},
11951+
"_1" : "null"
11952+
}
11953+
}
11954+
},
1192011955
{
1192111956
"abiName" : "bjs_throwsSwiftError",
1192211957
"effects" : {

0 commit comments

Comments
 (0)