fix(mangler): prevent slot reuse for block-scoped functions in sloppy mode

[Dunqing]

Summary

In sloppy mode JavaScript, function declarations inside blocks (if, try, switch, {}) have Annex B.3.2.1 semantics: they create an implicit var-like assignment in the enclosing function scope at block exit. The mangler was treating these as purely block-scoped (like let/const), allowing slot reuse with outer var bindings. When both got the same mangled name, the Annex B assignment would overwrite the outer variable at runtime, changing program behavior.

Before (broken)

// Original
function Z() { var param = "hello"; if (true) { function nested() {} } return param; }
// Mangled — both renamed to 'e', Annex B overwrites param!
function Z() { var e = "hello"; if (true) { function e() {} } return e; } // returns [Function: e]

After (fixed)

// Mangled — different names, no Annex B conflict
function Z() { var e = "hello"; if (true) { function t() {} } return e; } // returns "hello"

How the fix works

The mangler assigns short names by grouping variables into slots. Variables in the same slot get the same mangled name. Two variables can share a slot if their lifetimes don't overlap.

For var declarations in blocks, this isn't a problem — oxc_semantic already hoists them to the enclosing function scope (binder.rs lines 46-98), so the mangler sees them in the function scope's bindings and they naturally get unique slots.

But for Annex B function declarations, semantic does NOT hoist them — they stay in the block scope. The mangler processes scopes in DFS order (parent before children):

1. Function scope first → assigns var param to slot 0
2. Block scope next → slot 0 isn't "live" in this block (param not referenced there) → function reuses slot 0
3. Both get name e → Annex B overwrites param at runtime

The fix: When processing a sloppy-mode block scope (!scope_flags.is_var() && !scope_flags.is_strict_mode()), partition bindings into two regions:

[regular bindings ... | function declarations ...]
                      ^ regular_count

Only the first regular_count bindings are allowed to reuse existing slots (.take(regular_count)). Function declarations after that boundary always get fresh (new) slots, preventing them from reusing an outer var's slot.

Sibling scopes reusing the Annex B function's fresh slot is safe — only let/const/catch bindings can appear in sibling block scopes (var is hoisted), and these are truly block-scoped, so sharing a name just creates a harmless temporary shadow.

Strict mode and ES modules are unaffected — the guard ensures no Annex B handling (and no size regression) for strict code.

Fixes #20610. Fixes #14316.

Downstream: vitejs/vite#22009, rolldown/rolldown#8791.

Test plan

• 12 sloppy-mode snapshot test cases covering: if, try, plain blocks, parameters, nested blocks, arrow callbacks, multiple functions, cross-references, sibling let, sibling Annex B functions, sibling catch parameters, and sibling var (hoisted)
• All existing mangler tests pass unchanged
• No minsize benchmark regression (all benchmarks are ES modules / strict mode)

[Dunqing]

• fix(mangler): prevent slot reuse for block-scoped functions in sloppy mode #20705 👈 (View in Graphite)
• main

---

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• 0-merge - adds this PR to the back of the merge queue
• hotfix - for urgent changes, fast-track this PR to the front of 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.

[codspeed-hq]

Merging this PR will degrade performance by 5.53%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 5 regressed benchmarks
✅ 44 untouched benchmarks
⏩ 7 skipped benchmarks¹

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
❌	Simulation	mangler[RadixUIAdoptionSection.jsx_keep_names]	20.2 µs	20.9 µs	-3.31%
❌	Simulation	mangler[cal.com.tsx]	2.7 ms	2.8 ms	-4.6%
❌	Simulation	mangler[RadixUIAdoptionSection.jsx]	14.4 µs	15.2 µs	-5.38%
❌	Simulation	mangler[react.development.js]	225.9 µs	239.1 µs	-5.53%
❌	Simulation	mangler[binder.ts]	644.4 µs	672.7 µs	-4.2%

---

_{Comparing fix/mangler-annex-b-block-scoped-function (14c83d6) with main (b02fe6e)²}

Footnotes

1. 7 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

2. No successful run was found on main (2938e8a) during the generation of this report, so b02fe6e was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[sapphi-red]

