fix: #858 — closure-captured numeric params reach object-literal method body#866
Merged
Conversation
…iteral method body
The inliner's call-site param-substitution path was rewriting captured
`LocalGet(y)` to a literal arg (`Integer(2026)`) inside a nested
closure's body, which empties the closure's `captures` list at the
call site. But the closure body is compiled exactly once per `func_id`
from the FIRST occurrence collected — typically the original (uninlined)
definition, which still references `LocalGet(y)` and lists `captures: [y]`.
The compiled body therefore reads capture slot 0, while the call-site
closure-creation allocates zero capture slots. The mismatch reads
garbage (0.0 in practice) for `y`, breaking the canonical
`function makeDT(y){ return { toDate(): Date { return new
Date(Date.UTC(y, ...)); } } }` shape — perry printed
`-62155492224598` (Date.UTC with year zero) instead of node's
`1778866175402`.
Fix: in all six call-site code paths in `crates/perry-transform/src/inline.rs`
that drive `substitute_locals` over a function body (try_inline_simple_call's
fn Pattern 1, fn Pattern 2, single-Return method, void-method;
try_inline_call's fn + method), pre-walk the body for closure
captures and force-materialize closure-captured params as a fresh `Let`
even when the arg is trivial — except when the arg is already a
`LocalGet` (renumbering inside the closure is harmless). The closure
body keeps its `LocalGet(fresh)` reference and `captures: [fresh]`,
so closure-creation and compiled-body codegen agree on slot index 0.
Closes #858.
Likely closes #859 (downstream — shop-admin's @perryts/mysql
`MyDateTime.toDate()` SIGBUS site has identical shape; not separately
verified end-to-end in this worktree).
Refs #793.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #858. Likely closes #859 (downstream — shop-admin SIGBUS site has identical shape; not separately verified end-to-end in this worktree).
Repro (issue body, 12 lines)
Pre-fix perry:
INLINE: -62155492224598(that'sDate.UTC(0, 4, 15, ...)—yreads as 0). Node +tsx:INLINE: 1778866175402.Post-fix: byte-identical to Node —
INLINE: 1778866175402/HELPER: ... iso= 2026-05-15T17:29:35.402Z.Root cause
The inliner's call-site param-substitution path (
try_inline_simple_callPattern 1 and friends incrates/perry-transform/src/inline.rs) short-circuits "literal-trivial" args (Integer/Number/Bool/String/Null/Undefined peris_trivial_expr) intoparam_map.insert(param.id, arg.clone())and then runssubstitute_localsover the cloned function body.substitute_locals'sExpr::Closurearm recurses into the closure body, substitutes the capturedLocalGet(y)with the literal, and dropsyfrom the closure'scaptureslist.But closures are compiled exactly once per
func_idbycompile_closure, keyed by the FIRST occurrencecollect_closures_in_stmtssaw — typically the original (uninlined)Expr::Closureinside the source function's body, which still hasbody: [...LocalGet(y)...]andcaptures: [y]. The compiled body therefore reads capture slot 0. At the call site,lower_exprconsults the inlinedExpr::Closure(wherecaptures: []), socompute_auto_capturesallocates zero capture slots. Reading slot 0 returns uninitialized bits — 0.0 in practice — soDate.UTC(y, 4, 15, ...)runs asDate.UTC(0, 4, 15, ...).The bug is invisible to type checking and codegen warnings because both sides ARE internally consistent — just with each other disagreeing on the
func_id↔ capture-shape contract.Fix
New helper
collect_closure_captured_local_ids(one walk of the callee's body collecting every closure'scaptures/mutable_captures) drives a force-materialize rule at all six call-site code paths ininline.rsthat runsubstitute_locals/substitute_locals_in_stmts:try_inline_simple_call: fn-call Pattern 1 (single Return), fn-call Pattern 2 (Let-then-Return), single-Return method, void-methodtry_inline_call: fn-call, method-callFor each
(param, arg)pair: ifparam.idis in the closure-captured set ANDargis a trivial-but-not-LocalGetliteral, insert a freshLet __param_id = arginto setup_stmts and mapparam.id → LocalGet(fresh_id)instead of substituting the literal in place. ALocalGet → LocalGetsubstitution is benign because the closure body still references a local, just renumbered, and the captures list rewrites consistently.The closure body retains its
LocalGet(fresh_id)reference andcaptures: [fresh_id]stays non-empty — so closure-creation (lower_expr+compute_auto_captures) and compiled-body emission (compile_closure) agree on capture slot index 0.Why this also (likely) closes #859
@perryts/mysql'smakeMyDateTime(y, mo, d, h, mi, s, ms)is the exact same shape:Pre-fix, every numeric param routed through the same substitute-into-closure-body path; post-fix they all read correctly. The downstream SIGBUS in shop-admin is consistent with
Date.UTCreceiving an all-zero arg vector and producing a NaN/out-of-range Date that's then handed to a native-method dispatch expecting a valid date pointer. I called it "likely closes #859" rather than "closes #859" because shop-admin isn't part of this worktree — please flip to "Closes" once verified end-to-end, or file a follow-up if the SIGBUS still reproduces.Validation
test-files/test_issue_858_closure_numeric_capture.ts(7 closure-capture shapes: method shorthand with numeric/string/multi-primitive captures, arrow-returning shape, arrow nested inside object literal value,Array.prototype.mapcallback, multi-stmt Pattern 2). All seven byte-identical tonode --experimental-strip-types../run_parity_tests.sh): 345/372 pass; all 27 parity failures + 18 compile failures pre-listed intest-parity/known_failures.json. Zero new failures.cargo test --release --workspace(excluding cross-host UI crates per CLAUDE.md): green.Files touched
crates/perry-transform/src/inline.rs— new helper + six call-site updatestest-files/test_issue_858_closure_numeric_capture.ts— new regression testCargo.toml+CLAUDE.mdversion bump 0.5.921 → 0.5.922CHANGELOG.md—## v0.5.922 — fix(transform): #858 …entryTest plan
INLINE: -62155492224598INLINE: 1778866175402(byte-identical to Node)cargo build --release -p perry-runtime -p perry-stdlib && cargo build --release./run_parity_tests.sh: zero new failurescargo test --release --workspace --exclude perry-ui-ios --exclude perry-ui-tvos --exclude perry-ui-watchos --exclude perry-ui-visionos --exclude perry-ui-android --exclude perry-ui-windows --exclude perry-ui-gtk4: green