Skip to content

Comments

NFC: BridgeJS: Centralize type info on BridgeType, remove dead cleanup infrastructure#622

Closed
krodak wants to merge 1 commit intoswiftwasm:mainfrom
PassiveLogic:kr/non-stack-abi-refactor
Closed

NFC: BridgeJS: Centralize type info on BridgeType, remove dead cleanup infrastructure#622
krodak wants to merge 1 commit intoswiftwasm:mainfrom
PassiveLogic:kr/non-stack-abi-refactor

Conversation

@krodak
Copy link
Member

@krodak krodak commented Feb 12, 2026

NFC: BridgeJS: Centralize type info on BridgeType, remove dead cleanup infrastructure

Summary

As recently discussed, this PR centralizes scattered type-specific knowledge as computed properties on BridgeType. Codegen reads these properties directly instead of re-deriving the same facts in each switch. Codegen still switches on BridgeType cases, grouping them when they share logic.

Also removes the cleanup infrastructure that became dead code after #635 and #636.

64 files changed, +3667/-5050, net -1383 lines.

What changed

BridgeType computed properties (BridgeJSSkeleton.swift)

Previously, type-specific knowledge (ABI shape, lowering method, optional convention, etc.) was scattered across switches in ExportSwift, ImportTS, BridgeJSLink, and JSGlueGen. Each file re-derived the same facts independently. This PR gathers that information into computed properties on BridgeType itself, so when adding or modifying a type, there's one place to look.

10 focused properties organized by consumer:

  • ABI Shape - wasmParams, wasmReturnType, optionalConvention
  • ExportSwift - accessorTransform, lowerMethod, usesStackLifting
  • ImportTS - importParams, importReturnType
  • JSGlueGen - nilSentinel, optionalScalarKind

Supporting types: OptionalConvention, NilSentinel, OptionalScalarKind, AccessorTransform, LowerMethod.

NilSentinel captures whether a type has a bit pattern that represents nil without an extra isSome flag:

  • jsObject, swiftProtocol - sentinel 0 (object IDs start at 2)
  • swiftHeapObject - null pointer
  • caseEnum, associatedValueEnum - sentinel -1 (never a valid case index)

JSGlueGen helpers (JSGlueGen.swift)

Private helpers that switch on BridgeType for JS-side coercion: isSingleParamScalar(), stackLowerCoerce(), liftCoerce(), lowerCoerce(), varHint(). These consolidate the type-to-JS-coercion mapping that was previously spread across the four direction functions.

Dead cleanup infrastructure removal

After #635 and #636, no type produces cleanup code. Removed all of it:

  • cleanupCode printer from IntrinsicJSFragment.PrintCodeContext
  • tmpStructCleanups array and swift_js_struct_cleanup intrinsic handler
  • _swift_js_struct_cleanup extern and wrapper from BridgeJSIntrinsics.swift
  • Struct helper lower() no longer returns { cleanup } - just pushes fields
  • Enum helper lower() returns caseId directly instead of { caseId, cleanup }
  • init(unsafelyCopying:) no longer calls struct cleanup
  • All optional/array/dictionary wrappers simplified

~1680 lines removed across source and snapshots.

Compositional optional handling

optionalLowerParameter and optionalLiftParameter compose T's existing fragment inside an isSome conditional instead of switching per type. New types added to lowerParameter or liftParameter get optional handling for free. Same for struct fields via structFieldLowerFragment.

Other generated code improvements

  • JSValue optional lowering - __bjs_jsValueLower no longer called on null values; the isSome guard runs before lowering
  • Struct/enum helper factory collapse - () => { return () => ({...}); } simplified to () => ({...}); call-site ()() becomes ()
  • Typed defaults in optional lowering - 0.0 for floats, flag ? 1 : 0 for bools (previously 0 for everything)
  • Dead code removal - unused functions in ExportSwift, JSGlueGen, BridgeJSSkeleton

@krodak krodak self-assigned this Feb 12, 2026
@krodak krodak changed the title NFC: BridgeJS: Descriptor-driven codegen — decoupling type knowledge from code generation NFC: BridgeJS: Descriptor-driven codegen, decoupling type knowledge from code generation Feb 12, 2026
@krodak krodak force-pushed the kr/non-stack-abi-refactor branch 2 times, most recently from 771bd03 to 5726968 Compare February 12, 2026 08:23
@kateinoigakukun
Copy link
Member

kateinoigakukun commented Feb 12, 2026

I think the schema-driven approach makes sense to me.

On the other hand, I think what we really need to think more about is how to composite the type descriptors. e.g. Optional should have a different descriptor depending on wrapped type T, so we need to define a general rule for that. Or define a fallback convention that works without knowing the T's descriptor by using Stack ABI and define specialized descriptors for known cases.

I still haven't checked the entire changes yet so I might be missing something 🙇

@krodak
Copy link
Member Author

krodak commented Feb 12, 2026

@kateinoigakukun thanks for looking at this, I'll think on your feedback and try to progress in this direction when I can; in parallel I'll look for more simplifications around intrinsics like we are both currently doing 👌🏻

@krodak krodak force-pushed the kr/non-stack-abi-refactor branch 4 times, most recently from df5af2e to 1646876 Compare February 12, 2026 15:10
@krodak
Copy link
Member Author

krodak commented Feb 12, 2026

@kateinoigakukun updated PR description and made some more changes and fixes, no rush on this, but PR should be in a good to have a look for further discussion; let me know if some of the changes would satisfy your earlier remarks partially

@krodak krodak force-pushed the kr/non-stack-abi-refactor branch 6 times, most recently from 2536334 to 30f2b6f Compare February 16, 2026 14:00
@krodak krodak marked this pull request as ready for review February 16, 2026 14:24
@kateinoigakukun
Copy link
Member

I'm still thinking what would be the best shape. JFYI, after #635 and #636, currently any type doesn't have cleanup code. (we still have cleanup code emission for container types but they are currently all no-op)

@krodak krodak force-pushed the kr/non-stack-abi-refactor branch from 30f2b6f to 45d949d Compare February 17, 2026 09:17
@krodak krodak changed the title NFC: BridgeJS: Descriptor-driven codegen, decoupling type knowledge from code generation NFC: BridgeJS: Descriptor-driven codegen, remove dead cleanup infrastructure Feb 17, 2026
@krodak krodak force-pushed the kr/non-stack-abi-refactor branch from 45d949d to 1656b22 Compare February 17, 2026 09:29
@krodak
Copy link
Member Author

krodak commented Feb 17, 2026

@kateinoigakukun cleaned up code around clean up, did some smaller improvements, not sure I like final form honestly, but no further better ideas for direction. I know one of the most problematic parts for me initially was to work amongst dispersed BridgeType extensions, which is fixed by type descriptor. Unfortunately it seems that some of the special rules around individual types are hard to abstract beyond the work we've done on BridgeJSIntrinsic and your enums simplifications.
I'll leave it be for time being until I find something new to abstract / improve 🙏🏻

@krodak krodak marked this pull request as draft February 18, 2026 11:58
@krodak krodak changed the title NFC: BridgeJS: Descriptor-driven codegen, remove dead cleanup infrastructure NFC: BridgeJS: Centralize type info on BridgeType, remove dead cleanup infrastructure Feb 18, 2026
@krodak
Copy link
Member Author

krodak commented Feb 18, 2026

This is cleaned up, but still too large to reasonable go through, I'll split it into individual ones, more targeted 🙏🏻

@krodak krodak closed this Feb 18, 2026
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.

2 participants