Skip to content

Commit 76d11c0

Browse files
committed
BridgeJS: Address @js(as:) review feedback
Follow-up to the review on #750: - ImportTS: give the unreachable `.alias` cases in loweringParameterInfo / liftingReturnInfo a message stating the `.unaliased` invariant, so a future change that breaks it fails loudly instead of trapping silently. - JSGlueGen: in `optionalConvention` and `wasmParams`, delegate `.alias` to its underlying type rather than `preconditionFailure()`. These switch on `self` (not `.unaliased`), so this removes a latent crash for alias-wrapped values and matches how abiReturnType / mangleTypeName already handle `.alias`. - SwiftToSkeleton: diagnose `@JS(as:)` combined with `namespace:` instead of silently dropping the namespace; an alias adopts its representation's placement. Adds a diagnostics test. - BridgeJSSkeleton: note why alias mangling uses the (unique) swiftCallName only. - Docs: add an "Exporting a Type With a Custom JS Representation" article and link it from the exporting topics.
1 parent fce4193 commit 76d11c0

11 files changed

Lines changed: 171 additions & 16 deletions

File tree

Plugins/BridgeJS/Sources/BridgeJSCore/ImportTS.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -958,7 +958,7 @@ extension BridgeType {
958958
case .array, .dictionary:
959959
return LoweringParameterInfo(loweredParameters: [])
960960
case .alias:
961-
preconditionFailure()
961+
preconditionFailure("`.alias` must be resolved by `.unaliased` before reaching loweringParameterInfo")
962962
}
963963
}
964964

@@ -1032,7 +1032,7 @@ extension BridgeType {
10321032
case .array, .dictionary:
10331033
return LiftingReturnInfo(valueToLift: nil)
10341034
case .alias:
1035-
preconditionFailure()
1035+
preconditionFailure("`.alias` must be resolved by `.unaliased` before reaching liftingReturnInfo")
10361036
}
10371037
}
10381038
}

Plugins/BridgeJS/Sources/BridgeJSCore/SwiftToSkeleton.swift

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1635,7 +1635,7 @@ private final class ExportSwiftAPICollector: SyntaxAnyVisitor {
16351635
}
16361636

16371637
if let aliasTarget = parent.extractAliasTarget(from: jsAttribute) {
1638-
recordAlias(node: node, aliasTarget: aliasTarget)
1638+
recordAlias(node: node, jsAttribute: jsAttribute, aliasTarget: aliasTarget)
16391639
return .skipChildren
16401640
}
16411641

@@ -1739,9 +1739,20 @@ private final class ExportSwiftAPICollector: SyntaxAnyVisitor {
17391739

17401740
private func recordAlias(
17411741
node: some SyntaxProtocol & NamedDeclSyntax,
1742+
jsAttribute: AttributeSyntax,
17421743
aliasTarget: TypeSyntax
17431744
) {
17441745
let swiftCallName = SwiftToSkeleton.computeSwiftCallName(for: node, itemName: node.name.text)
1746+
if extractNamespace(from: jsAttribute) != nil {
1747+
errors.append(
1748+
DiagnosticError(
1749+
node: node,
1750+
message: "`namespace` is not supported on `@JS(as:)` types",
1751+
hint: "Remove the `namespace:` argument; an alias adopts its target's representation"
1752+
)
1753+
)
1754+
return
1755+
}
17451756
var lookupErrors: [DiagnosticError] = []
17461757
guard
17471758
let aliasBridgeType = parent.aliasType(
@@ -1774,7 +1785,7 @@ private final class ExportSwiftAPICollector: SyntaxAnyVisitor {
17741785
}
17751786

17761787
if let aliasTarget = parent.extractAliasTarget(from: jsAttribute) {
1777-
recordAlias(node: node, aliasTarget: aliasTarget)
1788+
recordAlias(node: node, jsAttribute: jsAttribute, aliasTarget: aliasTarget)
17781789
return .skipChildren
17791790
}
17801791

@@ -1962,7 +1973,7 @@ private final class ExportSwiftAPICollector: SyntaxAnyVisitor {
19621973
}
19631974

19641975
if let aliasTarget = parent.extractAliasTarget(from: jsAttribute) {
1965-
recordAlias(node: node, aliasTarget: aliasTarget)
1976+
recordAlias(node: node, jsAttribute: jsAttribute, aliasTarget: aliasTarget)
19661977
return .skipChildren
19671978
}
19681979

Plugins/BridgeJS/Sources/BridgeJSLink/JSGlueGen.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2685,8 +2685,8 @@ private extension BridgeType {
26852685
return .stackABI
26862686
case .nullable(let wrapped, _):
26872687
return wrapped.optionalConvention
2688-
case .alias:
2689-
preconditionFailure()
2688+
case .alias(_, let underlying):
2689+
return underlying.optionalConvention
26902690
}
26912691
}
26922692

@@ -2783,8 +2783,8 @@ private extension BridgeType {
27832783
return []
27842784
case .nullable(let wrapped, _):
27852785
return wrapped.wasmParams
2786-
case .alias:
2787-
preconditionFailure()
2786+
case .alias(_, let underlying):
2787+
return underlying.wasmParams
27882788
}
27892789
}
27902790

Plugins/BridgeJS/Sources/BridgeJSSkeleton/BridgeJSSkeleton.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1770,6 +1770,8 @@ extension BridgeType {
17701770
// Dictionary mangling: "SD" prefix followed by value type (key is always String)
17711771
return "SD\(valueType.mangleTypeName)"
17721772
case .alias(let name, _):
1773+
// `name` is the namespace-qualified swiftCallName (unique), so the underlying
1774+
// representation isn't mangled in - aliases bridge via their JS type's ABI.
17731775
return "Al\(name.count)\(name)"
17741776
}
17751777
}

