Skip to content

BridgeJS: Simplify Stack ABI for Optional#674

Merged
kateinoigakukun merged 1 commit intomainfrom
yt/fix-make-unittest
Feb 25, 2026
Merged

BridgeJS: Simplify Stack ABI for Optional#674
kateinoigakukun merged 1 commit intomainfrom
yt/fix-make-unittest

Conversation

@kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Feb 25, 2026

Now Optional conforms to _BridgedSwiftStackType whenever Wrapped conforms to _BridgedSwiftStackType. This makes generic programming easier and reduces boilerplate in intrinsics and codegen.

For specialized handling of certain types like associated value enums, where the presence of a value is encoded as a sentinel case ID (-1), we implement bridgeJSStack{Push,Pop}AsOptional methods, which is on the witness table of _BridgedSwiftStackType.

Also now some of optional types no longer pushes placeholders values for nil cases when pushing in JS and popping in Swift.

Now `Optional` conforms to `_BridgedSwiftStackType` whenever `Wrapped`
conforms to `_BridgedSwiftStackType`. This makes generic programming
easier and reduces boilerplate in intrinsics and codegen.

For specialized handling of certain types like associated value enums,
where the presence of a value is encoded as a sentinel case ID (-1),
we implement `bridgeJSStack{Push,Pop}AsOptional` methods, which is on
the witness table of `_BridgedSwiftStackType`.

Also now some of optional types no longer pushes placeholders values for
`nil` cases when pushing in JS and popping in Swift.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR simplifies the Stack ABI for Optional types in BridgeJS by making Optional conform to _BridgedSwiftStackType whenever its Wrapped type does. This architectural change reduces boilerplate and makes generic programming easier.

Changes:

  • Introduces specialization methods bridgeJSStackPopAsOptional and bridgeJSStackPushAsOptional on the _BridgedSwiftStackType protocol
  • Makes Optional conform to _BridgedSwiftStackType with a conditional conformance
  • Removes specialized bridgeJSStackPop implementations from various Optional extensions (for scalar types, strings, JSObject, heap objects, case enums, etc.)
  • Implements efficient optional handling for associated value enums using -1 as a sentinel instead of separate discriminator
  • Removes emission of placeholder values for nil cases in JavaScript codegen
  • Simplifies Swift codegen by removing optional-specific unwrapping boilerplate

Reviewed changes

Copilot reviewed 12 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Sources/JavaScriptKit/BridgeJSIntrinsics.swift Adds specialization methods to _BridgedSwiftStackType, makes Optional conform conditionally, removes specialized Optional implementations, adds -1 sentinel for optional enums
Plugins/BridgeJS/Sources/BridgeJSCore/ExportSwift.swift Removes optional-specific codegen, simplifies to unified bridgeJSStackPush calls
Plugins/BridgeJS/Sources/BridgeJSLink/JSGlueGen.swift Removes placeholder emission logic, adds specialized handling for optional associated value enums
Tests/BridgeJSRuntimeTests/Generated/BridgeJS.swift Generated Swift test code showing simplified optional handling without placeholder boilerplate
Plugins/BridgeJS/Tests/BridgeJSToolTests/Snapshots/BridgeJSLinkTests/*.js Generated JavaScript showing no placeholder emission for nil, conditional value pushing
Plugins/BridgeJS/Tests/BridgeJSToolTests/Snapshots/BridgeJSCodegenTests/*.swift Generated Swift showing simplified bridgeJSStackPush calls without manual unwrapping
Benchmarks/Sources/Generated/BridgeJS.swift Benchmark code updated with simplified optional handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@JSFunction static func jsRoundTripOptionalJSValueArrayNull(
_ v: Optional<[JSValue]>
) throws(JSException) -> Optional<[JSValue]>
// @JSFunction static func jsRoundTripOptionalJSValueArrayUndefined(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these supposed to be commented out?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they are not supported and will be followed up later PRs

Copy link
Member

@krodak krodak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done 👌🏻

@kateinoigakukun kateinoigakukun merged commit 3badf17 into main Feb 25, 2026
12 checks passed
@kateinoigakukun kateinoigakukun deleted the yt/fix-make-unittest branch February 25, 2026 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants