NFC: BridgeJS: Centralize type info on BridgeType, remove dead cleanup infrastructure#622
NFC: BridgeJS: Centralize type info on BridgeType, remove dead cleanup infrastructure#622krodak wants to merge 1 commit intoswiftwasm:mainfrom
Conversation
771bd03 to
5726968
Compare
|
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 🙇 |
|
@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 👌🏻 |
df5af2e to
1646876
Compare
|
@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 |
2536334 to
30f2b6f
Compare
30f2b6f to
45d949d
Compare
…anup infrastructure
45d949d to
1656b22
Compare
|
@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 |
|
This is cleaned up, but still too large to reasonable go through, I'll split it into individual ones, more targeted 🙏🏻 |
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 onBridgeTypecases, 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
BridgeTypeitself, so when adding or modifying a type, there's one place to look.10 focused properties organized by consumer:
wasmParams,wasmReturnType,optionalConventionaccessorTransform,lowerMethod,usesStackLiftingimportParams,importReturnTypenilSentinel,optionalScalarKindSupporting types:
OptionalConvention,NilSentinel,OptionalScalarKind,AccessorTransform,LowerMethod.NilSentinelcaptures whether a type has a bit pattern that represents nil without an extraisSomeflag:jsObject,swiftProtocol- sentinel0(object IDs start at 2)swiftHeapObject- null pointercaseEnum,associatedValueEnum- sentinel-1(never a valid case index)JSGlueGen helpers (JSGlueGen.swift)
Private helpers that switch on
BridgeTypefor 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:
cleanupCodeprinter fromIntrinsicJSFragment.PrintCodeContexttmpStructCleanupsarray andswift_js_struct_cleanupintrinsic handler_swift_js_struct_cleanupextern and wrapper fromBridgeJSIntrinsics.swiftlower()no longer returns{ cleanup }- just pushes fieldslower()returns caseId directly instead of{ caseId, cleanup }init(unsafelyCopying:)no longer calls struct cleanup~1680 lines removed across source and snapshots.
Compositional optional handling
optionalLowerParameterandoptionalLiftParametercompose T's existing fragment inside anisSomeconditional instead of switching per type. New types added tolowerParameterorliftParameterget optional handling for free. Same for struct fields viastructFieldLowerFragment.Other generated code improvements
__bjs_jsValueLowerno longer called on null values; theisSomeguard runs before lowering() => { return () => ({...}); }simplified to() => ({...}); call-site()()becomes()0.0for floats,flag ? 1 : 0for bools (previously0for everything)