mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
fix(mangler): assign correct slot to shadowed function-expression names (#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.
This commit is contained in:
1 parent
a006da8368
commit
50e9d26714
2 files changed
+89
-1
No files matched your search
@@ -47,6 +47,11 @@ impl MangleOptions {
|
||||
|
||||
type Slot = u32;
|
||||
|
||||
/// Sentinel for symbols the main assignment pass skipped; repaired below.
|
||||
/// Safe because `SymbolId` is `NonMaxU32`, so `symbols_len` maxes out at
|
||||
/// `u32::MAX - 1` and real slot values can never reach `Slot::MAX`.
|
||||
const SLOT_UNASSIGNED: Slot = Slot::MAX;
|
||||
|
||||
/// Enum to handle both owned and borrowed allocators. This is not `Cow` because that type
|
||||
/// requires `ToOwned`/`Clone`, which is not implemented for `Allocator`. Although this does
|
||||
/// incur some pointer indirection on each reference to the allocator, it allows the API to be
|
||||
@@ -323,7 +328,10 @@ impl<'t> Mangler<'t> {
|
||||
);
|
||||
|
||||
// All symbols with their assigned slots. Keyed by symbol id.
|
||||
let mut slots = Vec::from_iter_in(iter::repeat_n(0, scoping.symbols_len()), temp_allocator);
|
||||
let mut slots = Vec::from_iter_in(
|
||||
iter::repeat_n(SLOT_UNASSIGNED, scoping.symbols_len()),
|
||||
temp_allocator,
|
||||
);
|
||||
|
||||
// Stores the lived scope ids for each slot. Keyed by slot number.
|
||||
// Pre-allocate capacity based on number of symbols (upper bound for slots).
|
||||
@@ -455,6 +463,26 @@ impl<'t> Mangler<'t> {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Repair an orphaned named-fn-expr name: a same-named body declaration
|
||||
// (`var foo`, parameter `foo`) overwrites the fn-expr's binding-map entry,
|
||||
// so the fn-expr symbol never appears in `bindings` and the main pass leaves
|
||||
// its slot at `SLOT_UNASSIGNED`. Copy the shadower's slot so both render with
|
||||
// the same mangled name — safe because every body reference resolves to the
|
||||
// shadower, not the orphan. Only function expressions can host this orphaning;
|
||||
// a function declaration's name lives in the parent scope and is unaffected.
|
||||
// The shadower-slot guard catches the case where the shadower is in
|
||||
// `keep_name_symbols` (filtered out above and itself unassigned).
|
||||
if scoping.scope_flags(scope_id).is_function()
|
||||
&& let Some(func) = ast_nodes.kind(scoping.get_node_id(scope_id)).as_function()
|
||||
&& func.is_expression()
|
||||
&& let Some(id) = &func.id
|
||||
&& let Some(&shadower) = bindings.get(&id.name)
|
||||
&& shadower != id.symbol_id()
|
||||
&& slots[shadower.index()] != SLOT_UNASSIGNED
|
||||
{
|
||||
slots[id.symbol_id().index()] = slots[shadower.index()];
|
||||
}
|
||||
}
|
||||
|
||||
let total_number_of_slots = slot_liveness.len();
|
||||
@@ -569,6 +597,9 @@ impl<'t> Mangler<'t> {
|
||||
);
|
||||
|
||||
for (symbol_id, &slot) in slots.iter().enumerate() {
|
||||
if slot == SLOT_UNASSIGNED {
|
||||
continue;
|
||||
}
|
||||
let symbol_id = SymbolId::from_usize(symbol_id);
|
||||
let symbol_scope_id = scoping.symbol_scope_id(symbol_id);
|
||||
if symbol_scope_id == root_scope_id
|
||||
|
||||
@@ -219,6 +219,63 @@ fn private_member_mangling() {
|
||||
});
|
||||
}
|
||||
|
||||
/// A named function expression whose name is shadowed by a same-named declaration in its
|
||||
/// body must receive the same mangled name as the shadowing symbol; otherwise the emitted
|
||||
/// fn-expr name collides with whichever unrelated outer-scope variable happens to own slot 0.
|
||||
#[test]
|
||||
fn function_expression_name_shadowed() {
|
||||
let options = MangleOptions::default();
|
||||
|
||||
// `var` shadow.
|
||||
test(
|
||||
"function _() { var x; var f = function foo() { var foo = x; } }",
|
||||
"function _() { var e; var t = function t() { var t = e; } }",
|
||||
options,
|
||||
);
|
||||
|
||||
// Parameter shadow.
|
||||
test(
|
||||
"function _() { var x; (function foo(foo) { foo + x })() }",
|
||||
"function _() { var e; (function t(t) { t + e; })(); }",
|
||||
options,
|
||||
);
|
||||
|
||||
// `var` inside an `else` block — still hoists through the block scope to the fn-expr scope.
|
||||
test(
|
||||
"function _() { var x; var f = function foo() { if (x) {} else { var foo = x; } } }",
|
||||
"function _() { var e; var t = function t() { if (e) {} else { var t = e; } } }",
|
||||
options,
|
||||
);
|
||||
}
|
||||
|
||||
/// Re-mangling a mangled output must be a fixed point.
|
||||
#[test]
|
||||
fn shadowed_fn_expr_mangle_is_idempotent() {
|
||||
let options = MangleOptions::default();
|
||||
|
||||
let cases = [
|
||||
// Basic `var` shadow (sanity).
|
||||
"function _() { var x; var f = function foo() { var foo = x; } }",
|
||||
// Two shadowed fn-exprs in the same scope — unique coverage beyond `_shadowed` test.
|
||||
"
|
||||
(function() {
|
||||
var a = 1;
|
||||
var b = function foo() { var foo = a; };
|
||||
var c = function bar() { var bar = a; };
|
||||
})();
|
||||
",
|
||||
];
|
||||
|
||||
for case in cases {
|
||||
let pass1 = mangle(case, options);
|
||||
let pass2 = mangle(&pass1, options);
|
||||
assert_eq!(
|
||||
pass1, pass2,
|
||||
"\nIdempotency failure for:\n{case}\nPass 1:\n{pass1}\nPass 2:\n{pass2}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Annex B.3.2.1: In sloppy mode, function declarations inside blocks have var-like hoisting.
|
||||
/// The mangler must not assign the same name to such a function and an outer `var` binding.
|
||||
#[test]
|
||||
|
||||
Reference in new issue
Block a user