mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
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.
This commit is contained in:
1 parent
9a5e3e2b20
commit
ee9f6a4830
2 files changed
+4
-4
No files matched your search
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -18,7 +18,7 @@ function foo(e) {
|
||||
|
||||
var x; function foo(a) { ({ x } = y) }
|
||||
var x;
|
||||
function foo(a) {
|
||||
function foo(e) {
|
||||
({x} = y);
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user