From ee9f6a4830555cb3c9c69ba1342754ed53e9419a Mon Sep 17 00:00:00 2001 From: Dunqing <29533304+Dunqing@users.noreply.github.com> Date: Mon, 19 Jan 2026 11:44:46 +0000 Subject: [PATCH] fix(mangler): use `retain` instead of `truncate` to remove empty frequency slots (#18225) #18183 introduced a bug when removing empty frequency slots. The code used `truncate` after sorting: ```rust frequencies.sort_unstable_by_key(|x| std::cmp::Reverse(x.frequency)); if let Some(idx) = frequencies.iter().position(|x| x.symbol_ids.is_empty()) { frequencies.truncate(idx); } ``` The problem is that `sort_unstable_by_key` doesn't guarantee order for items with equal keys. When multiple slots have `frequency: 0`, some have symbols (that need renaming but have no references), and some are empty. Since the sort order is unstable, an empty slot could appear **before** a non-empty slot, and `truncate` would incorrectly remove valid slots. This fix uses `retain` instead, and moves it before the sort for better efficiency (sorting fewer elements): ```rust frequencies.retain(|x| !x.symbol_ids.is_empty()); frequencies.sort_unstable_by_key(|x| std::cmp::Reverse(x.frequency)); ``` Note for reviewer: I've locally tested in the `monitor-oxc`; there is no naming collision after this. --- crates/oxc_mangler/src/lib.rs | 6 +++--- crates/oxc_minifier/tests/mangler/snapshots/mangler.snap | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/oxc_mangler/src/lib.rs b/crates/oxc_mangler/src/lib.rs index 8deabaf82d94a..ea3fdb395d000 100644 --- a/crates/oxc_mangler/src/lib.rs +++ b/crates/oxc_mangler/src/lib.rs @@ -570,10 +570,10 @@ impl<'t> Mangler<'t> { frequencies[index].frequency += scoping.get_resolved_reference_ids(symbol_id).len(); frequencies[index].symbol_ids.push(symbol_id); } + + // Remove slots that have no symbols to rename before sorting. + frequencies.retain(|x| !x.symbol_ids.is_empty()); frequencies.sort_unstable_by_key(|x| std::cmp::Reverse(x.frequency)); - if let Some(idx) = frequencies.iter().position(|x| x.symbol_ids.is_empty()) { - frequencies.truncate(idx); - } frequencies } diff --git a/crates/oxc_minifier/tests/mangler/snapshots/mangler.snap b/crates/oxc_minifier/tests/mangler/snapshots/mangler.snap index c2d76641fcb74..2d074732f3433 100644 --- a/crates/oxc_minifier/tests/mangler/snapshots/mangler.snap +++ b/crates/oxc_minifier/tests/mangler/snapshots/mangler.snap @@ -18,7 +18,7 @@ function foo(e) { var x; function foo(a) { ({ x } = y) } var x; -function foo(a) { +function foo(e) { ({x} = y); }