Plugins/BridgeJS/Tests/BridgeJSToolTests/DiagnosticsTests.swift

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,27 @@ import Testing
426426
}
427427
}
428428

429+
@Test
430+
func jsAsWithNamespaceDiagnostic() throws {
431+
let source = """
432+
@JS final class Box { @JS init() {} }
433+
@JS(as: Box.self, namespace: "Foo") struct Wrapped {
434+
consuming func bridgeToJS() -> Box { fatalError() }
435+
static func bridgeFromJS(_ value: consuming Box) -> Wrapped { fatalError() }
436+
}
437+
"""
438+
let swiftAPI = SwiftToSkeleton(
439+
progress: .silent,
440+
moduleName: "TestModule",
441+
exposeToGlobal: false,
442+
externalModuleIndex: .empty
443+
)
444+
swiftAPI.addSourceFile(Parser.parse(source: source), inputFilePath: "test.swift")
445+
#expect(throws: BridgeJSCoreDiagnosticError.self) {
446+
_ = try swiftAPI.finalize()
447+
}
448+
}
449+
429450
@Test
430451
func omitsNextLineWhenErrorIsOnLastLine() throws {
431452
let source = """

Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/DocComments.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
{
22
"exported" : {
3+
"aliases" : [
4+
5+
],
36
"classes" : [
47
{
58
"constructor" : {

Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/Alias.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,14 +78,14 @@ export async function createInstantiator(options, swift) {
7878
bjs["swift_js_init_memory"] = function(sourceId, bytesPtr) {
7979
const source = swift.memory.getObject(sourceId);
8080
swift.memory.release(sourceId);
81-
const bytes = new Uint8Array(memory.buffer, bytesPtr);
81+
const bytes = new Uint8Array(memory.buffer, bytesPtr >>> 0);
8282
bytes.set(source);
8383
}
8484
bjs["swift_js_make_js_string"] = function(ptr, len) {
8585
return swift.memory.retain(decodeString(ptr, len));
8686
}
8787
bjs["swift_js_init_memory_with_result"] = function(ptr, len) {
88-
const target = new Uint8Array(memory.buffer, ptr, len);
88+
const target = new Uint8Array(memory.buffer, ptr >>> 0, len >>> 0);
8989
target.set(tmpRetBytes);
9090
tmpRetBytes = undefined;
9191
}
@@ -345,6 +345,7 @@ export async function createInstantiator(options, swift) {
345345
/// Represents a Swift heap object like a class instance or an actor instance.
346346
class SwiftHeapObject {
347347
static __wrap(pointer, deinit, prototype, identityCache) {
348+
pointer = pointer >>> 0;
348349
const makeFresh = (identityMap) => {
349350
const obj = Object.create(prototype);
350351
const state = { pointer, deinit, hasReleased: false, identityMap };

Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/AliasInClosure.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export async function createInstantiator(options, swift) {
3939
const state = { pointer, file, line, unregistered: false };
4040
const real = (...args) => {
4141
if (state.unregistered) {
42-
const bytes = new Uint8Array(memory.buffer, state.file);
42+
const bytes = new Uint8Array(memory.buffer, state.file >>> 0);
4343
let length = 0;
4444
while (bytes[length] !== 0) { length += 1; }
4545
const fileID = decodeString(state.file, length);
@@ -70,14 +70,14 @@ export async function createInstantiator(options, swift) {
7070
bjs["swift_js_init_memory"] = function(sourceId, bytesPtr) {
7171
const source = swift.memory.getObject(sourceId);
7272
swift.memory.release(sourceId);
73-
const bytes = new Uint8Array(memory.buffer, bytesPtr);
73+
const bytes = new Uint8Array(memory.buffer, bytesPtr >>> 0);
7474
bytes.set(source);
7575
}
7676
bjs["swift_js_make_js_string"] = function(ptr, len) {
7777
return swift.memory.retain(decodeString(ptr, len));
7878
}
7979
bjs["swift_js_init_memory_with_result"] = function(ptr, len) {
80-
const target = new Uint8Array(memory.buffer, ptr, len);
80+
const target = new Uint8Array(memory.buffer, ptr >>> 0, len >>> 0);
8181
target.set(tmpRetBytes);
8282
tmpRetBytes = undefined;
8383
}
@@ -312,6 +312,7 @@ export async function createInstantiator(options, swift) {
312312
/// Represents a Swift heap object like a class instance or an actor instance.
313313
class SwiftHeapObject {
314314
static __wrap(pointer, deinit, prototype, identityCache) {
315+
pointer = pointer >>> 0;
315316
const makeFresh = (identityMap) => {
316317
const obj = Object.create(prototype);
317318
const state = { pointer, deinit, hasReleased: false, identityMap };

Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/EnumAlias.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,14 @@ export async function createInstantiator(options, swift) {
4545
bjs["swift_js_init_memory"] = function(sourceId, bytesPtr) {
4646
const source = swift.memory.getObject(sourceId);
4747
swift.memory.release(sourceId);
48-
const bytes = new Uint8Array(memory.buffer, bytesPtr);
48+
const bytes = new Uint8Array(memory.buffer, bytesPtr >>> 0);
4949
bytes.set(source);
5050
}
5151
bjs["swift_js_make_js_string"] = function(ptr, len) {
5252
return swift.memory.retain(decodeString(ptr, len));
5353
}
5454
bjs["swift_js_init_memory_with_result"] = function(ptr, len) {
55-
const target = new Uint8Array(memory.buffer, ptr, len);
55+
const target = new Uint8Array(memory.buffer, ptr >>> 0, len >>> 0);
5656
target.set(tmpRetBytes);
5757
tmpRetBytes = undefined;
5858
}
@@ -237,6 +237,7 @@ export async function createInstantiator(options, swift) {
237237
/// Represents a Swift heap object like a class instance or an actor instance.
238238
class SwiftHeapObject {
239239
static __wrap(pointer, deinit, prototype, identityCache) {
240+
pointer = pointer >>> 0;
240241
const makeFresh = (identityMap) => {
241242
const obj = Object.create(prototype);
242243
const state = { pointer, deinit, hasReleased: false, identityMap };

Sources/JavaScriptKit/Documentation.docc/Articles/BridgeJS/Exporting-Swift-to-JavaScript.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ Configure your package and build for JavaScript as described in <doc:Setting-up-
2222
- <doc:Exporting-Swift-Closure>
2323
- <doc:Exporting-Swift-Protocols>
2424
- <doc:Exporting-Swift-Optional>
25+
- <doc:Exporting-Swift-Custom-Representation>
2526
- <doc:Exporting-Swift-Default-Parameters>
2627
- <doc:Exporting-Swift-Static-Functions>
2728
- <doc:Exporting-Swift-Static-Properties>

0 commit comments

Comments
 (0)