Skip to content

Commit 52ec56c

Browse files
committed
feat: preserve identity across all return shapes
Emit a per-class extension override of _BridgedSwiftHeapObject's bridgeJSLowerReturn that routes through the identity cache, symmetric to the existing bridgeJSStackPush override. Without it, Optional<SwiftIdentityClass> returns bypassed the per-class table: _BridgedAsOptional.bridgeJSLowerReturn calls value.bridgeJSLowerReturn() on the inner heap object, and that call previously hit the default implementation (passRetained + bare pointer) instead of our per-method thunk. With this override, scalar, array-element, and Optional returns all route through the same identity handshake. The existing Optional identity test is upgraded from observability-only to strict === . Side effect: the per-method `case .swiftHeapObject where isSwiftIdentityMode` branch in ExportedThunkBuilder.lowerReturnValue is now redundant — ret.bridgeJSLowerReturn() picks up the extension override automatically. The builder's isSwiftIdentityMode predicate is removed; codegen is simpler. Also drops the ENABLE_TEST_INTROSPECTION compile-flag gate on the Task 5 identity-table-size helpers. The generated BridgeJS.swift always declares the Wasm exports for `@JS func` declarations, so a conditional function definition broke the Generated file on toolchains that didn't pick up the target's swiftSettings define. The helpers are trivial and cost nothing to always-emit in test targets that don't ship. 165/165 tests green.
1 parent 37fbaf0 commit 52ec56c

17 files changed

Lines changed: 205 additions & 426 deletions

File tree

