Skip to content

BridgeJS: Fix for-loop emission in stack codegen#716

Merged
krodak merged 1 commit intoswiftwasm:mainfrom
PassiveLogic:kr/fix-stackcodegen-for-loop-emission
Apr 9, 2026
Merged

BridgeJS: Fix for-loop emission in stack codegen#716
krodak merged 1 commit intoswiftwasm:mainfrom
PassiveLogic:kr/fix-stackcodegen-for-loop-emission

Conversation

@krodak
Copy link
Copy Markdown
Member

@krodak krodak commented Apr 9, 2026

Overview

Three StackCodegen helpers in ExportSwift.swift built their for-loops out of separate CodeBlockItemSyntax fragments ("for … {", one item per body line, "}"). Each fragment was passed through swift-syntax individually, so none of them parsed as a well-formed statement on its own. swift-syntax 600-602's formatter rendered them leniently, but 603 glues the closing brace onto the previous line, producing output like:

for __bjs_elem_ret in ret {
_swift_js_push_i32((__bjs_elem_ret as! _BridgedSwiftProtocolExportable).bridgeJSLowerAsProtocolReturn())}

This was flagged by @kateinoigakukun on #714 for lowerProtocolArrayStatements. Two sibling helpers (lowerProtocolDictionaryStatements, lowerDictionaryStatementsInline) have the same structural bug but no snapshot coverage, so they went unnoticed. While chasing the fix I also hit a latent trivia bug in StructCodegen and EnumCodegen that only surfaces once StackCodegen.lowerStatements starts returning multi-statement results from a struct or enum codepath.

This PR fixes all four problems and adds regression coverage for the previously untested shapes.

1. Emit each lowering for-loop as a single CodeBlockItemSyntax. lowerProtocolArrayStatements, lowerProtocolDictionaryStatements, and lowerDictionaryStatementsInline now build the loop via a multi-line string literal so swift-syntax parses a complete for statement. This matches the pattern already used for the nullable protocol return lowering in _lowerReturnValue. The formatter now produces consistent output on swift-syntax 600-603:

for __bjs_elem_ret in ret {
    _swift_js_push_i32((__bjs_elem_ret as! _BridgedSwiftProtocolExportable).bridgeJSLowerAsProtocolReturn())
}
_swift_js_push_i32(Int32(ret.count))

lowerDictionaryStatementsInline is the trickier case because its body splices in sub-statements from recursive lowerStatements calls for the key and value. The dispatch in lowerDictionaryStatements only routes .nullable and .closure value types into this helper, and both of those paths return single-line lowerings, so the body can be joined as plain indented lines without worrying about nested multi-statement output.

2. Iterate lowering statements individually in struct/enum consumers. StructCodegen.generateStructLowerCode and EnumCodegen.generatePayloadPushingCode previously wrote lowering output via CodeBlockItemListSyntax(statements).description, which concatenates items without inserting separators when they lack leading trivia. ExportedThunkBuilder.append sidesteps the problem by explicitly attaching .newline trivia to each appended item, but the struct and enum paths had no equivalent step, so any multi-statement lowering (such as the array/dictionary of protocol cases) would be collapsed onto a single line. Iterate the statements and pass each one through printer.write(multilineString:) so the printer takes care of line breaks and indentation.

3. Regression coverage for the previously untested shapes.

  • Protocol.swift: add @JS func processDelegatesByName(_:) -> [String: MyViewControllerDelegate] and a var delegatesByName: [String: MyViewControllerDelegate] property on DelegateManager to exercise lowerProtocolDictionaryStatements via both the function-return thunk and the class property getter.
  • DictionaryTypes.swift: add @JS struct Counters { var name: String; var counts: [String: Int?] } and a matching roundtripCounters function to exercise lowerDictionaryStatementsInline via generateStructLowerCode.

Without the fixes, the new struct snapshot lands as glued output like:

for __bjs_kv_counts in self.counts {
    let __bjs_key_counts = __bjs_kv_counts.keylet __bjs_value_counts = __bjs_kv_counts.value__bjs_key_counts.bridgeJSStackPush()__bjs_value_counts.bridgeJSStackPush()
}_swift_js_push_i32(Int32(self.counts.count))

With the fixes, it renders as:

for __bjs_kv_counts in self.counts {
    let __bjs_key_counts = __bjs_kv_counts.key
    let __bjs_value_counts = __bjs_kv_counts.value
    __bjs_key_counts.bridgeJSStackPush()
    __bjs_value_counts.bridgeJSStackPush()
}
_swift_js_push_i32(Int32(self.counts.count))

Verification

Ran the BridgeJS test suite against every swift-syntax version the CI matrix currently exercises, plus 603 which #714 is preparing to add:

swift test --package-path ./Plugins/BridgeJS                                       # default 600.0.1
BRIDGEJS_OVERRIDE_SWIFT_SYNTAX_VERSION=601.0.0 swift test --package-path ./Plugins/BridgeJS
BRIDGEJS_OVERRIDE_SWIFT_SYNTAX_VERSION=602.0.0 swift test --package-path ./Plugins/BridgeJS
BRIDGEJS_OVERRIDE_SWIFT_SYNTAX_VERSION=603.0.0 swift test --package-path ./Plugins/BridgeJS

All 105-106 tests pass and the snapshots are stable across every version. The fixes leave the codegen output structurally identical for every test input that doesn't exercise the bug, so no unrelated snapshot drift.

Relationship to #714

#714 relaxes the swift-syntax constraint to allow 603 and adds a CI entry for it. Its first snapshot-delta fix already covered lowerProtocolArrayStatements - this PR can either land independently to eliminate the remaining latent bugs before that matrix entry flips on, or be rebased on top of it so only the two additional helpers plus the struct/enum consumer fix remain.

@krodak krodak requested a review from kateinoigakukun April 9, 2026 10:46
@krodak krodak self-assigned this Apr 9, 2026
Three `StackCodegen` helpers built their `for`-loops from separate
`CodeBlockItemSyntax` fragments ("for ... {", body, "}"). swift-syntax
603's formatter then renders such partial statements with the closing
brace glued to the previous line. Combine each for-loop into a single
multi-line `CodeBlockItemSyntax` so the formatter produces consistent
output across swift-syntax 600-603.

Also fix a latent bug in `StructCodegen.generateStructLowerCode` and
`EnumCodegen.generatePayloadPushingCode`: they concatenated lowering
statements via `CodeBlockItemListSyntax(statements).description` which
does not insert separators between items that lack leading trivia. The
thunk builder path was fine because `append` explicitly adds `.newline`
trivia, but any multi-statement lowering routed through a struct field
or enum payload was silently glued onto a single line. Iterate over the
statements and write each one individually through the printer.

Add regression coverage for the previously untested codepaths:

- `[String: MyProtocol]` as a function return, class property getter,
  and parameter (exercises `lowerProtocolDictionaryStatements`).
- A `@JS struct` field of type `[String: Int?]` (exercises
  `lowerDictionaryStatementsInline` via `generateStructLowerCode`).
@krodak krodak force-pushed the kr/fix-stackcodegen-for-loop-emission branch from 148e65b to 9f149cc Compare April 9, 2026 11:20
@krodak krodak merged commit 169d2cf into swiftwasm:main Apr 9, 2026
12 checks passed
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