I'm not sure why we need "Part 1". Isn't that we just need to treat function decls as a var decl? If not, why is "Part 1" not needed for var decls?

[Dunqing]

I'm not sure why we need "Part 1". Isn't that we just need to treat function decls as a var decl? If not, why is "Part 1" not needed for var decls?

Great question — you're right that Part 2 is unnecessary. I've removed it.

Why var doesn't need Part 1: oxc_semantic already hoists var bindings from block scopes to the enclosing function scope.

oxc/crates/oxc_semantic/src/binder.rs

Lines 46 to 98 in 9d7cb62

	// ------------------ var hosting ------------------
	let mut target_scope_id = builder.current_scope_id;
	let mut var_scope_ids = vec![];
	// Collect all scopes where variable hoisting can occur
	for scope_id in builder.scoping.scope_ancestors(target_scope_id) {
	let flags = builder.scoping.scope_flags(scope_id);
	if flags.is_var() {
	target_scope_id = scope_id;
	break;
	}
	var_scope_ids.push(scope_id);
	}
	self.id.bound_names(&mut |ident| {
	let span = ident.span;
	let name = ident.name;
	let mut declared_symbol_id = None;
	for &scope_id in &var_scope_ids {
	if let Some(symbol_id) =
	builder.check_redeclaration(scope_id, span, name, excludes, true)
	{
	builder.add_redeclare_variable(symbol_id, includes, span);
	declared_symbol_id = Some(symbol_id);
	// Hoist current symbol to target scope when it is not already declared
	// in the target scope.
	if !builder.scoping.scope_has_binding(target_scope_id, name) {
	// remove current scope binding and add to target scope
	// avoid same symbols appear in multi-scopes
	builder.scoping.remove_binding(scope_id, name);
	builder.scoping.add_binding(target_scope_id, name, symbol_id);
	builder.scoping.set_symbol_scope_id(symbol_id, target_scope_id);
	}
	break;
	}
	}
	// If a variable is already declared in the hoisted scopes,
	// we don't need to create another symbol with the same name
	// to make sure they point to the same symbol.
	let symbol_id = declared_symbol_id.unwrap_or_else(|| {
	builder.declare_symbol_on_scope(span, name, target_scope_id, includes, excludes)
	});
	ident.symbol_id.set(Some(symbol_id));
	// Finally, add the variable to all hoisted scopes
	// to support redeclaration checks when declaring variables with the same name later.
	for &scope_id in &var_scope_ids {
	builder.hoisting_variables.entry(scope_id).or_default().insert(name, symbol_id);
	}
	});

So the mangler sees var in the function scope's bindings — it's processed together with other function-scope symbols and naturally gets a unique slot. No reuse conflict is possible.

Why Annex B functions need Part 1: Semantic does NOT hoist them — they stay in the block scope's bindings. The mangler processes scopes in DFS order (parent before children). When it reaches the block scope, it finds the outer var's slot is not "live" in the block (the var isn't referenced there), so it reuses that slot for the function declaration. Both get the same name → Annex B overwrites the var.

I verified Part 2 is not needed by removing it — all tests pass including new edge cases for sibling scopes (let, catch, sibling Annex B functions). Sibling bindings reusing the function's slot is safe because they're truly block-scoped (let/const/catch) — sharing a name just creates a harmless temporary shadow.

[sapphi-red]

Why Annex B functions need Part 1: Semantic does NOT hoist them — they stay in the block scope's bindings. The mangler processes scopes in DFS order (parent before children). When it reaches the block scope, it finds the outer var's slot is not "live" in the block (the var isn't referenced there), so it reuses that slot for the function declaration. Both get the same name → Annex B overwrites the var.

Can we loop over the scopes once and generate the bindings list with the functions hoisted? It will require a bit of overhead, but I think that should allow reusing the variable names.

[Dunqing]

Can we loop over the scopes once and generate the bindings list with the functions hoisted? It will require a bit of overhead, but I think that should allow reusing the variable names.

I've tried the hoisting approach. Please see 14c83d6. It worked, but it looks complicated. I am trying to move the hoisting logic back to Semantic.