Benchmarks/Sources/Generated/BridgeJS.swift

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1822,13 +1822,7 @@ nonisolated(unsafe) var _SimpleClassSwiftIdentity_identityTable: Set<UnsafeMutab
18221822
public func _bjs_SimpleClassSwiftIdentity_init(_ nameBytes: Int32, _ nameLength: Int32, _ count: Int32, _ flag: Int32, _ rate: Float32, _ precise: Float64) -> UnsafeMutableRawPointer {
18231823
#if arch(wasm32)
18241824
let ret = SimpleClassSwiftIdentity(name: String.bridgeJSLiftParameter(nameBytes, nameLength), count: Int.bridgeJSLiftParameter(count), flag: Bool.bridgeJSLiftParameter(flag), rate: Float.bridgeJSLiftParameter(rate), precise: Double.bridgeJSLiftParameter(precise))
1825-
return withExtendedLifetime(ret) {
1826-
let ptr = Unmanaged.passUnretained(ret).toOpaque()
1827-
if _SimpleClassSwiftIdentity_identityTable.insert(ptr).inserted {
1828-
_ = Unmanaged.passRetained(ret)
1829-
}
1830-
return ptr
1831-
}
1825+
return ret.bridgeJSLowerReturn()
18321826
#else
18331827
fatalError("Only available on WebAssembly")
18341828
#endif
@@ -1961,6 +1955,16 @@ public func _bjs_SimpleClassSwiftIdentity_release_wrapper(_ pointer: UnsafeMutab
19611955
}
19621956

19631957
extension SimpleClassSwiftIdentity {
1958+
@_spi(BridgeJS) @_transparent
1959+
public consuming func bridgeJSLowerReturn() -> UnsafeMutableRawPointer {
1960+
return withExtendedLifetime(self) {
1961+
let ptr = Unmanaged.passUnretained(self).toOpaque()
1962+
if _SimpleClassSwiftIdentity_identityTable.insert(ptr).inserted {
1963+
_ = Unmanaged.passRetained(self)
1964+
}
1965+
return ptr
1966+
}
1967+
}
19641968
@_spi(BridgeJS) public consuming func bridgeJSStackPush() {
19651969
let ptr: UnsafeMutableRawPointer = withExtendedLifetime(self) {
19661970
let ptr = Unmanaged.passUnretained(self).toOpaque()
@@ -2001,13 +2005,7 @@ nonisolated(unsafe) var _ClassRoundtripSwiftIdentity_identityTable: Set<UnsafeMu
20012005
public func _bjs_ClassRoundtripSwiftIdentity_init() -> UnsafeMutableRawPointer {
20022006
#if arch(wasm32)
20032007
let ret = ClassRoundtripSwiftIdentity()
2004-
return withExtendedLifetime(ret) {
2005-
let ptr = Unmanaged.passUnretained(ret).toOpaque()
2006-
if _ClassRoundtripSwiftIdentity_identityTable.insert(ptr).inserted {
2007-
_ = Unmanaged.passRetained(ret)
2008-
}
2009-
return ptr
2010-
}
2008+
return ret.bridgeJSLowerReturn()
20112009
#else
20122010
fatalError("Only available on WebAssembly")
20132011
#endif
@@ -2018,13 +2016,7 @@ public func _bjs_ClassRoundtripSwiftIdentity_init() -> UnsafeMutableRawPointer {
20182016
public func _bjs_ClassRoundtripSwiftIdentity_roundtripSimpleClassSwiftIdentity(_ _self: UnsafeMutableRawPointer, _ obj: UnsafeMutableRawPointer) -> UnsafeMutableRawPointer {
20192017
#if arch(wasm32)
20202018
let ret = ClassRoundtripSwiftIdentity.bridgeJSLiftParameter(_self).roundtripSimpleClassSwiftIdentity(_: SimpleClassSwiftIdentity.bridgeJSLiftParameter(obj))
2021-
return withExtendedLifetime(ret) {
2022-
let ptr = Unmanaged.passUnretained(ret).toOpaque()
2023-
if _SimpleClassSwiftIdentity_identityTable.insert(ptr).inserted {
2024-
_ = Unmanaged.passRetained(ret)
2025-
}
2026-
return ptr
2027-
}
2019+
return ret.bridgeJSLowerReturn()
20282020
#else
20292021
fatalError("Only available on WebAssembly")
20302022
#endif
@@ -2035,13 +2027,7 @@ public func _bjs_ClassRoundtripSwiftIdentity_roundtripSimpleClassSwiftIdentity(_
20352027
public func _bjs_ClassRoundtripSwiftIdentity_makeSimpleClassSwiftIdentity(_ _self: UnsafeMutableRawPointer) -> UnsafeMutableRawPointer {
20362028
#if arch(wasm32)
20372029
let ret = ClassRoundtripSwiftIdentity.bridgeJSLiftParameter(_self).makeSimpleClassSwiftIdentity()
2038-
return withExtendedLifetime(ret) {
2039-
let ptr = Unmanaged.passUnretained(ret).toOpaque()
2040-
if _SimpleClassSwiftIdentity_identityTable.insert(ptr).inserted {
2041-
_ = Unmanaged.passRetained(ret)
2042-
}
2043-
return ptr
2044-
}
2030+
return ret.bridgeJSLowerReturn()
20452031
#else
20462032
fatalError("Only available on WebAssembly")
20472033
#endif
@@ -2079,6 +2065,16 @@ public func _bjs_ClassRoundtripSwiftIdentity_release_wrapper(_ pointer: UnsafeMu
20792065
}
20802066

20812067
extension ClassRoundtripSwiftIdentity {
2068+
@_spi(BridgeJS) @_transparent
2069+
public consuming func bridgeJSLowerReturn() -> UnsafeMutableRawPointer {
2070+
return withExtendedLifetime(self) {
2071+
let ptr = Unmanaged.passUnretained(self).toOpaque()
2072+
if _ClassRoundtripSwiftIdentity_identityTable.insert(ptr).inserted {
2073+
_ = Unmanaged.passRetained(self)
2074+
}
2075+
return ptr
2076+
}
2077+
}
20822078
@_spi(BridgeJS) public consuming func bridgeJSStackPush() {
20832079
let ptr: UnsafeMutableRawPointer = withExtendedLifetime(self) {
20842080
let ptr = Unmanaged.passUnretained(self).toOpaque()
@@ -2119,13 +2115,7 @@ nonisolated(unsafe) var _IdentityCacheBenchmarkSwiftIdentity_identityTable: Set<
21192115
public func _bjs_IdentityCacheBenchmarkSwiftIdentity_init() -> UnsafeMutableRawPointer {
21202116
#if arch(wasm32)
21212117
let ret = IdentityCacheBenchmarkSwiftIdentity()
2122-
return withExtendedLifetime(ret) {
2123-
let ptr = Unmanaged.passUnretained(ret).toOpaque()
2124-
if _IdentityCacheBenchmarkSwiftIdentity_identityTable.insert(ptr).inserted {
2125-
_ = Unmanaged.passRetained(ret)
2126-
}
2127-
return ptr
2128-
}
2118+
return ret.bridgeJSLowerReturn()
21292119
#else
21302120
fatalError("Only available on WebAssembly")
21312121
#endif
@@ -2174,6 +2164,16 @@ public func _bjs_IdentityCacheBenchmarkSwiftIdentity_release_wrapper(_ pointer:
21742164
}
21752165

21762166
extension IdentityCacheBenchmarkSwiftIdentity {
2167+
@_spi(BridgeJS) @_transparent
2168+
public consuming func bridgeJSLowerReturn() -> UnsafeMutableRawPointer {
2169+
return withExtendedLifetime(self) {
2170+
let ptr = Unmanaged.passUnretained(self).toOpaque()
2171+
if _IdentityCacheBenchmarkSwiftIdentity_identityTable.insert(ptr).inserted {
2172+
_ = Unmanaged.passRetained(self)
2173+
}
2174+
return ptr
2175+
}
2176+
}
21772177
@_spi(BridgeJS) public consuming func bridgeJSStackPush() {
21782178
let ptr: UnsafeMutableRawPointer = withExtendedLifetime(self) {
21792179
let ptr = Unmanaged.passUnretained(self).toOpaque()

Package.swift

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -225,11 +225,7 @@ let package = Package(
225225
"Generated/JavaScript",
226226
],
227227
swiftSettings: [
228-
.enableExperimentalFeature("Extern"),
229-
// Exposes gated `@JS func getSwift*ForChurn() -> Int` helpers
230-
// that let Task-5 id-recycling tests read `_<Class>_nextId`.
231-
// Do NOT export these in non-test builds.
232-
.define("ENABLE_TEST_INTROSPECTION"),
228+
.enableExperimentalFeature("Extern")
233229
],
234230
linkerSettings: testingLinkerFlags
235231
),
@@ -241,9 +237,7 @@ let package = Package(
241237
"Generated/JavaScript",
242238
],
243239
swiftSettings: [
244-
.enableExperimentalFeature("Extern"),
245-
// Same id-recycling introspection knob as BridgeJSIdentityTests.
246-
.define("ENABLE_TEST_INTROSPECTION"),
240+
.enableExperimentalFeature("Extern")
247241
],
248242
linkerSettings: testingLinkerFlags
249243
),

Plugins/BridgeJS/Sources/BridgeJSCore/ExportSwift.swift

Lines changed: 23 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,8 @@ public class ExportSwift {
3131
self.skeleton = skeleton
3232
}
3333

34-
/// Resolves whether a class is configured for `identityMode: "swift"`.
35-
///
36-
/// Per-class `@JS(identityMode:)` takes priority over the skeleton-wide
37-
/// default from `bridge-js.config.json`. Matches the resolution order in
38-
/// `BridgeJSLink.shouldUseIdentityCache` but checks for the `"swift"` value
39-
/// rather than `"pointer"`.
40-
///
41-
/// Looks up by either the `name` or `swiftCallName` of an exported class —
42-
/// the return-type's `.swiftHeapObject(String)` payload can be either
43-
/// depending on how the type was referenced.
34+
/// Whether a class resolves to `identityMode: "swift"`. Per-class annotation
35+
/// wins over the skeleton-wide config default.
4436
func isSwiftIdentityMode(_ className: String) -> Bool {
4537
guard
4638
let klass = skeleton.classes.first(where: {
@@ -125,15 +117,9 @@ public class ExportSwift {
125117
var abiReturnType: WasmCoreType?
126118
var externDecls: [DeclSyntax] = []
127119
let effects: Effects
128-
/// Predicate that resolves whether a class-name corresponds to an
129-
/// `identityMode: "swift"` class. Default: always false. Threaded
130-
/// through from `ExportSwift.isSwiftIdentityMode(_:)` at construction
131-
/// time so return-lowering can emit the per-class cache glue.
132-
let isSwiftIdentityMode: (String) -> Bool
133120

134-
init(effects: Effects, isSwiftIdentityMode: @escaping (String) -> Bool = { _ in false }) {
121+
init(effects: Effects) {
135122
self.effects = effects
136-
self.isSwiftIdentityMode = isSwiftIdentityMode
137123
}
138124

139125
private func append(_ item: CodeBlockItemSyntax) {
@@ -351,26 +337,6 @@ public class ExportSwift {
351337
}
352338
"""
353339
)
354-
case .swiftHeapObject(let className) where isSwiftIdentityMode(className):
355-
// identityMode: "swift" — Swift's per-class Set tracks which
356-
// pointers have been retained. On miss, retain once; on hit,
357-
// skip. No signalling to JS needed: JS checks its own
358-
// `Map<pointer, wrapper>` and derives hit/miss symmetrically.
359-
// (See DECISIONS.md D20 for why dropping freshBit is safe:
360-
// the Swift Set and the JS Map stay in lockstep by
361-
// construction — both are keyed by pointer and updated at
362-
// the same cache boundaries.)
363-
append(
364-
"""
365-
return withExtendedLifetime(ret) {
366-
let ptr = Unmanaged.passUnretained(ret).toOpaque()
367-
if _\(raw: className)_identityTable.insert(ptr).inserted {
368-
_ = Unmanaged.passRetained(ret)
369-
}
370-
return ptr
371-
}
372-
"""
373-
)
374340
default:
375341
append("return ret.bridgeJSLowerReturn()")
376342
}
@@ -506,8 +472,7 @@ public class ExportSwift {
506472
let isStatic = context.isStatic
507473

508474
let getterBuilder = ExportedThunkBuilder(
509-
effects: Effects(isAsync: false, isThrows: false, isStatic: isStatic),
510-
isSwiftIdentityMode: isSwiftIdentityMode
475+
effects: Effects(isAsync: false, isThrows: false, isStatic: isStatic)
511476
)
512477

513478
if !isStatic {
@@ -528,8 +493,7 @@ public class ExportSwift {
528493
// Generate property setter if not readonly
529494
if !property.isReadonly {
530495
let setterBuilder = ExportedThunkBuilder(
531-
effects: Effects(isAsync: false, isThrows: false, isStatic: isStatic),
532-
isSwiftIdentityMode: isSwiftIdentityMode
496+
effects: Effects(isAsync: false, isThrows: false, isStatic: isStatic)
533497
)
534498

535499
// Lift parameters based on property type
@@ -559,7 +523,7 @@ public class ExportSwift {
559523
}
560524

561525
func renderSingleExportedFunction(function: ExportedFunction) throws -> DeclSyntax {
562-
let builder = ExportedThunkBuilder(effects: function.effects, isSwiftIdentityMode: isSwiftIdentityMode)
526+
let builder = ExportedThunkBuilder(effects: function.effects)
563527
for param in function.parameters {
564528
try builder.liftParameter(param: param)
565529
}
@@ -588,7 +552,7 @@ public class ExportSwift {
588552
callName: String,
589553
returnType: BridgeType
590554
) throws -> DeclSyntax {
591-
let builder = ExportedThunkBuilder(effects: constructor.effects, isSwiftIdentityMode: isSwiftIdentityMode)
555+
let builder = ExportedThunkBuilder(effects: constructor.effects)
592556
for param in constructor.parameters {
593557
try builder.liftParameter(param: param)
594558
}
@@ -602,7 +566,7 @@ public class ExportSwift {
602566
ownerTypeName: String,
603567
instanceSelfType: BridgeType
604568
) throws -> DeclSyntax {
605-
let builder = ExportedThunkBuilder(effects: method.effects, isSwiftIdentityMode: isSwiftIdentityMode)
569+
let builder = ExportedThunkBuilder(effects: method.effects)
606570
if !method.effects.isStatic {
607571
try builder.liftParameter(param: Parameter(label: nil, name: "_self", type: instanceSelfType))
608572
}
@@ -704,10 +668,7 @@ public class ExportSwift {
704668
func renderSingleExportedClass(klass: ExportedClass) throws -> [DeclSyntax] {
705669
var decls: [DeclSyntax] = []
706670

707-
// identityMode: "swift" — per-class state. Post-D19: Swift no longer
708-
// holds the JS wrapper ref; JS keeps the wrapper alive in its own
709-
// strong `Map<pointer, wrapper>`. Swift only tracks "have I already
710-
// issued a wrapper for this pointer?" via a Set.
671+
// .swift identity mode: Swift tracks which pointers have been retained.
711672
if isSwiftIdentityMode(klass.name) {
712673
decls.append(
713674
"nonisolated(unsafe) var _\(raw: klass.name)_identityTable: Set<UnsafeMutableRawPointer> = []"
@@ -753,10 +714,8 @@ public class ExportSwift {
753714
decls.append(DeclSyntax(funcDecl))
754715
}
755716

756-
// identityMode: "swift" — emit the release thunk. Post-D19 there is
757-
// no separate register thunk: Swift does not hold the JS ref, JS does
758-
// (via its `Map<pointer, wrapper>`). Release just drops the Set entry
759-
// and deallocates the Swift heap object.
717+
// .swift identity mode: release thunk + overrides that route every
718+
// return shape (scalar, array element, Optional) through the cache.
760719
if isSwiftIdentityMode(klass.name) {
761720
do {
762721
let releaseDecl = SwiftCodePattern.buildExposedFunctionDecl(
@@ -772,12 +731,18 @@ public class ExportSwift {
772731
decls.append(DeclSyntax(releaseDecl))
773732
}
774733

775-
// Override the default `_BridgedSwiftHeapObject.bridgeJSStackPush`
776-
// so array-element returns (`[SwiftCached]`) go through the same
777-
// identity-cache handshake. Post-D20: no `freshBit` push — JS
778-
// checks its own Map for hit/miss.
779-
let stackPushExt: DeclSyntax = """
734+
let swiftIdentityOverridesExt: DeclSyntax = """
780735
extension \(raw: klass.swiftCallName) {
736+
@_spi(BridgeJS) @_transparent
737+
public consuming func bridgeJSLowerReturn() -> UnsafeMutableRawPointer {
738+
return withExtendedLifetime(self) {
739+
let ptr = Unmanaged.passUnretained(self).toOpaque()
740+
if _\(raw: klass.name)_identityTable.insert(ptr).inserted {
741+
_ = Unmanaged.passRetained(self)
742+
}
743+
return ptr
744+
}
745+
}
781746
@_spi(BridgeJS) public consuming func bridgeJSStackPush() {
782747
let ptr: UnsafeMutableRawPointer = withExtendedLifetime(self) {
783748
let ptr = Unmanaged.passUnretained(self).toOpaque()
@@ -790,7 +755,7 @@ public class ExportSwift {
790755
}
791756
}
792757
"""
793-
decls.append(stackPushExt)
758+
decls.append(swiftIdentityOverridesExt)
794759
}
795760

796761
// Generate ConvertibleToJSValue extension

Plugins/BridgeJS/Sources/BridgeJSCore/Misc.swift

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -346,10 +346,9 @@ public struct BridgeJSConfig: Codable {
346346
///
347347
/// Valid values: `"none"` | `"pointer"` | `"swift"`.
348348
///
349-
/// - `"none"` (or `nil`): No identity tracking. Each boundary crossing produces a fresh JS wrapper.
349+
/// - `"none"` (or `nil`): no identity tracking; each boundary crossing produces a fresh JS wrapper.
350350
/// - `"pointer"`: JS-side identity cache keyed by the Swift pointer (weak refs + `FinalizationRegistry`).
351-
/// - `"swift"`: Swift-side identity cache (opt-in; strong retention of the JS wrapper for the
352-
/// lifetime of the Swift heap object). See spec §3.
351+
/// - `"swift"`: Swift-owned strong identity cache; wrappers live until explicit `release()`.
353352
///
354353
/// A per-class `@JS(identityMode: ...)` annotation overrides this default.
355354
///

Plugins/BridgeJS/Sources/BridgeJSCore/SwiftToSkeleton.swift

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,13 +1202,11 @@ private final class ExportSwiftAPICollector: SyntaxAnyVisitor {
12021202
let identityArg = arguments.first(where: { $0.label?.text == "identityMode" })
12031203
else { return nil }
12041204
let text = identityArg.expression.trimmedDescription
1205-
// Enum member-access form (current `JSIdentityMode` API).
1205+
// Enum member-access (current API).
12061206
if text.contains(".swift") { return "swift" }
12071207
if text.contains(".pointer") { return "pointer" }
12081208
if text.contains(".none") { return "none" }
1209-
// Legacy Bool literals (pre-D8.1 spelling — map for forward compatibility
1210-
// during the transition; the macro itself no longer accepts these, but
1211-
// fixtures and third-party sources may still spell it this way.)
1209+
// Legacy Bool literals, kept for backward compatibility.
12121210
if text == "true" { return "pointer" }
12131211
if text == "false" { return "none" }
12141212
return nil

0 commit comments

Comments
 (0)