fix(mangler): assign correct slot to shadowed function-expression names#21535
fix(mangler): assign correct slot to shadowed function-expression names#21535graphite-app[bot] merged 1 commit intomainfrom
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Merging this PR will not alter performance
Comparing Footnotes
|
9f515db to
ce1489a
Compare
|
This is a workaround in the minifier for post-handling shadowed function-expression names created in Semantic. The real fix should implement oxc-project/backlog#176. However, this would affect every crate and lead to numerous regressions. |
ce1489a to
7e91afa
Compare
7db3439 to
23c546c
Compare
Merge activity
|
…es (#21535) ## Summary https://github.com/oxc-project/monitor-oxc/actions/runs/24643474969/job/72051723921 Fix a mangler bug where a named function expression whose name is shadowed by a same-named declaration in its body collides with an unrelated outer-scope variable in the mangled output. ## The bug A named function expression binds its name into the function's own scope. If anything inside the body redeclares that name — `var`, parameter, `let`, `const`, a nested `function`, or a nested `class` — the scope binding map entry for the fn-expr is overwritten and the fn-expr symbol is orphaned (it still exists, but `scoping.iter_bindings()` never yields it). Previously the slot array was initialised with `0`, so orphans silently kept slot `0` and ended up sharing the mangled name of whichever unrelated symbol actually owned slot `0`. ```js function _() { var x; var f = function foo() { var foo = x; }; } ``` Before this fix, `function foo` and the outer `_` collapsed onto the same identifier in the output. ## Fix - Initialise `slots` to a `SLOT_UNASSIGNED` sentinel (`Slot::MAX`). - Inside the per-scope slot-assignment loop, after slotting a function scope's bindings, check whether the scope's owning AST node is a named function with its `id` symbol still unassigned (the orphan signature — function declarations have their name slotted in the parent scope and so fail this check). If so, look up the shadower in the scope's binding map and copy its slot. Both render with the same mangled name — safe, since every body reference resolves to the shadower, not the orphan. Folded into the existing scope walk so there's no second pass over symbols, and confined to function scopes (the only place this orphaning can happen). - In `tally_slot_frequencies`, skip symbols still at `SLOT_UNASSIGNED` so unrepaired orphans (e.g. when the shadower is `keep_names`-preserved) retain their original names. ## Test plan - [x] New `function_expression_name_shadowed` test covers `var`, parameter, and hoisted-var-in-else-block shadow kinds with concrete mangled-output assertions. - [x] New `shadowed_fn_expr_mangle_is_idempotent` test confirms mangling the output a second time is a fixed point (sanity + multi-shadowed-fn-expr coverage). - [x] Existing mangler and semantic suites pass unchanged.
23c546c to
50e9d26
Compare
### 🐛 Bug Fixes - 48967e8 isolated_declarations: Drop required type check for private parameter properties on private constructors (#21515) (Dunqing) - 91e5bde transformer/typescript: Preserve computed-key static block when class has an empty constructor (#21562) (Dunqing) - 50e9d26 mangler: Assign correct slot to shadowed function-expression names (#21535) (Dunqing) - 065ce47 isolated_declarations: Collect types from private accessors for paired inference (#21516) (Dunqing) - 00fc136 codegen: Preserve coverage comments before object properties (#21312) (bab) - d676e0c minifier: Mark LHS of `??=` as read when converting from `== null &&` (#21546) (Gunnlaugur Thor Briem) ### ⚡ Performance - e45efc5 parser: Reduce `try_parse` usage in favour of `lookahead` (#21532) (Boshen) - ddb1bf8 parser: Avoid redundant `IdentifierReference` clone in shorthand property (#21511) (Boshen) - be2b392 allocator: Store pointers directly in `Arena` (#21483) (overlookmotel) Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com> Co-authored-by: Cameron <cameron.clark@hey.com>

Summary
https://github.com/oxc-project/monitor-oxc/actions/runs/24643474969/job/72051723921
Fix a mangler bug where a named function expression whose name is shadowed by a same-named declaration in its body collides with an unrelated outer-scope variable in the mangled output.
The bug
A named function expression binds its name into the function's own scope. If anything inside the body redeclares that name —
var, parameter,let,const, a nestedfunction, or a nestedclass— the scope binding map entry for the fn-expr is overwritten and the fn-expr symbol is orphaned (it still exists, butscoping.iter_bindings()never yields it).Previously the slot array was initialised with
0, so orphans silently kept slot0and ended up sharing the mangled name of whichever unrelated symbol actually owned slot0.Before this fix,
function fooand the outer_collapsed onto the same identifier in the output.Fix
slotsto aSLOT_UNASSIGNEDsentinel (Slot::MAX).idsymbol still unassigned (the orphan signature — function declarations have their name slotted in the parent scope and so fail this check). If so, look up the shadower in the scope's binding map and copy its slot. Both render with the same mangled name — safe, since every body reference resolves to the shadower, not the orphan. Folded into the existing scope walk so there's no second pass over symbols, and confined to function scopes (the only place this orphaning can happen).tally_slot_frequencies, skip symbols still atSLOT_UNASSIGNEDso unrepaired orphans (e.g. when the shadower iskeep_names-preserved) retain their original names.Test plan
function_expression_name_shadowedtest coversvar, parameter, and hoisted-var-in-else-block shadow kinds with concrete mangled-output assertions.shadowed_fn_expr_mangle_is_idempotenttest confirms mangling the output a second time is a fixed point (sanity + multi-shadowed-fn-expr coverage).