From d64e815777c8d584196f4a3f649d2d8725cec44e Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Wed, 10 Dec 2025 14:24:21 -0800 Subject: [PATCH 1/6] Turbopack: Add `turbo-frozenmap` crate with `FrozenMap` and `FrozenSet` implementations --- Cargo.lock | 10 + Cargo.toml | 9 +- turbopack/crates/turbo-frozenmap/Cargo.toml | 15 + turbopack/crates/turbo-frozenmap/src/lib.rs | 6 + turbopack/crates/turbo-frozenmap/src/map.rs | 911 ++++++++++++++++++ turbopack/crates/turbo-frozenmap/src/set.rs | 448 +++++++++ turbopack/crates/turbo-tasks/Cargo.toml | 1 + .../crates/turbo-tasks/src/marker_trait.rs | 2 + .../crates/turbo-tasks/src/task/task_input.rs | 55 +- turbopack/crates/turbo-tasks/src/trace.rs | 18 + 10 files changed, 1468 insertions(+), 7 deletions(-) create mode 100644 turbopack/crates/turbo-frozenmap/Cargo.toml create mode 100644 turbopack/crates/turbo-frozenmap/src/lib.rs create mode 100644 turbopack/crates/turbo-frozenmap/src/map.rs create mode 100644 turbopack/crates/turbo-frozenmap/src/set.rs diff --git a/Cargo.lock b/Cargo.lock index 71dfff27ac0f..51703acf9817 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9130,6 +9130,15 @@ dependencies = [ "turbo-tasks", ] +[[package]] +name = "turbo-frozenmap" +version = "0.1.0" +dependencies = [ + "bincode 2.0.1", + "indexmap 2.9.0", + "serde", +] + [[package]] name = "turbo-persistence" version = "0.1.0" @@ -9242,6 +9251,7 @@ dependencies = [ "triomphe 0.1.12", "turbo-bincode", "turbo-dyn-eq-hash", + "turbo-frozenmap", "turbo-rcstr", "turbo-tasks-hash", "turbo-tasks-macros", diff --git a/Cargo.toml b/Cargo.toml index dd8404ebc1f8..d802ae61f761 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -294,13 +294,12 @@ next-taskless = { path = "crates/next-taskless" } # Turbopack auto-hash-map = { path = "turbopack/crates/turbo-tasks-auto-hash-map" } turbo-bincode = { path = "turbopack/crates/turbo-bincode" } -turbo-prehash = { path = "turbopack/crates/turbo-prehash" } -turbo-rcstr = { path = "turbopack/crates/turbo-rcstr" } turbo-dyn-eq-hash = { path = "turbopack/crates/turbo-dyn-eq-hash" } turbo-esregex = { path = "turbopack/crates/turbo-esregex" } +turbo-frozenmap = { path = "turbopack/crates/turbo-frozenmap" } turbo-persistence = { path = "turbopack/crates/turbo-persistence" } -turbo-unix-path = { path = "turbopack/crates/turbo-unix-path" } -turbo-tasks-malloc = { path = "turbopack/crates/turbo-tasks-malloc", default-features = false } +turbo-prehash = { path = "turbopack/crates/turbo-prehash" } +turbo-rcstr = { path = "turbopack/crates/turbo-rcstr" } turbo-tasks = { path = "turbopack/crates/turbo-tasks" } turbo-tasks-backend = { path = "turbopack/crates/turbo-tasks-backend" } turbo-tasks-bytes = { path = "turbopack/crates/turbo-tasks-bytes" } @@ -309,7 +308,9 @@ turbo-tasks-fetch = { path = "turbopack/crates/turbo-tasks-fetch" } turbo-tasks-fs = { path = "turbopack/crates/turbo-tasks-fs" } turbo-tasks-hash = { path = "turbopack/crates/turbo-tasks-hash" } turbo-tasks-macros = { path = "turbopack/crates/turbo-tasks-macros" } +turbo-tasks-malloc = { path = "turbopack/crates/turbo-tasks-malloc", default-features = false } turbo-tasks-testing = { path = "turbopack/crates/turbo-tasks-testing" } +turbo-unix-path = { path = "turbopack/crates/turbo-unix-path" } turbopack = { path = "turbopack/crates/turbopack" } turbopack-bench = { path = "turbopack/crates/turbopack-bench" } turbopack-nodejs = { path = "turbopack/crates/turbopack-nodejs" } diff --git a/turbopack/crates/turbo-frozenmap/Cargo.toml b/turbopack/crates/turbo-frozenmap/Cargo.toml new file mode 100644 index 000000000000..9691b90a4d2d --- /dev/null +++ b/turbopack/crates/turbo-frozenmap/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "turbo-frozenmap" +version = "0.1.0" +edition = "2024" +license = "MIT" + +[dependencies] +bincode = { workspace = true } +indexmap = { workspace = true } +serde = { workspace = true } + +[dev-dependencies] + +[lints] +workspace = true diff --git a/turbopack/crates/turbo-frozenmap/src/lib.rs b/turbopack/crates/turbo-frozenmap/src/lib.rs new file mode 100644 index 000000000000..89984e3cdee9 --- /dev/null +++ b/turbopack/crates/turbo-frozenmap/src/lib.rs @@ -0,0 +1,6 @@ +//! A frozen (immutable) ordered map and set implementation. + +pub mod map; +pub mod set; + +pub use crate::{map::FrozenMap, set::FrozenSet}; diff --git a/turbopack/crates/turbo-frozenmap/src/map.rs b/turbopack/crates/turbo-frozenmap/src/map.rs new file mode 100644 index 000000000000..d0cca014c25c --- /dev/null +++ b/turbopack/crates/turbo-frozenmap/src/map.rs @@ -0,0 +1,911 @@ +use std::{ + borrow::Borrow, + collections::{BTreeMap, HashMap}, + fmt::{self, Debug}, + hash::{BuildHasher, Hash}, + iter::FusedIterator, + ops::{Bound, Index, RangeBounds}, +}; + +use bincode::{BorrowDecode, Decode, Encode}; +use indexmap::IndexMap; +use serde::{Deserialize, Serialize}; + +/// A compact frozen (immutable) ordered map backed by a sorted boxed slice. +/// +/// This is a read-only map that stores key-value pairs in a contiguous, sorted array. It provides +/// efficient binary search lookups and iteration, but cannot be modified after construction. +#[derive( + Clone, Default, PartialEq, Eq, Hash, PartialOrd, Ord, Encode, Decode, Serialize, Deserialize, +)] +#[rustfmt::skip] // rustfmt breaks bincode's proc macro string processing +#[bincode( + decode_bounds = "K: Decode<__Context> + 'static, V: Decode<__Context> + 'static", + borrow_decode_bounds = "K: BorrowDecode<'__de, __Context> + '__de, V: BorrowDecode<'__de, __Context> + '__de" +)] +pub struct FrozenMap { + /// Invariant: entries are sorted by key in ascending order with no duplicates. + entries: Box<[(K, V)]>, +} + +impl FrozenMap { + /// Creates an empty `FrozenMap`. Does not perform any heap allocations. + pub fn new() -> Self { + FrozenMap { + // Box does not perform heap allocations for zero-sized types. + // In theory this could even be `const` using `Unique::dangling`, but there's no way to + // construct a `Box` from a pointer during `const`. + entries: Box::from([]), + } + } + + /// Creates a `FrozenMap` from a pre-sorted boxed slice with unique keys. This is a `const` + /// version of `From>`. + /// + /// # Correctness + /// + /// The caller must ensure that: + /// - The slice is sorted by key in ascending order according to [`K: Ord`][Ord] + /// - There are no duplicate keys + /// + /// If these invariants are not upheld, the map will behave incorrectly (e.g., `get` may fail to + /// find keys that are present), but no memory unsafety will occur. + pub const fn from_uniq_sorted_box(entries: Box<[(K, V)]>) -> Self { + FrozenMap { entries } + } + + /// Creates a `FrozenMap` from a pre-sorted slice with unique keys. This is equivalent to + /// `from_uniq_sorted_iter`, but more efficient, because it can be reduced to a simple + /// `memmove`. + /// + /// # Correctness + /// + /// The caller must ensure that: + /// - The slice is sorted by key in ascending order according to [`K: Ord`][Ord] + /// - There are no duplicate keys + /// + /// If these invariants are not upheld, the map will behave incorrectly (e.g., + /// [`FrozenMap::get`] may fail to find keys that are present), but no memory unsafety will + /// occur. + pub fn from_uniq_sorted_slice(slice: &[(K, V)]) -> Self + where + K: Clone, + V: Clone, + { + Self::from_uniq_sorted_box(Box::from(slice)) + } + + /// Creates a [`FrozenMap`] from a pre-sorted iterator with unique keys. + /// + /// # Correctness + /// + /// The caller must ensure that: + /// - The iterator yields elements sorted by key in ascending order according to `K: Ord` + /// - There are no duplicate keys + /// + /// If these invariants are not upheld, the map will behave incorrectly (e.g., + /// [`FrozenMap::get`] may fail to find keys that are present), but no memory unsafety will + /// occur. + pub fn from_uniq_sorted_iter(iter: impl IntoIterator) -> Self { + let entries: Box<[(K, V)]> = iter.into_iter().collect(); + Self::from_uniq_sorted_box(entries) + } + + /// Creates a [`FrozenMap`] from an unsorted iterator, sorting by key. This is more efficient + /// than [`FromIterator`] if you know that the iterator does not contain duplicate entries. + /// + /// # Correctness + /// + /// The caller must ensure that there are no duplicate keys. + /// + /// If this invariant is not upheld, the map will behave incorrectly (e.g., [`FrozenMap::get`] + /// may fail to find keys that are present), but no memory unsafety will occur. + pub fn from_uniq_iter(iter: impl IntoIterator) -> Self + where + K: Ord, + { + let entries: Box<[(K, V)]> = iter.into_iter().collect(); + if entries.is_empty() { + return Self::new(); + } + Self::from_uniq_box_inner(entries) + } + + /// Helper: skips `.is_empty` optimization, expects the caller to do that. + fn from_uniq_box_inner(mut entries: Box<[(K, V)]>) -> Self + where + K: Ord, + { + // Sort by key (stable sort preserves insertion order for equal keys) + entries.sort_by(|a, b| a.0.cmp(&b.0)); + Self::from_uniq_sorted_box(entries) + } + + /// Helper: skips `.is_empty` optimization, expects the caller to do that. + fn from_vec_inner(mut entries: Vec<(K, V)>) -> Self + where + K: Ord, + { + // Sort by key (stable sort preserves insertion order for equal keys) + entries.sort_by(|a, b| a.0.cmp(&b.0)); + + // Deduplicate, keeping the last value for each key. + // dedup_by removes the first argument when returning true, so we swap + // to keep the later (last) value in the earlier slot. + entries.dedup_by(|later, earlier| { + if later.0 == earlier.0 { + std::mem::swap(later, earlier); + true + } else { + false + } + }); + + Self::from_uniq_sorted_box(entries.into_boxed_slice()) + } +} + +impl FromIterator<(K, V)> for FrozenMap { + /// Creates a [`FrozenMap`] from an iterator of key-value pairs. + /// + /// If there are duplicate keys, the last value for each key is kept. + fn from_iter>(iter: T) -> Self { + let mut entries: Vec<(K, V)> = iter.into_iter().collect(); + if entries.is_empty() { + return Self::new(); + } + + // Sort by key (stable sort preserves insertion order for equal keys) + entries.sort_by(|a, b| a.0.cmp(&b.0)); + + // Deduplicate, keeping the last value for each key. + // dedup_by removes the first argument when returning true, so we swap + // to keep the later (last) value in the earlier slot. + entries.dedup_by(|later, earlier| { + if later.0 == earlier.0 { + std::mem::swap(later, earlier); + true + } else { + false + } + }); + + Self::from_uniq_sorted_box(entries.into_boxed_slice()) + } +} + +impl From> for FrozenMap { + /// Creates a [`FrozenMap`] from a [`BTreeMap`]. + /// + /// This is more efficient than `From>` because [`BTreeMap`] already iterates in + /// sorted order, so no re-sorting is needed. + fn from(map: BTreeMap) -> Self { + Self::from_uniq_sorted_iter(map) + } +} + +impl From> for FrozenMap +where + K: Ord, + S: BuildHasher, +{ + /// Creates a [`FrozenMap`] from a [`HashMap`]. + /// + /// The entries are sorted by key during construction. + fn from(map: HashMap) -> Self { + if map.is_empty() { + return Self::new(); + } + Self::from_uniq_box_inner(map.into_iter().collect()) + } +} + +impl From> for FrozenMap +where + K: Ord, + S: BuildHasher, +{ + /// Creates a [`FrozenMap`] from an [`IndexMap`]. + /// + /// The entries are sorted by key during construction. + fn from(map: IndexMap) -> Self { + if map.is_empty() { + return Self::new(); + } + Self::from_uniq_box_inner(map.into_iter().collect()) + } +} + +impl From> for FrozenMap { + /// Creates a [`FrozenMap`] from an array of key-value pairs. + /// + /// If there are duplicate keys, the last value for each key is kept. + fn from(entries: Vec<(K, V)>) -> Self { + if entries.is_empty() { + return Self::new(); + } + Self::from_vec_inner(entries) + } +} + +impl From> for FrozenMap { + /// Creates a [`FrozenMap`] from an array of key-value pairs. + /// + /// If there are duplicate keys, the last value for each key is kept. + fn from(entries: Box<[(K, V)]>) -> Self { + if entries.is_empty() { + return Self::new(); + } + Self::from_vec_inner(Vec::from(entries)) + } +} + +impl From<&[(K, V)]> for FrozenMap +where + K: Ord + Clone, + V: Clone, +{ + /// Creates a [`FrozenMap`] from a slice of key-value pairs. + /// + /// If there are duplicate keys, the last value for each key is kept. + fn from(entries: &[(K, V)]) -> Self { + if entries.is_empty() { + return Self::new(); + } + Self::from_vec_inner(Vec::from(entries)) + } +} + +impl From<[(K, V); N]> for FrozenMap { + /// Creates a [`FrozenMap`] from an array of key-value pairs. + /// + /// If there are duplicate keys, the last value for each key is kept. + fn from(entries: [(K, V); N]) -> Self { + if entries.is_empty() { + return Self::new(); + } + Self::from_vec_inner(Vec::from(entries)) + } +} + +impl FrozenMap { + /// Returns the number of elements in the map. + pub const fn len(&self) -> usize { + self.entries.len() + } + + /// Returns `true` if the map contains no elements. + pub const fn is_empty(&self) -> bool { + self.entries.is_empty() + } + + /// Returns a reference to the underlying sorted slice. + pub const fn as_slice(&self) -> &[(K, V)] { + &self.entries + } + + /// Returns a reference to the value corresponding to the key. + pub fn get(&self, key: &Q) -> Option<&V> + where + K: Borrow + Ord, + Q: Ord + ?Sized, + { + self.get_key_value(key).map(|(_, v)| v) + } + + /// Returns the key-value pair corresponding to the supplied key. + pub fn get_key_value(&self, key: &Q) -> Option<(&K, &V)> + where + K: Borrow + Ord, + Q: Ord + ?Sized, + { + let idx = self + .entries + .binary_search_by(|(k, _)| k.borrow().cmp(key)) + .ok()?; + let (k, v) = &self.entries[idx]; + Some((k, v)) + } + + /// Returns `true` if the map contains a value for the specified key. + pub fn contains_key(&self, key: &Q) -> bool + where + K: Borrow + Ord, + Q: Ord + ?Sized, + { + self.entries + .binary_search_by(|(k, _)| k.borrow().cmp(key)) + .is_ok() + } + + /// Returns the first key-value pair in the map. + pub fn first_key_value(&self) -> Option<(&K, &V)> { + self.entries.first().map(|(k, v)| (k, v)) + } + + /// Returns the last key-value pair in the map. + pub fn last_key_value(&self) -> Option<(&K, &V)> { + self.entries.last().map(|(k, v)| (k, v)) + } + + /// Gets an iterator over the entries of the map, sorted by key. + pub fn iter(&self) -> Iter<'_, K, V> { + Iter { + inner: self.entries.iter(), + } + } + + /// Gets an iterator over the keys of the map, in sorted order. + pub fn keys(&self) -> Keys<'_, K, V> { + Keys { inner: self.iter() } + } + + /// Gets an iterator over the values of the map, in order by key. + pub fn values(&self) -> Values<'_, K, V> { + Values { inner: self.iter() } + } + + /// Creates a consuming iterator visiting all the keys, in sorted order. + pub fn into_keys(self) -> IntoKeys { + IntoKeys { + inner: self.into_iter(), + } + } + + /// Creates a consuming iterator visiting all the values, in order by key. + pub fn into_values(self) -> IntoValues { + IntoValues { + inner: self.into_iter(), + } + } + + /// Constructs a double-ended iterator over a sub-range of elements in the map. + pub fn range(&self, range: R) -> Range<'_, K, V> + where + T: Ord + ?Sized, + K: Borrow + Ord, + R: RangeBounds, + { + let start = match range.start_bound() { + Bound::Included(key) => self + .entries + .binary_search_by(|(k, _)| k.borrow().cmp(key)) + .unwrap_or_else(|i| i), + Bound::Excluded(key) => { + match self.entries.binary_search_by(|(k, _)| k.borrow().cmp(key)) { + Ok(i) => i + 1, + Err(i) => i, + } + } + Bound::Unbounded => 0, + }; + + let end = match range.end_bound() { + Bound::Included(key) => { + match self.entries.binary_search_by(|(k, _)| k.borrow().cmp(key)) { + Ok(i) => i + 1, + Err(i) => i, + } + } + Bound::Excluded(key) => self + .entries + .binary_search_by(|(k, _)| k.borrow().cmp(key)) + .unwrap_or_else(|i| i), + Bound::Unbounded => self.entries.len(), + }; + + let slice = if start <= end && end <= self.entries.len() { + &self.entries[start..end] + } else { + &[] + }; + + Range { + inner: slice.iter(), + } + } +} + +impl Debug for FrozenMap { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_map().entries(self.iter()).finish() + } +} + +impl Index<&Q> for FrozenMap +where + K: Borrow + Ord, + Q: Ord, +{ + type Output = V; + + fn index(&self, key: &Q) -> &V { + self.get(key).expect("no entry found for key") + } +} + +impl AsRef<[(K, V)]> for FrozenMap { + fn as_ref(&self) -> &[(K, V)] { + self.as_slice() + } +} + +impl From> for Box<[(K, V)]> { + fn from(map: FrozenMap) -> Self { + map.entries + } +} + +impl<'a, K, V> IntoIterator for &'a FrozenMap { + type Item = (&'a K, &'a V); + type IntoIter = Iter<'a, K, V>; + + fn into_iter(self) -> Iter<'a, K, V> { + self.iter() + } +} + +impl IntoIterator for FrozenMap { + type Item = (K, V); + type IntoIter = IntoIter; + + fn into_iter(self) -> IntoIter { + IntoIter { + inner: self.entries.into_vec().into_iter(), + } + } +} + +/// An iterator over the entries of a [`FrozenMap`]. +pub struct Iter<'a, K, V> { + inner: std::slice::Iter<'a, (K, V)>, +} + +impl Debug for Iter<'_, K, V> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_list() + .entries(self.inner.clone().map(|(k, v)| (k, v))) + .finish() + } +} + +impl<'a, K, V> Iterator for Iter<'a, K, V> { + type Item = (&'a K, &'a V); + + fn next(&mut self) -> Option { + self.inner.next().map(|(k, v)| (k, v)) + } + + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() + } + + fn last(mut self) -> Option { + self.next_back() + } + + fn nth(&mut self, n: usize) -> Option { + self.inner.nth(n).map(|(k, v)| (k, v)) + } + + fn count(self) -> usize { + self.inner.len() + } +} + +impl DoubleEndedIterator for Iter<'_, K, V> { + fn next_back(&mut self) -> Option { + self.inner.next_back().map(|(k, v)| (k, v)) + } + + fn nth_back(&mut self, n: usize) -> Option { + self.inner.nth_back(n).map(|(k, v)| (k, v)) + } +} + +impl ExactSizeIterator for Iter<'_, K, V> { + fn len(&self) -> usize { + self.inner.len() + } +} + +impl FusedIterator for Iter<'_, K, V> {} + +// Manual implementation because the derive would add unnecessary `K: Clone, V: Clone` type bounds. +impl Clone for Iter<'_, K, V> { + fn clone(&self) -> Self { + Self { + inner: self.inner.clone(), + } + } +} + +/// An owning iterator over the entries of a `FrozenMap`. +pub struct IntoIter { + inner: std::vec::IntoIter<(K, V)>, +} + +impl Debug for IntoIter { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_list().entries(self.inner.as_slice()).finish() + } +} + +impl Iterator for IntoIter { + type Item = (K, V); + + fn next(&mut self) -> Option { + self.inner.next() + } + + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() + } + + fn count(self) -> usize { + self.inner.len() + } +} + +impl DoubleEndedIterator for IntoIter { + fn next_back(&mut self) -> Option { + self.inner.next_back() + } +} + +impl ExactSizeIterator for IntoIter { + fn len(&self) -> usize { + self.inner.len() + } +} + +impl FusedIterator for IntoIter {} + +/// An iterator over the keys of a `FrozenMap`. +pub struct Keys<'a, K, V> { + inner: Iter<'a, K, V>, +} + +impl Debug for Keys<'_, K, V> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_list() + .entries(self.inner.inner.clone().map(|(k, _)| k)) + .finish() + } +} + +impl<'a, K, V> Iterator for Keys<'a, K, V> { + type Item = &'a K; + + fn next(&mut self) -> Option { + self.inner.next().map(|(k, _)| k) + } + + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() + } + + fn last(mut self) -> Option { + self.next_back() + } + + fn count(self) -> usize { + self.inner.len() + } +} + +impl DoubleEndedIterator for Keys<'_, K, V> { + fn next_back(&mut self) -> Option { + self.inner.next_back().map(|(k, _)| k) + } +} + +impl ExactSizeIterator for Keys<'_, K, V> { + fn len(&self) -> usize { + self.inner.len() + } +} + +impl FusedIterator for Keys<'_, K, V> {} + +// Manual implementation because the derive would add an unnecessary `K: Clone, V: Clone` type +// bounds. +impl Clone for Keys<'_, K, V> { + fn clone(&self) -> Self { + Self { + inner: self.inner.clone(), + } + } +} + +/// An iterator over the values of a `FrozenMap`. +pub struct Values<'a, K, V> { + inner: Iter<'a, K, V>, +} + +impl Debug for Values<'_, K, V> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_list() + .entries(self.inner.inner.clone().map(|(_, v)| v)) + .finish() + } +} + +impl<'a, K, V> Iterator for Values<'a, K, V> { + type Item = &'a V; + + fn next(&mut self) -> Option { + self.inner.next().map(|(_, v)| v) + } + + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() + } + + fn last(mut self) -> Option { + self.next_back() + } + + fn count(self) -> usize { + self.inner.len() + } +} + +impl DoubleEndedIterator for Values<'_, K, V> { + fn next_back(&mut self) -> Option { + self.inner.next_back().map(|(_, v)| v) + } +} + +impl ExactSizeIterator for Values<'_, K, V> { + fn len(&self) -> usize { + self.inner.len() + } +} + +impl FusedIterator for Values<'_, K, V> {} + +// Manual implementation because the derive would add an unnecessary `K: Clone, V: Clone` type +// bounds. +impl Clone for Values<'_, K, V> { + fn clone(&self) -> Self { + Self { + inner: self.inner.clone(), + } + } +} + +/// An owning iterator over the keys of a `FrozenMap`. +pub struct IntoKeys { + inner: IntoIter, +} + +impl Debug for IntoKeys { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_list() + .entries(self.inner.inner.as_slice().iter().map(|(k, _)| k)) + .finish() + } +} + +impl Iterator for IntoKeys { + type Item = K; + + fn next(&mut self) -> Option { + self.inner.next().map(|(k, _)| k) + } + + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() + } + + fn count(self) -> usize { + self.inner.len() + } +} + +impl DoubleEndedIterator for IntoKeys { + fn next_back(&mut self) -> Option { + self.inner.next_back().map(|(k, _)| k) + } +} + +impl ExactSizeIterator for IntoKeys { + fn len(&self) -> usize { + self.inner.len() + } +} + +impl FusedIterator for IntoKeys {} + +/// An owning iterator over the values of a `FrozenMap`. +pub struct IntoValues { + inner: IntoIter, +} + +impl Debug for IntoValues { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_list() + .entries(self.inner.inner.as_slice().iter().map(|(_, v)| v)) + .finish() + } +} + +impl Iterator for IntoValues { + type Item = V; + + fn next(&mut self) -> Option { + self.inner.next().map(|(_, v)| v) + } + + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() + } + + fn count(self) -> usize { + self.inner.len() + } +} + +impl DoubleEndedIterator for IntoValues { + fn next_back(&mut self) -> Option { + self.inner.next_back().map(|(_, v)| v) + } +} + +impl ExactSizeIterator for IntoValues { + fn len(&self) -> usize { + self.inner.len() + } +} + +impl FusedIterator for IntoValues {} + +/// An iterator over a sub-range of entries in a `FrozenMap`. +pub struct Range<'a, K, V> { + inner: std::slice::Iter<'a, (K, V)>, +} + +impl Debug for Range<'_, K, V> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_list().entries(self.clone()).finish() + } +} + +impl<'a, K, V> Iterator for Range<'a, K, V> { + type Item = (&'a K, &'a V); + + fn next(&mut self) -> Option { + self.inner.next().map(|(k, v)| (k, v)) + } + + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() + } + + fn last(mut self) -> Option { + self.next_back() + } + + fn count(self) -> usize { + self.inner.len() + } +} + +impl DoubleEndedIterator for Range<'_, K, V> { + fn next_back(&mut self) -> Option { + self.inner.next_back().map(|(k, v)| (k, v)) + } +} + +impl ExactSizeIterator for Range<'_, K, V> { + fn len(&self) -> usize { + self.inner.len() + } +} + +impl FusedIterator for Range<'_, K, V> {} + +// Manual implementation because the derive would add unnecessary `K: Clone, V: Clone` type bounds. +impl Clone for Range<'_, K, V> { + fn clone(&self) -> Self { + Self { + inner: self.inner.clone(), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_empty() { + let map = FrozenMap::::new(); + assert!(map.is_empty()); + assert_eq!(map.len(), 0); + assert_eq!(map.get(&1), None); + } + + #[test] + fn test_from_btreemap() { + let mut btree = BTreeMap::new(); + btree.insert(3, "c"); + btree.insert(1, "a"); + btree.insert(2, "b"); + + let frozen = FrozenMap::from(btree); + assert_eq!(frozen.len(), 3); + assert_eq!(frozen.get(&1), Some(&"a")); + assert_eq!(frozen.get(&2), Some(&"b")); + assert_eq!(frozen.get(&3), Some(&"c")); + + let keys: Vec<_> = frozen.keys().copied().collect(); + assert_eq!(keys, vec![1, 2, 3]); + } + + #[test] + fn test_from_array() { + let frozen = FrozenMap::from([(3, "c"), (1, "a"), (2, "b")]); + assert_eq!(frozen.len(), 3); + assert_eq!(frozen.get(&1), Some(&"a")); + + let keys: Vec<_> = frozen.keys().copied().collect(); + assert_eq!(keys, vec![1, 2, 3]); + } + + #[test] + fn test_from_iter_with_duplicates() { + let frozen: FrozenMap<_, _> = [(1, "a"), (1, "b"), (2, "c")].into_iter().collect(); + assert_eq!(frozen.len(), 2); + // Last value wins for duplicates + assert_eq!(frozen.get(&1), Some(&"b")); + assert_eq!(frozen.get(&2), Some(&"c")); + } + + #[test] + fn test_range() { + let frozen = FrozenMap::from([(1, "a"), (2, "b"), (3, "c"), (4, "d"), (5, "e")]); + + let range: Vec<_> = frozen.range(2..4).collect(); + assert_eq!(range, vec![(&2, &"b"), (&3, &"c")]); + + let range: Vec<_> = frozen.range(2..=4).collect(); + assert_eq!(range, vec![(&2, &"b"), (&3, &"c"), (&4, &"d")]); + + let range: Vec<_> = frozen.range(..3).collect(); + assert_eq!(range, vec![(&1, &"a"), (&2, &"b")]); + } + + #[test] + fn test_index() { + let frozen = FrozenMap::from([(1, "a"), (2, "b")]); + assert_eq!(frozen[&1], "a"); + assert_eq!(frozen[&2], "b"); + } + + #[test] + #[should_panic(expected = "no entry found for key")] + fn test_index_missing() { + let frozen = FrozenMap::from([(1, "a")]); + let _ = frozen[&2]; + } + + #[test] + fn test_first_last() { + let frozen = FrozenMap::from([(2, "b"), (1, "a"), (3, "c")]); + assert_eq!(frozen.first_key_value(), Some((&1, &"a"))); + assert_eq!(frozen.last_key_value(), Some((&3, &"c"))); + + let empty = FrozenMap::::new(); + assert_eq!(empty.first_key_value(), None); + assert_eq!(empty.last_key_value(), None); + } + + #[test] + fn test_as_ref() { + let frozen = FrozenMap::from([(2, "b"), (1, "a"), (3, "c")]); + let slice: &[(i32, &str)] = frozen.as_ref(); + assert_eq!(slice, &[(1, "a"), (2, "b"), (3, "c")]); + } +} diff --git a/turbopack/crates/turbo-frozenmap/src/set.rs b/turbopack/crates/turbo-frozenmap/src/set.rs new file mode 100644 index 000000000000..dcd1c6373e84 --- /dev/null +++ b/turbopack/crates/turbo-frozenmap/src/set.rs @@ -0,0 +1,448 @@ +use std::{ + borrow::Borrow, + collections::{BTreeSet, HashSet}, + fmt::{self, Debug}, + hash::{BuildHasher, Hash}, + iter::FusedIterator, + ops::RangeBounds, +}; + +use bincode::{BorrowDecode, Decode, Encode}; +use indexmap::IndexSet; +use serde::{Deserialize, Serialize}; + +use crate::map::{self, FrozenMap}; + +/// A compact frozen (immutable) ordered set backed by a [`FrozenMap`]. +/// +/// This is a read-only set that stores elements in a contiguous, sorted array. It provides +/// efficient binary search lookups and iteration, but cannot be modified after construction. +#[derive( + Clone, Default, PartialEq, Eq, Hash, PartialOrd, Ord, Encode, Decode, Serialize, Deserialize, +)] +#[rustfmt::skip] +#[bincode( + decode_bounds = "T: Decode<__Context> + 'static", + borrow_decode_bounds = "T: BorrowDecode<'__de, __Context> + '__de" +)] +pub struct FrozenSet { + map: FrozenMap, +} + +impl FrozenSet { + /// Creates an empty `FrozenSet`. Does not perform any heap allocations. + pub fn new() -> Self { + FrozenSet { + map: FrozenMap::new(), + } + } + + /// Creates a [`FrozenSet`] from a pre-sorted iterator with unique elements. + /// + /// # Correctness + /// + /// The caller must ensure that: + /// - The iterator yields elements sorted in ascending order according to [`T: Ord`][Ord] + /// - There are no duplicate elements + /// + /// If these invariants are not upheld, the set will behave incorrectly (e.g., + /// [`FrozenSet::contains`] may fail to find elements that are present), but no memory + /// unsafety will occur. + pub fn from_uniq_sorted_iter(iter: impl IntoIterator) -> Self { + FrozenSet { + map: FrozenMap::from_uniq_sorted_iter(iter.into_iter().map(|t| (t, ()))), + } + } + + /// Creates a [`FrozenSet`] from an unsorted iterator, sorting by element. This is more + /// efficient than [`FromIterator`] if you know that the iterator does not contain duplicate + /// entries. + /// + /// # Correctness + /// + /// The caller must ensure that there are no duplicate elements. + /// + /// If this invariant is not upheld, the set will behave incorrectly (e.g., + /// [`FrozenSet::contains`] may fail to find elements that are present), but no memory unsafety + /// will occur. + pub fn from_uniq_iter(iter: impl IntoIterator) -> Self + where + T: Ord, + { + FrozenSet { + map: FrozenMap::from_uniq_iter(iter.into_iter().map(|t| (t, ()))), + } + } +} + +impl FromIterator for FrozenSet { + /// Creates a [`FrozenSet`] from an iterator of elements. + /// + /// If there are duplicate elements, only one copy is kept (the last one encountered). + fn from_iter>(iter: I) -> Self { + FrozenSet { + map: FrozenMap::from_iter(iter.into_iter().map(|t| (t, ()))), + } + } +} + +impl From> for FrozenSet { + /// Creates a [`FrozenSet`] from a [`BTreeSet`]. + /// + /// This is more efficient than [`From>`] because [`BTreeSet`] already iterates in + /// sorted order, so no re-sorting is needed. + fn from(set: BTreeSet) -> Self { + FrozenSet::from_uniq_sorted_iter(set) + } +} + +impl From> for FrozenSet +where + T: Ord, + S: BuildHasher, +{ + /// Creates a [`FrozenSet`] from a [`HashSet`]. + /// + /// The elements are sorted during construction. + fn from(set: HashSet) -> Self { + FrozenSet { + map: FrozenMap::from_uniq_iter(set.into_iter().map(|t| (t, ()))), + } + } +} + +impl From> for FrozenSet +where + T: Ord, + S: BuildHasher, +{ + /// Creates a [`FrozenSet`] from an [`IndexSet`]. + /// + /// The elements are sorted during construction. + fn from(set: IndexSet) -> Self { + FrozenSet { + map: FrozenMap::from_uniq_iter(set.into_iter().map(|t| (t, ()))), + } + } +} + +impl From> for FrozenSet { + /// Creates a [`FrozenSet`] from a vector of elements. + /// + /// If there are duplicate elements, the last copy is kept. + fn from(elements: Vec) -> Self { + Self::from_iter(elements) + } +} + +impl From> for FrozenSet { + /// Creates a [`FrozenSet`] from a boxed slice of elements. + /// + /// If there are duplicate elements, the last copy is kept. + fn from(elements: Box<[T]>) -> Self { + Self::from_iter(elements) + } +} + +impl From<&[T]> for FrozenSet +where + T: Ord + Clone, +{ + /// Creates a [`FrozenSet`] from a slice of elements. + /// + /// If there are duplicate elements, the last copy is kept. + fn from(elements: &[T]) -> Self { + FrozenSet::from(Vec::from(elements)) + } +} + +impl From<[T; N]> for FrozenSet { + /// Creates a [`FrozenSet`] from an array of elements. + /// + /// If there are duplicate elements, the last copy is kept. + fn from(elements: [T; N]) -> Self { + Self::from_iter(elements) + } +} + +impl FrozenSet { + /// Returns the number of elements in the set. + pub const fn len(&self) -> usize { + self.map.len() + } + + /// Returns `true` if the set contains no elements. + pub const fn is_empty(&self) -> bool { + self.map.is_empty() + } + + /// Returns `true` if the set contains an element equal to the value. + pub fn contains(&self, value: &Q) -> bool + where + T: Borrow + Ord, + Q: Ord + ?Sized, + { + self.map.contains_key(value) + } + + /// Returns a reference to the element in the set, if any, that is equal to the value. + pub fn get(&self, value: &Q) -> Option<&T> + where + T: Borrow + Ord, + Q: Ord + ?Sized, + { + self.map.get_key_value(value).map(|(t, _)| t) + } + + /// Returns a reference to the first element in the set, if any. + /// This element is always the minimum of all elements in the set. + pub fn first(&self) -> Option<&T> + where + T: Ord, + { + self.map.first_key_value().map(|(t, _)| t) + } + + /// Returns a reference to the last element in the set, if any. + /// This element is always the maximum of all elements in the set. + pub fn last(&self) -> Option<&T> + where + T: Ord, + { + self.map.last_key_value().map(|(t, _)| t) + } + + /// Gets an iterator that visits the elements in the `FrozenSet` in ascending order. + pub fn iter(&self) -> Iter<'_, T> { + self.map.keys() + } + + /// Constructs a double-ended iterator over a sub-range of elements in the set. + pub fn range(&self, range: R) -> Range<'_, T> + where + Q: Ord + ?Sized, + T: Borrow + Ord, + R: RangeBounds, + { + Range { + inner: self.map.range(range), + } + } + + /// Returns `true` if `self` has no elements in common with `other`. + /// This is equivalent to checking for an empty intersection. + pub fn is_disjoint(&self, other: &Self) -> bool + where + T: Ord, + { + if self.len() <= other.len() { + self.iter().all(|v| !other.contains(v)) + } else { + other.iter().all(|v| !self.contains(v)) + } + } + + /// Returns `true` if the set is a subset of another, + /// i.e., `other` contains at least all the elements in `self`. + pub fn is_subset(&self, other: &Self) -> bool + where + T: Ord, + { + if self.len() > other.len() { + return false; + } + self.iter().all(|v| other.contains(v)) + } + + /// Returns `true` if the set is a superset of another, + /// i.e., `self` contains at least all the elements in `other`. + pub fn is_superset(&self, other: &Self) -> bool + where + T: Ord, + { + other.is_subset(self) + } +} + +impl Debug for FrozenSet { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_set().entries(self.iter()).finish() + } +} + +impl<'a, T> IntoIterator for &'a FrozenSet { + type Item = &'a T; + type IntoIter = Iter<'a, T>; + + fn into_iter(self) -> Iter<'a, T> { + self.iter() + } +} + +impl IntoIterator for FrozenSet { + type Item = T; + type IntoIter = IntoIter; + + fn into_iter(self) -> IntoIter { + self.map.into_keys() + } +} + +// These could be newtype wrappers (BTreeSet does this), but type aliases are simpler to implement. +type Iter<'a, T> = map::Keys<'a, T, ()>; +type IntoIter = map::IntoKeys; + +/// An iterator over a sub-range of elements in a [`FrozenSet`]. +pub struct Range<'a, T> { + inner: map::Range<'a, T, ()>, +} + +impl Debug for Range<'_, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_list().entries(self.clone()).finish() + } +} + +impl<'a, T> Iterator for Range<'a, T> { + type Item = &'a T; + + fn next(&mut self) -> Option<&'a T> { + self.inner.next().map(|(t, _)| t) + } + + fn last(self) -> Option<&'a T> { + self.inner.last().map(|(t, _)| t) + } + + fn count(self) -> usize { + self.inner.len() + } +} + +impl<'a, T> DoubleEndedIterator for Range<'a, T> { + fn next_back(&mut self) -> Option<&'a T> { + self.inner.next_back().map(|(t, _)| t) + } +} + +impl ExactSizeIterator for Range<'_, T> { + fn len(&self) -> usize { + self.inner.len() + } +} + +impl FusedIterator for Range<'_, T> {} + +// Manual implementation because the derive would add an unnecessary `T: Clone` type bounds. +impl Clone for Range<'_, T> { + fn clone(&self) -> Self { + Self { + inner: self.inner.clone(), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_empty() { + let set = FrozenSet::::new(); + assert!(set.is_empty()); + assert_eq!(set.len(), 0); + assert!(!set.contains(&1)); + } + + #[test] + fn test_from_btreeset() { + let mut btree = BTreeSet::new(); + btree.insert(3); + btree.insert(1); + btree.insert(2); + + let frozen = FrozenSet::from(btree); + assert_eq!(frozen.len(), 3); + assert!(frozen.contains(&1)); + assert!(frozen.contains(&2)); + assert!(frozen.contains(&3)); + + let elements: Vec<_> = frozen.iter().copied().collect(); + assert_eq!(elements, vec![1, 2, 3]); + } + + #[test] + fn test_from_array() { + let frozen = FrozenSet::from([3, 1, 2]); + assert_eq!(frozen.len(), 3); + assert!(frozen.contains(&1)); + + let elements: Vec<_> = frozen.iter().copied().collect(); + assert_eq!(elements, vec![1, 2, 3]); + } + + #[test] + fn test_from_iter_with_duplicates() { + let frozen: FrozenSet<_> = [1, 1, 2].into_iter().collect(); + assert_eq!(frozen.len(), 2); + assert!(frozen.contains(&1)); + assert!(frozen.contains(&2)); + } + + #[test] + fn test_range() { + let frozen = FrozenSet::from([1, 2, 3, 4, 5]); + + let range: Vec<_> = frozen.range(2..4).copied().collect(); + assert_eq!(range, vec![2, 3]); + + let range: Vec<_> = frozen.range(2..=4).copied().collect(); + assert_eq!(range, vec![2, 3, 4]); + + let range: Vec<_> = frozen.range(..3).copied().collect(); + assert_eq!(range, vec![1, 2]); + } + + #[test] + fn test_first_last() { + let frozen = FrozenSet::from([2, 1, 3]); + assert_eq!(frozen.first(), Some(&1)); + assert_eq!(frozen.last(), Some(&3)); + + let empty = FrozenSet::::new(); + assert_eq!(empty.first(), None); + assert_eq!(empty.last(), None); + } + + #[test] + fn test_is_disjoint() { + let a = FrozenSet::from([1, 2, 3]); + let b = FrozenSet::from([4, 5, 6]); + let c = FrozenSet::from([3, 4, 5]); + + assert!(a.is_disjoint(&b)); + assert!(!a.is_disjoint(&c)); + } + + #[test] + fn test_is_subset() { + let a = FrozenSet::from([1, 2]); + let b = FrozenSet::from([1, 2, 3]); + let c = FrozenSet::from([2, 3, 4]); + + assert!(a.is_subset(&b)); + assert!(!a.is_subset(&c)); + assert!(a.is_subset(&a)); + } + + #[test] + fn test_is_superset() { + let a = FrozenSet::from([1, 2, 3]); + let b = FrozenSet::from([1, 2]); + let c = FrozenSet::from([2, 3, 4]); + + assert!(a.is_superset(&b)); + assert!(!a.is_superset(&c)); + assert!(a.is_superset(&a)); + } +} diff --git a/turbopack/crates/turbo-tasks/Cargo.toml b/turbopack/crates/turbo-tasks/Cargo.toml index 5697a6b28b57..b7697bbb63b9 100644 --- a/turbopack/crates/turbo-tasks/Cargo.toml +++ b/turbopack/crates/turbo-tasks/Cargo.toml @@ -51,6 +51,7 @@ tracing = { workspace = true } triomphe = { workspace = true, features = ["unsize", "unstable"] } turbo-bincode = { workspace = true } turbo-dyn-eq-hash = { workspace = true } +turbo-frozenmap = { workspace = true } turbo-rcstr = { workspace = true } turbo-tasks-hash = { workspace = true } turbo-tasks-macros = { workspace = true } diff --git a/turbopack/crates/turbo-tasks/src/marker_trait.rs b/turbopack/crates/turbo-tasks/src/marker_trait.rs index dbcad957123f..bffb8fd08c38 100644 --- a/turbopack/crates/turbo-tasks/src/marker_trait.rs +++ b/turbopack/crates/turbo-tasks/src/marker_trait.rs @@ -55,11 +55,13 @@ macro_rules! impl_auto_marker_trait { unsafe impl $trait for ::auto_hash_map::AutoSet {} unsafe impl $trait for ::std::collections::BTreeSet {} unsafe impl $trait for ::indexmap::IndexSet {} + unsafe impl $trait for ::turbo_frozenmap::FrozenSet {} unsafe impl $trait for ::std::collections::HashMap {} unsafe impl $trait for ::auto_hash_map::AutoMap {} unsafe impl $trait for ::std::collections::BTreeMap {} unsafe impl $trait for ::indexmap::IndexMap {} + unsafe impl $trait for ::turbo_frozenmap::FrozenMap {} unsafe impl $trait for ::std::boxed::Box {} unsafe impl $trait for ::std::sync::Arc {} unsafe impl $trait diff --git a/turbopack/crates/turbo-tasks/src/task/task_input.rs b/turbopack/crates/turbo-tasks/src/task/task_input.rs index 1a466303d5ab..3428472616c2 100644 --- a/turbopack/crates/turbo-tasks/src/task/task_input.rs +++ b/turbopack/crates/turbo-tasks/src/task/task_input.rs @@ -16,6 +16,7 @@ use bincode::{ error::{DecodeError, EncodeError}, }; use either::Either; +use turbo_frozenmap::{FrozenMap, FrozenSet}; use turbo_rcstr::RcStr; // This import is necessary for derive macros to work, as their expansion refers to the crate @@ -284,11 +285,59 @@ where T: TaskInput + Ord, { async fn resolve_input(&self) -> Result { - let mut new_map = BTreeSet::new(); + let mut new_set = BTreeSet::new(); for value in self { - new_map.insert(TaskInput::resolve_input(value).await?); + new_set.insert(TaskInput::resolve_input(value).await?); } - Ok(new_map) + Ok(new_set) + } + + fn is_resolved(&self) -> bool { + self.iter().all(TaskInput::is_resolved) + } + + fn is_transient(&self) -> bool { + self.iter().any(TaskInput::is_transient) + } +} + +impl TaskInput for FrozenMap +where + K: TaskInput + Ord + 'static, + V: TaskInput + 'static, +{ + async fn resolve_input(&self) -> Result { + let mut new_entries = Vec::new(); + for (k, v) in self { + new_entries.push(( + TaskInput::resolve_input(k).await?, + TaskInput::resolve_input(v).await?, + )); + } + Ok(Self::from(new_entries)) + } + + fn is_resolved(&self) -> bool { + self.iter() + .all(|(k, v)| TaskInput::is_resolved(k) && TaskInput::is_resolved(v)) + } + + fn is_transient(&self) -> bool { + self.iter() + .any(|(k, v)| TaskInput::is_transient(k) || TaskInput::is_transient(v)) + } +} + +impl TaskInput for FrozenSet +where + T: TaskInput + Ord + 'static, +{ + async fn resolve_input(&self) -> Result { + let mut new_set = Vec::new(); + for value in self { + new_set.push(TaskInput::resolve_input(value).await?); + } + Ok(Self::from(new_set)) } fn is_resolved(&self) -> bool { diff --git a/turbopack/crates/turbo-tasks/src/trace.rs b/turbopack/crates/turbo-tasks/src/trace.rs index 83ae57e1e597..e0bc148ef58b 100644 --- a/turbopack/crates/turbo-tasks/src/trace.rs +++ b/turbopack/crates/turbo-tasks/src/trace.rs @@ -12,6 +12,7 @@ use auto_hash_map::{AutoMap, AutoSet}; use either::Either; use indexmap::{IndexMap, IndexSet}; use smallvec::SmallVec; +use turbo_frozenmap::{FrozenMap, FrozenSet}; use turbo_rcstr::RcStr; use crate::RawVc; @@ -200,6 +201,14 @@ impl TraceRawVcs for IndexSet { } } +impl TraceRawVcs for FrozenSet { + fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) { + for item in self.iter() { + TraceRawVcs::trace_raw_vcs(item, trace_context); + } + } +} + impl TraceRawVcs for HashMap { fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) { for (key, value) in self.iter() { @@ -236,6 +245,15 @@ impl TraceRawVcs for IndexMap { } } +impl TraceRawVcs for FrozenMap { + fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) { + for (key, value) in self.iter() { + TraceRawVcs::trace_raw_vcs(key, trace_context); + TraceRawVcs::trace_raw_vcs(value, trace_context); + } + } +} + impl TraceRawVcs for Box { fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) { TraceRawVcs::trace_raw_vcs(&**self, trace_context); From ba470e9adbdf65ff70d8f6681c3874cd06ab1cbe Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Wed, 10 Dec 2025 20:14:24 -0800 Subject: [PATCH 2/6] re-do constructors to require explicitness when overlapping in a map is desired --- turbopack/crates/turbo-frozenmap/src/map.rs | 399 ++++++++++++------ turbopack/crates/turbo-frozenmap/src/set.rs | 215 +++++++--- .../crates/turbo-tasks/src/task/task_input.rs | 7 +- 3 files changed, 417 insertions(+), 204 deletions(-) diff --git a/turbopack/crates/turbo-frozenmap/src/map.rs b/turbopack/crates/turbo-frozenmap/src/map.rs index d0cca014c25c..50df6ef5be34 100644 --- a/turbopack/crates/turbo-frozenmap/src/map.rs +++ b/turbopack/crates/turbo-frozenmap/src/map.rs @@ -2,7 +2,7 @@ use std::{ borrow::Borrow, collections::{BTreeMap, HashMap}, fmt::{self, Debug}, - hash::{BuildHasher, Hash}, + hash::BuildHasher, iter::FusedIterator, ops::{Bound, Index, RangeBounds}, }; @@ -14,7 +14,25 @@ use serde::{Deserialize, Serialize}; /// A compact frozen (immutable) ordered map backed by a sorted boxed slice. /// /// This is a read-only map that stores key-value pairs in a contiguous, sorted array. It provides -/// efficient binary search lookups and iteration, but cannot be modified after construction. +/// efficient sorted iteration and binary search lookups, but cannot be modified after construction. +/// +/// # Construction +/// +/// If you're building a new map, and you don't expect many overlapping keys, consider pushing +/// elements into a [`Vec`] and calling [`FrozenMap::from_unique_vec`] or +/// [`FrozenMap::from_overlapping_vec`]. It is typically cheaper to collect into a [`Vec`] and sort +/// the entries once at the end than it is to maintain a temporary map data structure. +/// +/// If you already have a map, or you have many overlapping keys that you don't want to temporarily +/// hold onto, you can use the [`From`] or [`Into`] traits to create a [`FrozenMap`] from one of +/// many common collections. You should prefer using a [`BTreeMap`], as it matches the sorted +/// semantics of [`FrozenMap`] and avoids a sort operation during conversion. +/// +/// [`FromIterator`] and `From>` trait implementations are intentionally not provided, +/// because the desired behavior around overlapping keys is potentially ambiguous. +/// +/// There are a variety of constructors provided that can be more efficient if you know that your +/// data is sorted and/or unique. #[derive( Clone, Default, PartialEq, Eq, Hash, PartialOrd, Ord, Encode, Decode, Serialize, Deserialize, )] @@ -24,12 +42,12 @@ use serde::{Deserialize, Serialize}; borrow_decode_bounds = "K: BorrowDecode<'__de, __Context> + '__de, V: BorrowDecode<'__de, __Context> + '__de" )] pub struct FrozenMap { - /// Invariant: entries are sorted by key in ascending order with no duplicates. + /// Invariant: entries are sorted by key in ascending order with no overlapping keys. entries: Box<[(K, V)]>, } impl FrozenMap { - /// Creates an empty `FrozenMap`. Does not perform any heap allocations. + /// Creates an empty [`FrozenMap`]. Does not perform any heap allocations. pub fn new() -> Self { FrozenMap { // Box does not perform heap allocations for zero-sized types. @@ -39,40 +57,80 @@ impl FrozenMap { } } - /// Creates a `FrozenMap` from a pre-sorted boxed slice with unique keys. This is a `const` - /// version of `From>`. + /// Creates a [`FrozenMap`] from a pre-sorted boxed slice with unique keys. + /// + /// Panics if the keys in `entries` are not unique and sorted. + pub fn from_unique_sorted_box(entries: Box<[(K, V)]>) -> Self + where + K: Ord, + { + assert_unique_sorted(&entries); + Self::from_unique_sorted_box_unchecked(entries) + } + + /// Creates a [`FrozenMap`] from a pre-sorted boxed slice with unique keys. /// /// # Correctness /// /// The caller must ensure that: - /// - The slice is sorted by key in ascending order according to [`K: Ord`][Ord] - /// - There are no duplicate keys + /// - The entries are sorted by key in ascending order according to [`K: Ord`][Ord] + /// - There are no overlapping keys /// - /// If these invariants are not upheld, the map will behave incorrectly (e.g., `get` may fail to - /// find keys that are present), but no memory unsafety will occur. - pub const fn from_uniq_sorted_box(entries: Box<[(K, V)]>) -> Self { + /// If these invariants are not upheld, the map will behave incorrectly (e.g., + /// [`FrozenMap::get`] may fail to find keys that are present), but no memory unsafety will + /// occur. + pub const fn from_unique_sorted_box_unchecked(entries: Box<[(K, V)]>) -> Self { FrozenMap { entries } } - /// Creates a `FrozenMap` from a pre-sorted slice with unique keys. This is equivalent to - /// `from_uniq_sorted_iter`, but more efficient, because it can be reduced to a simple - /// `memmove`. + /// Creates a [`FrozenMap`] from a pre-sorted slice with unique keys. + /// + /// This may be more efficient than [`FrozenMap::from_unique_sorted_iter`] because it + /// may be reduced to a simple `memmove` for types implementing [`Copy`], plus an ordering + /// check. + /// + /// Panics if the keys in `entries` are not unique and sorted. + pub fn from_unique_sorted_slice(entries: &[(K, V)]) -> Self + where + K: Clone + Ord, + V: Clone, + { + assert_unique_sorted(entries); + Self::from_unique_sorted_slice_unchecked(entries) + } + + /// Creates a [`FrozenMap`] from a pre-sorted slice with unique keys. + /// + /// This may be more efficient than [`FrozenMap::from_unique_sorted_iter_unchecked`] because it + /// may be reduced to a simple `memmove` for types implementing [`Copy`]. /// /// # Correctness /// /// The caller must ensure that: - /// - The slice is sorted by key in ascending order according to [`K: Ord`][Ord] - /// - There are no duplicate keys + /// - The entries are sorted by key in ascending order according to [`K: Ord`][Ord] + /// - There are no overlapping keys /// /// If these invariants are not upheld, the map will behave incorrectly (e.g., /// [`FrozenMap::get`] may fail to find keys that are present), but no memory unsafety will /// occur. - pub fn from_uniq_sorted_slice(slice: &[(K, V)]) -> Self + pub fn from_unique_sorted_slice_unchecked(entries: &[(K, V)]) -> Self where K: Clone, V: Clone, { - Self::from_uniq_sorted_box(Box::from(slice)) + Self::from_unique_sorted_box_unchecked(Box::from(entries)) + } + + /// Creates a [`FrozenMap`] from an iterator that yields sorted unique keys. + /// + /// Panics if the keys in `entries` are not unique and sorted. + pub fn from_unique_sorted_iter(entries: impl IntoIterator) -> Self + where + K: Ord, + { + let this = Self::from_unique_sorted_iter_unchecked(entries); + assert_unique_sorted(&this.entries); + this } /// Creates a [`FrozenMap`] from a pre-sorted iterator with unique keys. @@ -80,56 +138,113 @@ impl FrozenMap { /// # Correctness /// /// The caller must ensure that: - /// - The iterator yields elements sorted by key in ascending order according to `K: Ord` - /// - There are no duplicate keys + /// - The iterator yields elements sorted by key in ascending order according to [`K: Ord`][Ord] + /// - There are no overlapping keys /// /// If these invariants are not upheld, the map will behave incorrectly (e.g., /// [`FrozenMap::get`] may fail to find keys that are present), but no memory unsafety will /// occur. - pub fn from_uniq_sorted_iter(iter: impl IntoIterator) -> Self { - let entries: Box<[(K, V)]> = iter.into_iter().collect(); - Self::from_uniq_sorted_box(entries) + pub fn from_unique_sorted_iter_unchecked(iter: impl IntoIterator) -> Self { + Self::from_unique_sorted_box_unchecked(iter.into_iter().collect()) } - /// Creates a [`FrozenMap`] from an unsorted iterator, sorting by key. This is more efficient - /// than [`FromIterator`] if you know that the iterator does not contain duplicate entries. + /// Creates a [`FrozenMap`] from an unsorted iterator of unique keys. Entries are sorted by key. + /// + /// This is more efficient than [`FrozenMap::from_overlapping_iter`] if you know the keys are + /// unique. + /// + /// Panics if any of the keys in `entries` are overlapping. + pub fn from_unique_iter(entries: impl IntoIterator) -> Self + where + K: Ord, + { + let this = Self::from_unique_iter_unchecked(entries); + assert_unique(&this.entries); + this + } + + /// Creates a [`FrozenMap`] from an unsorted iterator of unique keys. Entries are sorted by key. /// /// # Correctness /// - /// The caller must ensure that there are no duplicate keys. + /// The caller must ensure that there are no overlapping keys. /// /// If this invariant is not upheld, the map will behave incorrectly (e.g., [`FrozenMap::get`] /// may fail to find keys that are present), but no memory unsafety will occur. - pub fn from_uniq_iter(iter: impl IntoIterator) -> Self + pub fn from_unique_iter_unchecked(entries: impl IntoIterator) -> Self where K: Ord, { - let entries: Box<[(K, V)]> = iter.into_iter().collect(); + let entries: Box<[(K, V)]> = entries.into_iter().collect(); if entries.is_empty() { return Self::new(); } - Self::from_uniq_box_inner(entries) + Self::from_unique_box_unchecked(entries) } - /// Helper: skips `.is_empty` optimization, expects the caller to do that. - fn from_uniq_box_inner(mut entries: Box<[(K, V)]>) -> Self + /// Creates a [`FrozenMap`] from a boxed slice with unique keys. + /// + /// Panics if any of the keys in `entries` are overlapping. + pub fn from_unique_box(entries: Box<[(K, V)]>) -> Self where K: Ord, { - // Sort by key (stable sort preserves insertion order for equal keys) - entries.sort_by(|a, b| a.0.cmp(&b.0)); - Self::from_uniq_sorted_box(entries) + let this = Self::from_unique_box_unchecked(entries); + assert_unique(&this.entries); + this } - /// Helper: skips `.is_empty` optimization, expects the caller to do that. - fn from_vec_inner(mut entries: Vec<(K, V)>) -> Self + /// Creates a [`FrozenMap`] from a boxed slice with unique keys. + /// + /// # Correctness + /// + /// The caller must ensure that there are no overlapping keys. + /// + /// If this invariant is not upheld, the map will behave incorrectly (e.g., [`FrozenMap::get`] + /// may fail to find keys that are present), but no memory unsafety will occur. + pub fn from_unique_box_unchecked(mut entries: Box<[(K, V)]>) -> Self where K: Ord, { - // Sort by key (stable sort preserves insertion order for equal keys) - entries.sort_by(|a, b| a.0.cmp(&b.0)); + entries.sort_unstable_by(|a, b| a.0.cmp(&b.0)); + Self::from_unique_sorted_box_unchecked(entries) + } - // Deduplicate, keeping the last value for each key. + /// Creates a [`FrozenMap`] from a [`Vec`] with unique keys. + /// + /// Panics if any of the keys in `entries` are overlapping. + pub fn from_unique_vec(entries: Vec<(K, V)>) -> Self + where + K: Ord, + { + Self::from_unique_box(entries.into_boxed_slice()) + } + + /// Creates a [`FrozenMap`] from a [`Vec`] with unique keys. + /// + /// # Correctness + /// + /// The caller must ensure that there are no overlapping keys. + /// + /// If this invariant is not upheld, the map will behave incorrectly (e.g., [`FrozenMap::get`] + /// may fail to find keys that are present), but no memory unsafety will occur. + pub fn from_unique_vec_unchecked(entries: Vec<(K, V)>) -> Self + where + K: Ord, + { + Self::from_unique_box_unchecked(entries.into_boxed_slice()) + } + + /// Creates a [`FrozenMap`] from a sorted [`Vec`] with potentially-overlapping keys. + /// + /// In the case of overlapping keys, only the last overlapping entry is preserved. + /// + /// Panics if the keys in `entries` are not in sorted order. + pub fn from_overlapping_sorted_vec(mut entries: Vec<(K, V)>) -> Self + where + K: Ord, + { + // Remove overlapping keys, keeping the last value for each key. // dedup_by removes the first argument when returning true, so we swap // to keep the later (last) value in the earlier slot. entries.dedup_by(|later, earlier| { @@ -137,30 +252,27 @@ impl FrozenMap { std::mem::swap(later, earlier); true } else { + // doing the assertion here avoids an extra loop over the entries + assert!(later.0 > earlier.0, "FrozenMap entries must be sorted"); false } }); - Self::from_uniq_sorted_box(entries.into_boxed_slice()) + // `into_boxed_slice` discards excess capacity (calls `shrink_to_fit`) before boxing + Self::from_unique_sorted_box(entries.into_boxed_slice()) } -} -impl FromIterator<(K, V)> for FrozenMap { - /// Creates a [`FrozenMap`] from an iterator of key-value pairs. + /// Creates a [`FrozenMap`] from a sorted [`Vec`] with potentially-overlapping keys. /// - /// If there are duplicate keys, the last value for each key is kept. - fn from_iter>(iter: T) -> Self { - let mut entries: Vec<(K, V)> = iter.into_iter().collect(); - if entries.is_empty() { - return Self::new(); - } - - // Sort by key (stable sort preserves insertion order for equal keys) - entries.sort_by(|a, b| a.0.cmp(&b.0)); - - // Deduplicate, keeping the last value for each key. - // dedup_by removes the first argument when returning true, so we swap - // to keep the later (last) value in the earlier slot. + /// In the case of overlapping keys, only the last overlapping entry is preserved. + /// + /// # Correctness + /// + /// The caller must ensure that the entries are sorted by their key. + pub fn from_overlapping_sorted_vec_unchecked(mut entries: Vec<(K, V)>) -> Self + where + K: Eq, + { entries.dedup_by(|later, earlier| { if later.0 == earlier.0 { std::mem::swap(later, earlier); @@ -169,18 +281,61 @@ impl FromIterator<(K, V)> for FrozenMap { false } }); + Self::from_unique_sorted_box_unchecked(entries.into_boxed_slice()) + } - Self::from_uniq_sorted_box(entries.into_boxed_slice()) + /// Creates a [`FrozenMap`] from a [`Vec`] with unsorted and potentially-overlapping keys. + /// + /// In the case of overlapping keys, only the last overlapping entry is preserved. + pub fn from_overlapping_vec(mut entries: Vec<(K, V)>) -> Self + where + K: Ord, + { + // stable sort preserves insertion order for overlapping keys + entries.sort_by(|a, b| a.0.cmp(&b.0)); + Self::from_overlapping_sorted_vec_unchecked(entries) + } + + /// Creates a [`FrozenMap`] from a type implementing [`IntoIterator`] with unsorted and + /// potentially-overlapping keys. + /// + /// In the case of overlapping keys, only the last overlapping entry is preserved. + /// + /// This is a thin convenience wrapper around [`FrozenMap::from_overlapping_vec`]. + pub fn from_overlapping_iter(entries: impl IntoIterator) -> Self + where + K: Ord, + { + Self::from_overlapping_vec(entries.into_iter().collect()) } } +#[track_caller] +fn assert_unique_sorted(entries: &[(K, V)]) { + assert!( + entries.is_sorted_by(|a, b| a.0 < b.0), + "FrozenMap entries must be sorted and unique", + ) +} + +#[track_caller] +fn assert_unique(entries: &[(K, V)]) { + assert!( + entries.is_sorted_by(|a, b| a.0 != b.0), + "FrozenMap entries must be unique", + ) +} + impl From> for FrozenMap { /// Creates a [`FrozenMap`] from a [`BTreeMap`]. /// /// This is more efficient than `From>` because [`BTreeMap`] already iterates in /// sorted order, so no re-sorting is needed. fn from(map: BTreeMap) -> Self { - Self::from_uniq_sorted_iter(map) + if map.is_empty() { + return Self::new(); + } + Self::from_unique_sorted_iter_unchecked(map) } } @@ -196,7 +351,7 @@ where if map.is_empty() { return Self::new(); } - Self::from_uniq_box_inner(map.into_iter().collect()) + Self::from_unique_box_unchecked(map.into_iter().collect()) } } @@ -212,59 +367,7 @@ where if map.is_empty() { return Self::new(); } - Self::from_uniq_box_inner(map.into_iter().collect()) - } -} - -impl From> for FrozenMap { - /// Creates a [`FrozenMap`] from an array of key-value pairs. - /// - /// If there are duplicate keys, the last value for each key is kept. - fn from(entries: Vec<(K, V)>) -> Self { - if entries.is_empty() { - return Self::new(); - } - Self::from_vec_inner(entries) - } -} - -impl From> for FrozenMap { - /// Creates a [`FrozenMap`] from an array of key-value pairs. - /// - /// If there are duplicate keys, the last value for each key is kept. - fn from(entries: Box<[(K, V)]>) -> Self { - if entries.is_empty() { - return Self::new(); - } - Self::from_vec_inner(Vec::from(entries)) - } -} - -impl From<&[(K, V)]> for FrozenMap -where - K: Ord + Clone, - V: Clone, -{ - /// Creates a [`FrozenMap`] from a slice of key-value pairs. - /// - /// If there are duplicate keys, the last value for each key is kept. - fn from(entries: &[(K, V)]) -> Self { - if entries.is_empty() { - return Self::new(); - } - Self::from_vec_inner(Vec::from(entries)) - } -} - -impl From<[(K, V); N]> for FrozenMap { - /// Creates a [`FrozenMap`] from an array of key-value pairs. - /// - /// If there are duplicate keys, the last value for each key is kept. - fn from(entries: [(K, V); N]) -> Self { - if entries.is_empty() { - return Self::new(); - } - Self::from_vec_inner(Vec::from(entries)) + Self::from_unique_box_unchecked(map.into_iter().collect()) } } @@ -520,7 +623,7 @@ impl Clone for Iter<'_, K, V> { } } -/// An owning iterator over the entries of a `FrozenMap`. +/// An owning iterator over the entries of a [`FrozenMap`]. pub struct IntoIter { inner: std::vec::IntoIter<(K, V)>, } @@ -561,7 +664,7 @@ impl ExactSizeIterator for IntoIter { impl FusedIterator for IntoIter {} -/// An iterator over the keys of a `FrozenMap`. +/// An iterator over the keys of a [`FrozenMap`]. pub struct Keys<'a, K, V> { inner: Iter<'a, K, V>, } @@ -608,8 +711,7 @@ impl ExactSizeIterator for Keys<'_, K, V> { impl FusedIterator for Keys<'_, K, V> {} -// Manual implementation because the derive would add an unnecessary `K: Clone, V: Clone` type -// bounds. +// Manual implementation because the derive would add unnecessary `K: Clone, V: Clone` type bounds. impl Clone for Keys<'_, K, V> { fn clone(&self) -> Self { Self { @@ -618,7 +720,7 @@ impl Clone for Keys<'_, K, V> { } } -/// An iterator over the values of a `FrozenMap`. +/// An iterator over the values of a [`FrozenMap`]. pub struct Values<'a, K, V> { inner: Iter<'a, K, V>, } @@ -665,8 +767,7 @@ impl ExactSizeIterator for Values<'_, K, V> { impl FusedIterator for Values<'_, K, V> {} -// Manual implementation because the derive would add an unnecessary `K: Clone, V: Clone` type -// bounds. +// Manual implementation because the derive would add unnecessary `K: Clone, V: Clone` type bounds. impl Clone for Values<'_, K, V> { fn clone(&self) -> Self { Self { @@ -675,7 +776,7 @@ impl Clone for Values<'_, K, V> { } } -/// An owning iterator over the keys of a `FrozenMap`. +/// An owning iterator over the keys of a [`FrozenMap`]. pub struct IntoKeys { inner: IntoIter, } @@ -718,7 +819,7 @@ impl ExactSizeIterator for IntoKeys { impl FusedIterator for IntoKeys {} -/// An owning iterator over the values of a `FrozenMap`. +/// An owning iterator over the values of a [`FrozenMap`]. pub struct IntoValues { inner: IntoIter, } @@ -761,7 +862,7 @@ impl ExactSizeIterator for IntoValues { impl FusedIterator for IntoValues {} -/// An iterator over a sub-range of entries in a `FrozenMap`. +/// An iterator over a sub-range of entries in a [`FrozenMap`]. pub struct Range<'a, K, V> { inner: std::slice::Iter<'a, (K, V)>, } @@ -845,8 +946,8 @@ mod tests { } #[test] - fn test_from_array() { - let frozen = FrozenMap::from([(3, "c"), (1, "a"), (2, "b")]); + fn test_from_unique_vec() { + let frozen = FrozenMap::from_unique_vec(vec![(3, "c"), (1, "a"), (2, "b")]); assert_eq!(frozen.len(), 3); assert_eq!(frozen.get(&1), Some(&"a")); @@ -855,17 +956,18 @@ mod tests { } #[test] - fn test_from_iter_with_duplicates() { - let frozen: FrozenMap<_, _> = [(1, "a"), (1, "b"), (2, "c")].into_iter().collect(); + fn test_from_overlapping_vec() { + let frozen = FrozenMap::from_overlapping_vec(vec![(1, "a"), (1, "b"), (2, "c")]); assert_eq!(frozen.len(), 2); - // Last value wins for duplicates + // Last value wins for overlapping keys assert_eq!(frozen.get(&1), Some(&"b")); assert_eq!(frozen.get(&2), Some(&"c")); } #[test] fn test_range() { - let frozen = FrozenMap::from([(1, "a"), (2, "b"), (3, "c"), (4, "d"), (5, "e")]); + let frozen = + FrozenMap::from_unique_vec(vec![(1, "a"), (2, "b"), (3, "c"), (4, "d"), (5, "e")]); let range: Vec<_> = frozen.range(2..4).collect(); assert_eq!(range, vec![(&2, &"b"), (&3, &"c")]); @@ -879,7 +981,7 @@ mod tests { #[test] fn test_index() { - let frozen = FrozenMap::from([(1, "a"), (2, "b")]); + let frozen = FrozenMap::from_unique_vec(vec![(1, "a"), (2, "b")]); assert_eq!(frozen[&1], "a"); assert_eq!(frozen[&2], "b"); } @@ -887,13 +989,13 @@ mod tests { #[test] #[should_panic(expected = "no entry found for key")] fn test_index_missing() { - let frozen = FrozenMap::from([(1, "a")]); + let frozen = FrozenMap::from_unique_vec(vec![(1, "a")]); let _ = frozen[&2]; } #[test] fn test_first_last() { - let frozen = FrozenMap::from([(2, "b"), (1, "a"), (3, "c")]); + let frozen = FrozenMap::from_unique_vec(vec![(2, "b"), (1, "a"), (3, "c")]); assert_eq!(frozen.first_key_value(), Some((&1, &"a"))); assert_eq!(frozen.last_key_value(), Some((&3, &"c"))); @@ -904,8 +1006,41 @@ mod tests { #[test] fn test_as_ref() { - let frozen = FrozenMap::from([(2, "b"), (1, "a"), (3, "c")]); + let frozen = FrozenMap::from_unique_vec(vec![(2, "b"), (1, "a"), (3, "c")]); let slice: &[(i32, &str)] = frozen.as_ref(); assert_eq!(slice, &[(1, "a"), (2, "b"), (3, "c")]); + + let empty = FrozenMap::::new(); + let empty_slice: &[(i32, i32)] = empty.as_ref(); + assert_eq!(empty_slice, &[]); + } + + #[test] + fn test_from_hashmap() { + let mut map = HashMap::new(); + map.insert(3, "c"); + map.insert(1, "a"); + map.insert(2, "b"); + + let frozen = FrozenMap::from(map); + assert_eq!(frozen.len(), 3); + assert_eq!(frozen.get(&1), Some(&"a")); + let keys: Vec<_> = frozen.keys().copied().collect(); + assert_eq!(keys, vec![1, 2, 3]); + } + + #[test] + #[should_panic(expected = "FrozenMap entries must be unique")] + fn test_from_unique_vec_duplicates_panics() { + let _ = FrozenMap::from_unique_vec(vec![(1, "a"), (1, "b")]); + } + + #[test] + fn test_from_overlapping_sorted_vec() { + let frozen = + FrozenMap::from_overlapping_sorted_vec(vec![(1, "a"), (1, "b"), (2, "c"), (2, "d")]); + assert_eq!(frozen.len(), 2); + assert_eq!(frozen.get(&1), Some(&"b")); // last value wins + assert_eq!(frozen.get(&2), Some(&"d")); } } diff --git a/turbopack/crates/turbo-frozenmap/src/set.rs b/turbopack/crates/turbo-frozenmap/src/set.rs index dcd1c6373e84..4ba5f8a24cc5 100644 --- a/turbopack/crates/turbo-frozenmap/src/set.rs +++ b/turbopack/crates/turbo-frozenmap/src/set.rs @@ -2,7 +2,7 @@ use std::{ borrow::Borrow, collections::{BTreeSet, HashSet}, fmt::{self, Debug}, - hash::{BuildHasher, Hash}, + hash::BuildHasher, iter::FusedIterator, ops::RangeBounds, }; @@ -17,10 +17,30 @@ use crate::map::{self, FrozenMap}; /// /// This is a read-only set that stores elements in a contiguous, sorted array. It provides /// efficient binary search lookups and iteration, but cannot be modified after construction. +/// +/// # Construction +/// +/// If you're building a new set, and you don't expect many overlapping items, consider pushing +/// elements into a [`Vec`] and calling [`FrozenSet::from_unique_iter`] or using the +/// [`FromIterator`] implementation. It is typically cheaper to collect into a [`Vec`] and sort the +/// items once at the end than it is to maintain a temporary set data structure. +/// +/// If you already have a set, or you have many overlapping items that you don't want to temporarily +/// hold onto, you can use the [`From`] or [`Into`] traits to create a [`FrozenSet`] from one of +/// many common collections. You should prefer using a [`BTreeSet`], as it matches the sorted +/// semantics of [`FrozenSet`] and avoids a sort operation during conversion. +/// +/// Unlike [`FrozenMap`], a [`FromIterator`] implementation is provided as there is less potential +/// ambiguity about desired behavior of overlapping items. Only the last overlapping item is +/// preserved during conversion. +/// +/// Similar to the API of [`BTreeSet`], there are no convenience methods for constructing from a +/// [`Vec`] or boxed slice. Because of limitations of the internal representation and Rust's memory +/// layout rules, the most efficient way to convert from these data structures is via an +/// [`Iterator`]. #[derive( Clone, Default, PartialEq, Eq, Hash, PartialOrd, Ord, Encode, Decode, Serialize, Deserialize, )] -#[rustfmt::skip] #[bincode( decode_bounds = "T: Decode<__Context> + 'static", borrow_decode_bounds = "T: BorrowDecode<'__de, __Context> + '__de" @@ -30,58 +50,122 @@ pub struct FrozenSet { } impl FrozenSet { - /// Creates an empty `FrozenSet`. Does not perform any heap allocations. + /// Creates an empty [`FrozenSet`]. Does not perform any heap allocations. pub fn new() -> Self { FrozenSet { map: FrozenMap::new(), } } - /// Creates a [`FrozenSet`] from a pre-sorted iterator with unique elements. + /// Creates a [`FrozenSet`] from a pre-sorted iterator with unique items. + /// + /// This is more efficient than [`FromIterator`] if you know that the iterator is sorted and + /// has no overlapping items. + /// + /// Panics if the `items` are not unique and sorted. + pub fn from_unique_sorted_iter(items: impl IntoIterator) -> Self + where + T: Ord, + { + FrozenSet { + map: FrozenMap::from_unique_sorted_iter(items.into_iter().map(|t| (t, ()))), + } + } + + /// Creates a [`FrozenSet`] from a pre-sorted iterator with unique items. + /// + /// This is more efficient than [`FromIterator`] if you know that the iterator is sorted and + /// has no overlapping items. /// /// # Correctness /// /// The caller must ensure that: - /// - The iterator yields elements sorted in ascending order according to [`T: Ord`][Ord] - /// - There are no duplicate elements + /// - The iterator yields elements in ascending order according to [`T: Ord`][Ord] + /// - There are no overlapping items /// /// If these invariants are not upheld, the set will behave incorrectly (e.g., - /// [`FrozenSet::contains`] may fail to find elements that are present), but no memory - /// unsafety will occur. - pub fn from_uniq_sorted_iter(iter: impl IntoIterator) -> Self { + /// [`FrozenSet::contains`] may fail to find items that are present), but no memory unsafety + /// will occur. + pub fn from_unique_sorted_iter_unchecked(items: impl IntoIterator) -> Self { + FrozenSet { + map: FrozenMap::from_unique_sorted_iter_unchecked(items.into_iter().map(|t| (t, ()))), + } + } + + /// Creates a [`FrozenSet`] from an unsorted iterator with unique items. + /// + /// Panics if the `items` are not unique. + pub fn from_unique_iter(items: impl IntoIterator) -> Self + where + T: Ord, + { FrozenSet { - map: FrozenMap::from_uniq_sorted_iter(iter.into_iter().map(|t| (t, ()))), + map: FrozenMap::from_unique_iter(items.into_iter().map(|t| (t, ()))), } } - /// Creates a [`FrozenSet`] from an unsorted iterator, sorting by element. This is more - /// efficient than [`FromIterator`] if you know that the iterator does not contain duplicate - /// entries. + /// Creates a [`FrozenSet`] from an unsorted iterator with unique items. + /// + /// This is more efficient than [`FromIterator`] if you know that the iterator has no + /// overlapping items. /// /// # Correctness /// - /// The caller must ensure that there are no duplicate elements. + /// The caller must ensure that there are no overlapping items. /// /// If this invariant is not upheld, the set will behave incorrectly (e.g., /// [`FrozenSet::contains`] may fail to find elements that are present), but no memory unsafety /// will occur. - pub fn from_uniq_iter(iter: impl IntoIterator) -> Self + pub fn from_unique_iter_unchecked(items: impl IntoIterator) -> Self + where + T: Ord, + { + FrozenSet { + map: FrozenMap::from_unique_iter_unchecked(items.into_iter().map(|t| (t, ()))), + } + } + + /// Creates a [`FrozenSet`] from a sorted iterator with potentially overlapping items. + /// + /// Panics if the `items` are not in sorted order. + pub fn from_sorted_iter(items: impl IntoIterator) -> Self where T: Ord, { FrozenSet { - map: FrozenMap::from_uniq_iter(iter.into_iter().map(|t| (t, ()))), + map: FrozenMap::from_overlapping_sorted_vec( + items.into_iter().map(|t| (t, ())).collect(), + ), + } + } + + /// Creates a [`FrozenSet`] from a sorted iterator with potentially overlapping items. + /// + /// # Correctness + /// + /// The caller must ensure that the items are in sorted order. + /// + /// If this invariant is not upheld, the set will behave incorrectly (e.g., + /// [`FrozenSet::contains`] may fail to find elements that are present), but no memory unsafety + /// will occur. + pub fn from_sorted_iter_unchecked(items: impl IntoIterator) -> Self + where + T: Eq, + { + FrozenSet { + map: FrozenMap::from_overlapping_sorted_vec_unchecked( + items.into_iter().map(|t| (t, ())).collect(), + ), } } } impl FromIterator for FrozenSet { - /// Creates a [`FrozenSet`] from an iterator of elements. - /// - /// If there are duplicate elements, only one copy is kept (the last one encountered). - fn from_iter>(iter: I) -> Self { + /// Creates a [`FrozenSet`] from an iterator of elements. If there are overlapping elements, + /// only the last copy is kept. + fn from_iter>(items: I) -> Self { FrozenSet { - map: FrozenMap::from_iter(iter.into_iter().map(|t| (t, ()))), + map: FrozenMap::from_overlapping_iter(items.into_iter().map(|t| (t, ()))), } } } @@ -92,7 +176,7 @@ impl From> for FrozenSet { /// This is more efficient than [`From>`] because [`BTreeSet`] already iterates in /// sorted order, so no re-sorting is needed. fn from(set: BTreeSet) -> Self { - FrozenSet::from_uniq_sorted_iter(set) + Self::from_unique_sorted_iter_unchecked(set) } } @@ -105,9 +189,7 @@ where /// /// The elements are sorted during construction. fn from(set: HashSet) -> Self { - FrozenSet { - map: FrozenMap::from_uniq_iter(set.into_iter().map(|t| (t, ()))), - } + Self::from_unique_iter_unchecked(set) } } @@ -120,46 +202,14 @@ where /// /// The elements are sorted during construction. fn from(set: IndexSet) -> Self { - FrozenSet { - map: FrozenMap::from_uniq_iter(set.into_iter().map(|t| (t, ()))), - } - } -} - -impl From> for FrozenSet { - /// Creates a [`FrozenSet`] from a vector of elements. - /// - /// If there are duplicate elements, the last copy is kept. - fn from(elements: Vec) -> Self { - Self::from_iter(elements) - } -} - -impl From> for FrozenSet { - /// Creates a [`FrozenSet`] from a boxed slice of elements. - /// - /// If there are duplicate elements, the last copy is kept. - fn from(elements: Box<[T]>) -> Self { - Self::from_iter(elements) - } -} - -impl From<&[T]> for FrozenSet -where - T: Ord + Clone, -{ - /// Creates a [`FrozenSet`] from a slice of elements. - /// - /// If there are duplicate elements, the last copy is kept. - fn from(elements: &[T]) -> Self { - FrozenSet::from(Vec::from(elements)) + Self::from_unique_iter_unchecked(set) } } impl From<[T; N]> for FrozenSet { /// Creates a [`FrozenSet`] from an array of elements. /// - /// If there are duplicate elements, the last copy is kept. + /// If there are overlapping elements, the last copy is kept. fn from(elements: [T; N]) -> Self { Self::from_iter(elements) } @@ -196,23 +246,17 @@ impl FrozenSet { /// Returns a reference to the first element in the set, if any. /// This element is always the minimum of all elements in the set. - pub fn first(&self) -> Option<&T> - where - T: Ord, - { + pub fn first(&self) -> Option<&T> { self.map.first_key_value().map(|(t, _)| t) } /// Returns a reference to the last element in the set, if any. /// This element is always the maximum of all elements in the set. - pub fn last(&self) -> Option<&T> - where - T: Ord, - { + pub fn last(&self) -> Option<&T> { self.map.last_key_value().map(|(t, _)| t) } - /// Gets an iterator that visits the elements in the `FrozenSet` in ascending order. + /// Gets an iterator that visits the elements in the [`FrozenSet`] in ascending order. pub fn iter(&self) -> Iter<'_, T> { self.map.keys() } @@ -310,6 +354,10 @@ impl<'a, T> Iterator for Range<'a, T> { self.inner.next().map(|(t, _)| t) } + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() + } + fn last(self) -> Option<&'a T> { self.inner.last().map(|(t, _)| t) } @@ -333,7 +381,7 @@ impl ExactSizeIterator for Range<'_, T> { impl FusedIterator for Range<'_, T> {} -// Manual implementation because the derive would add an unnecessary `T: Clone` type bounds. +// Manual implementation because the derive would add an unnecessary `T: Clone` type bound. impl Clone for Range<'_, T> { fn clone(&self) -> Self { Self { @@ -445,4 +493,33 @@ mod tests { assert!(!a.is_superset(&c)); assert!(a.is_superset(&a)); } + + #[test] + fn test_from_hashset() { + let mut set = HashSet::new(); + set.insert(3); + set.insert(1); + set.insert(2); + + let frozen = FrozenSet::from(set); + assert_eq!(frozen.len(), 3); + assert!(frozen.contains(&1)); + let elements: Vec<_> = frozen.iter().copied().collect(); + assert_eq!(elements, vec![1, 2, 3]); + } + + #[test] + #[should_panic(expected = "FrozenMap entries must be unique")] + fn test_from_unique_iter_duplicates_panics() { + let _ = FrozenSet::from_unique_iter([1, 1, 2]); + } + + #[test] + fn test_from_sorted_iter() { + let frozen = FrozenSet::from_sorted_iter([1, 1, 2, 2, 3]); + assert_eq!(frozen.len(), 3); + assert!(frozen.contains(&1)); + assert!(frozen.contains(&2)); + assert!(frozen.contains(&3)); + } } diff --git a/turbopack/crates/turbo-tasks/src/task/task_input.rs b/turbopack/crates/turbo-tasks/src/task/task_input.rs index 3428472616c2..285c08b44d72 100644 --- a/turbopack/crates/turbo-tasks/src/task/task_input.rs +++ b/turbopack/crates/turbo-tasks/src/task/task_input.rs @@ -307,13 +307,14 @@ where V: TaskInput + 'static, { async fn resolve_input(&self) -> Result { - let mut new_entries = Vec::new(); + let mut new_entries = Vec::with_capacity(self.len()); for (k, v) in self { new_entries.push(( TaskInput::resolve_input(k).await?, TaskInput::resolve_input(v).await?, )); } + // note: resolving might deduplicate `Vc`s in keys Ok(Self::from(new_entries)) } @@ -333,11 +334,11 @@ where T: TaskInput + Ord + 'static, { async fn resolve_input(&self) -> Result { - let mut new_set = Vec::new(); + let mut new_set = Vec::with_capacity(self.len()); for value in self { new_set.push(TaskInput::resolve_input(value).await?); } - Ok(Self::from(new_set)) + Ok(Self::from_iter(new_set)) } fn is_resolved(&self) -> bool { From c789c3f3347c6ebe238885aae230ca12e6ccb2bd Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Wed, 10 Dec 2025 22:10:32 -0800 Subject: [PATCH 3/6] Fix type bound on Default --- turbopack/crates/turbo-frozenmap/src/map.rs | 11 ++++++++--- turbopack/crates/turbo-frozenmap/src/set.rs | 11 ++++++++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/turbopack/crates/turbo-frozenmap/src/map.rs b/turbopack/crates/turbo-frozenmap/src/map.rs index 50df6ef5be34..c9b340ad7df3 100644 --- a/turbopack/crates/turbo-frozenmap/src/map.rs +++ b/turbopack/crates/turbo-frozenmap/src/map.rs @@ -33,9 +33,7 @@ use serde::{Deserialize, Serialize}; /// /// There are a variety of constructors provided that can be more efficient if you know that your /// data is sorted and/or unique. -#[derive( - Clone, Default, PartialEq, Eq, Hash, PartialOrd, Ord, Encode, Decode, Serialize, Deserialize, -)] +#[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Encode, Decode, Serialize, Deserialize)] #[rustfmt::skip] // rustfmt breaks bincode's proc macro string processing #[bincode( decode_bounds = "K: Decode<__Context> + 'static, V: Decode<__Context> + 'static", @@ -509,6 +507,13 @@ impl FrozenMap { } } +// Manual implementation because the derive would add unnecessary `K: Default, V: Default` bounds. +impl Default for FrozenMap { + fn default() -> Self { + Self::new() + } +} + impl Debug for FrozenMap { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_map().entries(self.iter()).finish() diff --git a/turbopack/crates/turbo-frozenmap/src/set.rs b/turbopack/crates/turbo-frozenmap/src/set.rs index 4ba5f8a24cc5..f35680148b4f 100644 --- a/turbopack/crates/turbo-frozenmap/src/set.rs +++ b/turbopack/crates/turbo-frozenmap/src/set.rs @@ -38,9 +38,7 @@ use crate::map::{self, FrozenMap}; /// [`Vec`] or boxed slice. Because of limitations of the internal representation and Rust's memory /// layout rules, the most efficient way to convert from these data structures is via an /// [`Iterator`]. -#[derive( - Clone, Default, PartialEq, Eq, Hash, PartialOrd, Ord, Encode, Decode, Serialize, Deserialize, -)] +#[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Encode, Decode, Serialize, Deserialize)] #[bincode( decode_bounds = "T: Decode<__Context> + 'static", borrow_decode_bounds = "T: BorrowDecode<'__de, __Context> + '__de" @@ -308,6 +306,13 @@ impl FrozenSet { } } +// Manual implementation because the derive would add unnecessary `T: Default` bounds. +impl Default for FrozenSet { + fn default() -> Self { + Self::new() + } +} + impl Debug for FrozenSet { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_set().entries(self.iter()).finish() From bcee70380b69f3e4fec704a4e846e18f07c158b1 Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Wed, 10 Dec 2025 22:24:06 -0800 Subject: [PATCH 4/6] WIP: Turbopack: Replace SliceMap with FrozenMap From 6434e19a210d9a11e0873f8bb1a3904a4f639346 Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Thu, 11 Dec 2025 19:17:06 -0800 Subject: [PATCH 5/6] Make the constructors simpler, go back to implicit overlapping behavior, remove the long tail of optimized constructor variants --- turbopack/crates/turbo-frozenmap/src/map.rs | 384 +++++++------------- turbopack/crates/turbo-frozenmap/src/set.rs | 142 +++----- 2 files changed, 174 insertions(+), 352 deletions(-) diff --git a/turbopack/crates/turbo-frozenmap/src/map.rs b/turbopack/crates/turbo-frozenmap/src/map.rs index c9b340ad7df3..7503e16e1d04 100644 --- a/turbopack/crates/turbo-frozenmap/src/map.rs +++ b/turbopack/crates/turbo-frozenmap/src/map.rs @@ -19,20 +19,25 @@ use serde::{Deserialize, Serialize}; /// # Construction /// /// If you're building a new map, and you don't expect many overlapping keys, consider pushing -/// elements into a [`Vec`] and calling [`FrozenMap::from_unique_vec`] or -/// [`FrozenMap::from_overlapping_vec`]. It is typically cheaper to collect into a [`Vec`] and sort -/// the entries once at the end than it is to maintain a temporary map data structure. +/// elements into a [`Vec<(K, V)>`] and calling [`FrozenMap::from`]. It is typically cheaper to +/// collect into a [`Vec`] and sort the entries once at the end than it is to maintain a temporary +/// map data structure. /// -/// If you already have a map, or you have many overlapping keys that you don't want to temporarily -/// hold onto, you can use the [`From`] or [`Into`] traits to create a [`FrozenMap`] from one of -/// many common collections. You should prefer using a [`BTreeMap`], as it matches the sorted -/// semantics of [`FrozenMap`] and avoids a sort operation during conversion. +/// If you already have a map, need to perform lookups during construction, or you have many +/// overlapping keys that you don't want to temporarily hold onto, you can use the provided [`From`] +/// trait implementations to create a [`FrozenMap`] from one of many common collections. You should +/// prefer using a [`BTreeMap`], as it matches the sorted iteration order of [`FrozenMap`] and +/// avoids a sort operation during conversion. /// -/// [`FromIterator`] and `From>` trait implementations are intentionally not provided, -/// because the desired behavior around overlapping keys is potentially ambiguous. +/// If you don't have an existing collection, you can use the [`FromIterator<(K, V)>`] trait +/// implementation to [`.collect()`][Iterator::collect] tuples into a [`FrozenMap`]. /// -/// There are a variety of constructors provided that can be more efficient if you know that your -/// data is sorted and/or unique. +/// Finally, if you have a list of pre-sorted tuples with unique keys, you can use the advanced +/// [`FrozenMap::from_unique_sorted_box`] or [`FrozenMap::from_unique_sorted_box_unchecked`] +/// constructors, which provide the cheapest possible construction. +/// +/// Overlapping keys encountered during construction preserve the last overlapping entry, matching +/// similar behavior for other sets in the standard library. #[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Encode, Decode, Serialize, Deserialize)] #[rustfmt::skip] // rustfmt breaks bincode's proc macro string processing #[bincode( @@ -41,7 +46,7 @@ use serde::{Deserialize, Serialize}; )] pub struct FrozenMap { /// Invariant: entries are sorted by key in ascending order with no overlapping keys. - entries: Box<[(K, V)]>, + pub(crate) entries: Box<[(K, V)]>, } impl FrozenMap { @@ -54,53 +59,21 @@ impl FrozenMap { entries: Box::from([]), } } +} +impl FrozenMap +where + K: Ord, +{ /// Creates a [`FrozenMap`] from a pre-sorted boxed slice with unique keys. /// /// Panics if the keys in `entries` are not unique and sorted. - pub fn from_unique_sorted_box(entries: Box<[(K, V)]>) -> Self - where - K: Ord, - { + pub fn from_unique_sorted_box(entries: Box<[(K, V)]>) -> Self { assert_unique_sorted(&entries); - Self::from_unique_sorted_box_unchecked(entries) - } - - /// Creates a [`FrozenMap`] from a pre-sorted boxed slice with unique keys. - /// - /// # Correctness - /// - /// The caller must ensure that: - /// - The entries are sorted by key in ascending order according to [`K: Ord`][Ord] - /// - There are no overlapping keys - /// - /// If these invariants are not upheld, the map will behave incorrectly (e.g., - /// [`FrozenMap::get`] may fail to find keys that are present), but no memory unsafety will - /// occur. - pub const fn from_unique_sorted_box_unchecked(entries: Box<[(K, V)]>) -> Self { FrozenMap { entries } } - /// Creates a [`FrozenMap`] from a pre-sorted slice with unique keys. - /// - /// This may be more efficient than [`FrozenMap::from_unique_sorted_iter`] because it - /// may be reduced to a simple `memmove` for types implementing [`Copy`], plus an ordering - /// check. - /// - /// Panics if the keys in `entries` are not unique and sorted. - pub fn from_unique_sorted_slice(entries: &[(K, V)]) -> Self - where - K: Clone + Ord, - V: Clone, - { - assert_unique_sorted(entries); - Self::from_unique_sorted_slice_unchecked(entries) - } - - /// Creates a [`FrozenMap`] from a pre-sorted slice with unique keys. - /// - /// This may be more efficient than [`FrozenMap::from_unique_sorted_iter_unchecked`] because it - /// may be reduced to a simple `memmove` for types implementing [`Copy`]. + /// Creates a [`FrozenMap`] from a pre-sorted boxed slice with unique keys. /// /// # Correctness /// @@ -111,166 +84,30 @@ impl FrozenMap { /// If these invariants are not upheld, the map will behave incorrectly (e.g., /// [`FrozenMap::get`] may fail to find keys that are present), but no memory unsafety will /// occur. - pub fn from_unique_sorted_slice_unchecked(entries: &[(K, V)]) -> Self - where - K: Clone, - V: Clone, - { - Self::from_unique_sorted_box_unchecked(Box::from(entries)) - } - - /// Creates a [`FrozenMap`] from an iterator that yields sorted unique keys. - /// - /// Panics if the keys in `entries` are not unique and sorted. - pub fn from_unique_sorted_iter(entries: impl IntoIterator) -> Self - where - K: Ord, - { - let this = Self::from_unique_sorted_iter_unchecked(entries); - assert_unique_sorted(&this.entries); - this - } - - /// Creates a [`FrozenMap`] from a pre-sorted iterator with unique keys. - /// - /// # Correctness - /// - /// The caller must ensure that: - /// - The iterator yields elements sorted by key in ascending order according to [`K: Ord`][Ord] - /// - There are no overlapping keys - /// - /// If these invariants are not upheld, the map will behave incorrectly (e.g., - /// [`FrozenMap::get`] may fail to find keys that are present), but no memory unsafety will - /// occur. - pub fn from_unique_sorted_iter_unchecked(iter: impl IntoIterator) -> Self { - Self::from_unique_sorted_box_unchecked(iter.into_iter().collect()) - } - - /// Creates a [`FrozenMap`] from an unsorted iterator of unique keys. Entries are sorted by key. - /// - /// This is more efficient than [`FrozenMap::from_overlapping_iter`] if you know the keys are - /// unique. - /// - /// Panics if any of the keys in `entries` are overlapping. - pub fn from_unique_iter(entries: impl IntoIterator) -> Self - where - K: Ord, - { - let this = Self::from_unique_iter_unchecked(entries); - assert_unique(&this.entries); - this - } - - /// Creates a [`FrozenMap`] from an unsorted iterator of unique keys. Entries are sorted by key. - /// - /// # Correctness - /// - /// The caller must ensure that there are no overlapping keys. - /// - /// If this invariant is not upheld, the map will behave incorrectly (e.g., [`FrozenMap::get`] - /// may fail to find keys that are present), but no memory unsafety will occur. - pub fn from_unique_iter_unchecked(entries: impl IntoIterator) -> Self - where - K: Ord, - { - let entries: Box<[(K, V)]> = entries.into_iter().collect(); - if entries.is_empty() { - return Self::new(); - } - Self::from_unique_box_unchecked(entries) - } - - /// Creates a [`FrozenMap`] from a boxed slice with unique keys. /// - /// Panics if any of the keys in `entries` are overlapping. - pub fn from_unique_box(entries: Box<[(K, V)]>) -> Self - where - K: Ord, - { - let this = Self::from_unique_box_unchecked(entries); - assert_unique(&this.entries); - this + /// When `debug_assertions` is enabled, this will panic if an invariant is not upheld. + pub fn from_unique_sorted_box_unchecked(entries: Box<[(K, V)]>) -> Self { + debug_assert_unique_sorted(&entries); + FrozenMap { entries } } - /// Creates a [`FrozenMap`] from a boxed slice with unique keys. + /// Helper: Sorts keys before constructing. Does not perform any assertions. /// - /// # Correctness - /// - /// The caller must ensure that there are no overlapping keys. - /// - /// If this invariant is not upheld, the map will behave incorrectly (e.g., [`FrozenMap::get`] - /// may fail to find keys that are present), but no memory unsafety will occur. - pub fn from_unique_box_unchecked(mut entries: Box<[(K, V)]>) -> Self - where - K: Ord, - { + /// The caller of this helper should provide a fast-path for empty collections. + pub(crate) fn from_unique_box_inner(mut entries: Box<[(K, V)]>) -> Self { entries.sort_unstable_by(|a, b| a.0.cmp(&b.0)); Self::from_unique_sorted_box_unchecked(entries) } - /// Creates a [`FrozenMap`] from a [`Vec`] with unique keys. - /// - /// Panics if any of the keys in `entries` are overlapping. - pub fn from_unique_vec(entries: Vec<(K, V)>) -> Self - where - K: Ord, - { - Self::from_unique_box(entries.into_boxed_slice()) - } - - /// Creates a [`FrozenMap`] from a [`Vec`] with unique keys. - /// - /// # Correctness + /// Helper: Sorts and deduplicates keys before constructing. Does not perform any assertions. /// - /// The caller must ensure that there are no overlapping keys. - /// - /// If this invariant is not upheld, the map will behave incorrectly (e.g., [`FrozenMap::get`] - /// may fail to find keys that are present), but no memory unsafety will occur. - pub fn from_unique_vec_unchecked(entries: Vec<(K, V)>) -> Self - where - K: Ord, - { - Self::from_unique_box_unchecked(entries.into_boxed_slice()) - } - - /// Creates a [`FrozenMap`] from a sorted [`Vec`] with potentially-overlapping keys. - /// - /// In the case of overlapping keys, only the last overlapping entry is preserved. - /// - /// Panics if the keys in `entries` are not in sorted order. - pub fn from_overlapping_sorted_vec(mut entries: Vec<(K, V)>) -> Self - where - K: Ord, - { - // Remove overlapping keys, keeping the last value for each key. - // dedup_by removes the first argument when returning true, so we swap - // to keep the later (last) value in the earlier slot. - entries.dedup_by(|later, earlier| { - if later.0 == earlier.0 { - std::mem::swap(later, earlier); - true - } else { - // doing the assertion here avoids an extra loop over the entries - assert!(later.0 > earlier.0, "FrozenMap entries must be sorted"); - false - } - }); - - // `into_boxed_slice` discards excess capacity (calls `shrink_to_fit`) before boxing - Self::from_unique_sorted_box(entries.into_boxed_slice()) - } - - /// Creates a [`FrozenMap`] from a sorted [`Vec`] with potentially-overlapping keys. - /// - /// In the case of overlapping keys, only the last overlapping entry is preserved. - /// - /// # Correctness - /// - /// The caller must ensure that the entries are sorted by their key. - pub fn from_overlapping_sorted_vec_unchecked(mut entries: Vec<(K, V)>) -> Self - where - K: Eq, - { + /// The caller of this helper should provide a fast-path for empty collections. + pub(crate) fn from_vec_inner(mut entries: Vec<(K, V)>) -> Self { + // stable sort preserves insertion order for overlapping keys + entries.sort_by(|a, b| a.0.cmp(&b.0)); + // Deduplicate, keeping the last value for each key. + // `dedup_by` removes the first argument when returning true, so we swap to keep the later + // (last) value in the earlier slot. entries.dedup_by(|later, earlier| { if later.0 == earlier.0 { std::mem::swap(later, earlier); @@ -281,31 +118,6 @@ impl FrozenMap { }); Self::from_unique_sorted_box_unchecked(entries.into_boxed_slice()) } - - /// Creates a [`FrozenMap`] from a [`Vec`] with unsorted and potentially-overlapping keys. - /// - /// In the case of overlapping keys, only the last overlapping entry is preserved. - pub fn from_overlapping_vec(mut entries: Vec<(K, V)>) -> Self - where - K: Ord, - { - // stable sort preserves insertion order for overlapping keys - entries.sort_by(|a, b| a.0.cmp(&b.0)); - Self::from_overlapping_sorted_vec_unchecked(entries) - } - - /// Creates a [`FrozenMap`] from a type implementing [`IntoIterator`] with unsorted and - /// potentially-overlapping keys. - /// - /// In the case of overlapping keys, only the last overlapping entry is preserved. - /// - /// This is a thin convenience wrapper around [`FrozenMap::from_overlapping_vec`]. - pub fn from_overlapping_iter(entries: impl IntoIterator) -> Self - where - K: Ord, - { - Self::from_overlapping_vec(entries.into_iter().collect()) - } } #[track_caller] @@ -317,13 +129,23 @@ fn assert_unique_sorted(entries: &[(K, V)]) { } #[track_caller] -fn assert_unique(entries: &[(K, V)]) { - assert!( - entries.is_sorted_by(|a, b| a.0 != b.0), - "FrozenMap entries must be unique", +fn debug_assert_unique_sorted(entries: &[(K, V)]) { + debug_assert!( + entries.is_sorted_by(|a, b| a.0 < b.0), + "FrozenMap entries must be sorted and unique", ) } +impl FromIterator<(K, V)> for FrozenMap { + /// Creates a [`FrozenMap`] from an iterator of key-value pairs. + /// + /// If there are overlapping keys, the last entry for each key is kept. + fn from_iter>(entries: T) -> Self { + let entries: Vec<_> = entries.into_iter().collect(); + Self::from(entries) + } +} + impl From> for FrozenMap { /// Creates a [`FrozenMap`] from a [`BTreeMap`]. /// @@ -333,7 +155,9 @@ impl From> for FrozenMap { if map.is_empty() { return Self::new(); } - Self::from_unique_sorted_iter_unchecked(map) + FrozenMap { + entries: map.into_iter().collect(), + } } } @@ -349,7 +173,7 @@ where if map.is_empty() { return Self::new(); } - Self::from_unique_box_unchecked(map.into_iter().collect()) + Self::from_unique_box_inner(map.into_iter().collect()) } } @@ -365,7 +189,59 @@ where if map.is_empty() { return Self::new(); } - Self::from_unique_box_unchecked(map.into_iter().collect()) + Self::from_unique_box_inner(map.into_iter().collect()) + } +} + +impl From> for FrozenMap { + /// Creates a [`FrozenMap`] from an array of key-value pairs. + /// + /// If there are overlapping keys, the last entry for each key is kept. + fn from(entries: Vec<(K, V)>) -> Self { + if entries.is_empty() { + return Self::new(); + } + Self::from_vec_inner(entries) + } +} + +impl From> for FrozenMap { + /// Creates a [`FrozenMap`] from an array of key-value pairs. + /// + /// If there are overlapping keys, the last entry for each key is kept. + fn from(entries: Box<[(K, V)]>) -> Self { + if entries.is_empty() { + return Self::new(); + } + Self::from_vec_inner(Vec::from(entries)) + } +} + +impl From<&[(K, V)]> for FrozenMap +where + K: Ord + Clone, + V: Clone, +{ + /// Creates a [`FrozenMap`] from a slice of key-value pairs. Keys and values are cloned. + /// + /// If there are overlapping keys, the last entry for each key is kept. + fn from(entries: &[(K, V)]) -> Self { + if entries.is_empty() { + return Self::new(); + } + Self::from_vec_inner(Vec::from(entries)) + } +} + +impl From<[(K, V); N]> for FrozenMap { + /// Creates a [`FrozenMap`] from an owned array of key-value pairs. + /// + /// If there are overlapping keys, the last entry for each key is kept. + fn from(entries: [(K, V); N]) -> Self { + if entries.is_empty() { + return Self::new(); + } + Self::from_vec_inner(Vec::from(entries)) } } @@ -950,19 +826,9 @@ mod tests { assert_eq!(keys, vec![1, 2, 3]); } - #[test] - fn test_from_unique_vec() { - let frozen = FrozenMap::from_unique_vec(vec![(3, "c"), (1, "a"), (2, "b")]); - assert_eq!(frozen.len(), 3); - assert_eq!(frozen.get(&1), Some(&"a")); - - let keys: Vec<_> = frozen.keys().copied().collect(); - assert_eq!(keys, vec![1, 2, 3]); - } - #[test] fn test_from_overlapping_vec() { - let frozen = FrozenMap::from_overlapping_vec(vec![(1, "a"), (1, "b"), (2, "c")]); + let frozen = FrozenMap::from(vec![(1, "a"), (1, "b"), (2, "c")]); assert_eq!(frozen.len(), 2); // Last value wins for overlapping keys assert_eq!(frozen.get(&1), Some(&"b")); @@ -971,8 +837,7 @@ mod tests { #[test] fn test_range() { - let frozen = - FrozenMap::from_unique_vec(vec![(1, "a"), (2, "b"), (3, "c"), (4, "d"), (5, "e")]); + let frozen = FrozenMap::from([(1, "a"), (2, "b"), (3, "c"), (4, "d"), (5, "e")]); let range: Vec<_> = frozen.range(2..4).collect(); assert_eq!(range, vec![(&2, &"b"), (&3, &"c")]); @@ -986,7 +851,7 @@ mod tests { #[test] fn test_index() { - let frozen = FrozenMap::from_unique_vec(vec![(1, "a"), (2, "b")]); + let frozen = FrozenMap::from([(1, "a"), (2, "b")]); assert_eq!(frozen[&1], "a"); assert_eq!(frozen[&2], "b"); } @@ -994,13 +859,13 @@ mod tests { #[test] #[should_panic(expected = "no entry found for key")] fn test_index_missing() { - let frozen = FrozenMap::from_unique_vec(vec![(1, "a")]); + let frozen = FrozenMap::from([(1, "a")]); let _ = frozen[&2]; } #[test] fn test_first_last() { - let frozen = FrozenMap::from_unique_vec(vec![(2, "b"), (1, "a"), (3, "c")]); + let frozen = FrozenMap::from([(2, "b"), (1, "a"), (3, "c")]); assert_eq!(frozen.first_key_value(), Some((&1, &"a"))); assert_eq!(frozen.last_key_value(), Some((&3, &"c"))); @@ -1011,7 +876,7 @@ mod tests { #[test] fn test_as_ref() { - let frozen = FrozenMap::from_unique_vec(vec![(2, "b"), (1, "a"), (3, "c")]); + let frozen = FrozenMap::from([(2, "b"), (1, "a"), (3, "c")]); let slice: &[(i32, &str)] = frozen.as_ref(); assert_eq!(slice, &[(1, "a"), (2, "b"), (3, "c")]); @@ -1035,17 +900,16 @@ mod tests { } #[test] - #[should_panic(expected = "FrozenMap entries must be unique")] - fn test_from_unique_vec_duplicates_panics() { - let _ = FrozenMap::from_unique_vec(vec![(1, "a"), (1, "b")]); + fn test_from_unique_sorted_box() { + let frozen = FrozenMap::from_unique_sorted_box(Box::from([(1, "a"), (2, "b")])); + assert_eq!(frozen.len(), 2); + assert_eq!(frozen.get(&1), Some(&"a")); + assert_eq!(frozen.get(&2), Some(&"b")); } #[test] - fn test_from_overlapping_sorted_vec() { - let frozen = - FrozenMap::from_overlapping_sorted_vec(vec![(1, "a"), (1, "b"), (2, "c"), (2, "d")]); - assert_eq!(frozen.len(), 2); - assert_eq!(frozen.get(&1), Some(&"b")); // last value wins - assert_eq!(frozen.get(&2), Some(&"d")); + #[should_panic(expected = "FrozenMap entries must be sorted and unique")] + fn test_from_unique_sorted_box_panics() { + let _ = FrozenMap::from_unique_sorted_box(Box::from([(1, "a"), (1, "b")])); } } diff --git a/turbopack/crates/turbo-frozenmap/src/set.rs b/turbopack/crates/turbo-frozenmap/src/set.rs index f35680148b4f..1eb4fd3a0746 100644 --- a/turbopack/crates/turbo-frozenmap/src/set.rs +++ b/turbopack/crates/turbo-frozenmap/src/set.rs @@ -21,18 +21,17 @@ use crate::map::{self, FrozenMap}; /// # Construction /// /// If you're building a new set, and you don't expect many overlapping items, consider pushing -/// elements into a [`Vec`] and calling [`FrozenSet::from_unique_iter`] or using the -/// [`FromIterator`] implementation. It is typically cheaper to collect into a [`Vec`] and sort the -/// items once at the end than it is to maintain a temporary set data structure. +/// elements into a [`Vec`] and calling [`FrozenSet::from`] or using the [`FromIterator`] +/// implementation via [`Iterator::collect`]. It is typically cheaper to collect into a [`Vec`] and +/// sort the items once at the end than it is to maintain a temporary set data structure. /// /// If you already have a set, or you have many overlapping items that you don't want to temporarily /// hold onto, you can use the [`From`] or [`Into`] traits to create a [`FrozenSet`] from one of /// many common collections. You should prefer using a [`BTreeSet`], as it matches the sorted /// semantics of [`FrozenSet`] and avoids a sort operation during conversion. /// -/// Unlike [`FrozenMap`], a [`FromIterator`] implementation is provided as there is less potential -/// ambiguity about desired behavior of overlapping items. Only the last overlapping item is -/// preserved during conversion. +/// Overlapping keys encountered during construction preserve the last overlapping entry, matching +/// similar behavior for other sets in the standard library. /// /// Similar to the API of [`BTreeSet`], there are no convenience methods for constructing from a /// [`Vec`] or boxed slice. Because of limitations of the internal representation and Rust's memory @@ -54,11 +53,16 @@ impl FrozenSet { map: FrozenMap::new(), } } +} +impl FrozenSet +where + T: Ord, +{ /// Creates a [`FrozenSet`] from a pre-sorted iterator with unique items. /// - /// This is more efficient than [`FromIterator`] if you know that the iterator is sorted and - /// has no overlapping items. + /// This is more efficient than [`Iterator::collect`] or [`FromIterator::from_iter`] if you know + /// that the iterator is sorted and has no overlapping items. /// /// Panics if the `items` are not unique and sorted. pub fn from_unique_sorted_iter(items: impl IntoIterator) -> Self @@ -66,14 +70,14 @@ impl FrozenSet { T: Ord, { FrozenSet { - map: FrozenMap::from_unique_sorted_iter(items.into_iter().map(|t| (t, ()))), + map: FrozenMap::from_unique_sorted_box(items.into_iter().map(|t| (t, ())).collect()), } } /// Creates a [`FrozenSet`] from a pre-sorted iterator with unique items. /// - /// This is more efficient than [`FromIterator`] if you know that the iterator is sorted and - /// has no overlapping items. + /// This is more efficient than [`Iterator::collect`] or [`FromIterator::from_iter`] if you know + /// that the iterator is sorted and has no overlapping items. /// /// # Correctness /// @@ -84,74 +88,11 @@ impl FrozenSet { /// If these invariants are not upheld, the set will behave incorrectly (e.g., /// [`FrozenSet::contains`] may fail to find items that are present), but no memory unsafety /// will occur. - pub fn from_unique_sorted_iter_unchecked(items: impl IntoIterator) -> Self { - FrozenSet { - map: FrozenMap::from_unique_sorted_iter_unchecked(items.into_iter().map(|t| (t, ()))), - } - } - - /// Creates a [`FrozenSet`] from an unsorted iterator with unique items. - /// - /// Panics if the `items` are not unique. - pub fn from_unique_iter(items: impl IntoIterator) -> Self - where - T: Ord, - { - FrozenSet { - map: FrozenMap::from_unique_iter(items.into_iter().map(|t| (t, ()))), - } - } - - /// Creates a [`FrozenSet`] from an unsorted iterator with unique items. - /// - /// This is more efficient than [`FromIterator`] if you know that the iterator has no - /// overlapping items. - /// - /// # Correctness - /// - /// The caller must ensure that there are no overlapping items. - /// - /// If this invariant is not upheld, the set will behave incorrectly (e.g., - /// [`FrozenSet::contains`] may fail to find elements that are present), but no memory unsafety - /// will occur. - pub fn from_unique_iter_unchecked(items: impl IntoIterator) -> Self - where - T: Ord, - { - FrozenSet { - map: FrozenMap::from_unique_iter_unchecked(items.into_iter().map(|t| (t, ()))), - } - } - - /// Creates a [`FrozenSet`] from a sorted iterator with potentially overlapping items. - /// - /// Panics if the `items` are not in sorted order. - pub fn from_sorted_iter(items: impl IntoIterator) -> Self - where - T: Ord, - { - FrozenSet { - map: FrozenMap::from_overlapping_sorted_vec( - items.into_iter().map(|t| (t, ())).collect(), - ), - } - } - - /// Creates a [`FrozenSet`] from a sorted iterator with potentially overlapping items. /// - /// # Correctness - /// - /// The caller must ensure that the items are in sorted order. - /// - /// If this invariant is not upheld, the set will behave incorrectly (e.g., - /// [`FrozenSet::contains`] may fail to find elements that are present), but no memory unsafety - /// will occur. - pub fn from_sorted_iter_unchecked(items: impl IntoIterator) -> Self - where - T: Eq, - { + /// When `debug_assertions` is enabled, this will panic if an invariant is not upheld. + pub fn from_unique_sorted_iter_unchecked(items: impl IntoIterator) -> Self { FrozenSet { - map: FrozenMap::from_overlapping_sorted_vec_unchecked( + map: FrozenMap::from_unique_sorted_box_unchecked( items.into_iter().map(|t| (t, ())).collect(), ), } @@ -163,7 +104,7 @@ impl FromIterator for FrozenSet { /// only the last copy is kept. fn from_iter>(items: I) -> Self { FrozenSet { - map: FrozenMap::from_overlapping_iter(items.into_iter().map(|t| (t, ()))), + map: FrozenMap::from_iter(items.into_iter().map(|t| (t, ()))), } } } @@ -174,7 +115,14 @@ impl From> for FrozenSet { /// This is more efficient than [`From>`] because [`BTreeSet`] already iterates in /// sorted order, so no re-sorting is needed. fn from(set: BTreeSet) -> Self { - Self::from_unique_sorted_iter_unchecked(set) + if set.is_empty() { + return Self::new(); + } + FrozenSet { + map: FrozenMap { + entries: set.into_iter().map(|t| (t, ())).collect(), + }, + } } } @@ -187,7 +135,12 @@ where /// /// The elements are sorted during construction. fn from(set: HashSet) -> Self { - Self::from_unique_iter_unchecked(set) + if set.is_empty() { + return Self::new(); + } + FrozenSet { + map: FrozenMap::from_unique_box_inner(set.into_iter().map(|t| (t, ())).collect()), + } } } @@ -200,14 +153,20 @@ where /// /// The elements are sorted during construction. fn from(set: IndexSet) -> Self { - Self::from_unique_iter_unchecked(set) + if set.is_empty() { + return Self::new(); + } + FrozenSet { + map: FrozenMap::from_unique_box_inner(set.into_iter().map(|t| (t, ())).collect()), + } } } impl From<[T; N]> for FrozenSet { - /// Creates a [`FrozenSet`] from an array of elements. + /// Creates a [`FrozenSet`] from an array of elements. If there are overlapping elements, the + /// last copy is kept. /// - /// If there are overlapping elements, the last copy is kept. + /// The elements are sorted during construction. fn from(elements: [T; N]) -> Self { Self::from_iter(elements) } @@ -514,17 +473,16 @@ mod tests { } #[test] - #[should_panic(expected = "FrozenMap entries must be unique")] - fn test_from_unique_iter_duplicates_panics() { - let _ = FrozenSet::from_unique_iter([1, 1, 2]); + fn test_from_unique_sorted_iter() { + let frozen = FrozenSet::from_unique_sorted_iter([1, 2]); + assert_eq!(frozen.len(), 2); + assert!(frozen.contains(&1)); + assert!(frozen.contains(&2)); } #[test] - fn test_from_sorted_iter() { - let frozen = FrozenSet::from_sorted_iter([1, 1, 2, 2, 3]); - assert_eq!(frozen.len(), 3); - assert!(frozen.contains(&1)); - assert!(frozen.contains(&2)); - assert!(frozen.contains(&3)); + #[should_panic(expected = "FrozenMap entries must be sorted and unique")] + fn test_from_unique_sorted_iter_panics() { + let _ = FrozenSet::from_unique_sorted_iter([1, 1, 2]); } } From d384d3da9a862839994bffc5b486c46e8c6870a8 Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Thu, 11 Dec 2025 19:41:30 -0800 Subject: [PATCH 6/6] more typo fixes --- turbopack/crates/turbo-frozenmap/src/map.rs | 16 +++--- turbopack/crates/turbo-frozenmap/src/set.rs | 57 ++++++++++----------- 2 files changed, 35 insertions(+), 38 deletions(-) diff --git a/turbopack/crates/turbo-frozenmap/src/map.rs b/turbopack/crates/turbo-frozenmap/src/map.rs index 7503e16e1d04..1a876984b155 100644 --- a/turbopack/crates/turbo-frozenmap/src/map.rs +++ b/turbopack/crates/turbo-frozenmap/src/map.rs @@ -19,7 +19,7 @@ use serde::{Deserialize, Serialize}; /// # Construction /// /// If you're building a new map, and you don't expect many overlapping keys, consider pushing -/// elements into a [`Vec<(K, V)>`] and calling [`FrozenMap::from`]. It is typically cheaper to +/// entries into a [`Vec<(K, V)>`] and calling [`FrozenMap::from`]. It is typically cheaper to /// collect into a [`Vec`] and sort the entries once at the end than it is to maintain a temporary /// map data structure. /// @@ -37,7 +37,7 @@ use serde::{Deserialize, Serialize}; /// constructors, which provide the cheapest possible construction. /// /// Overlapping keys encountered during construction preserve the last overlapping entry, matching -/// similar behavior for other sets in the standard library. +/// similar behavior for other maps in the standard library. #[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Encode, Decode, Serialize, Deserialize)] #[rustfmt::skip] // rustfmt breaks bincode's proc macro string processing #[bincode( @@ -124,7 +124,7 @@ where fn assert_unique_sorted(entries: &[(K, V)]) { assert!( entries.is_sorted_by(|a, b| a.0 < b.0), - "FrozenMap entries must be sorted and unique", + "FrozenMap entries must be unique and sorted", ) } @@ -132,7 +132,7 @@ fn assert_unique_sorted(entries: &[(K, V)]) { fn debug_assert_unique_sorted(entries: &[(K, V)]) { debug_assert!( entries.is_sorted_by(|a, b| a.0 < b.0), - "FrozenMap entries must be sorted and unique", + "FrozenMap entries must be unique and sorted", ) } @@ -194,7 +194,7 @@ where } impl From> for FrozenMap { - /// Creates a [`FrozenMap`] from an array of key-value pairs. + /// Creates a [`FrozenMap`] from a [`Vec`] of key-value pairs. /// /// If there are overlapping keys, the last entry for each key is kept. fn from(entries: Vec<(K, V)>) -> Self { @@ -206,7 +206,7 @@ impl From> for FrozenMap { } impl From> for FrozenMap { - /// Creates a [`FrozenMap`] from an array of key-value pairs. + /// Creates a [`FrozenMap`] from a boxed slice of key-value pairs. /// /// If there are overlapping keys, the last entry for each key is kept. fn from(entries: Box<[(K, V)]>) -> Self { @@ -336,7 +336,7 @@ impl FrozenMap { } } - /// Constructs a double-ended iterator over a sub-range of elements in the map. + /// Constructs a double-ended iterator over a sub-range of entries in the map. pub fn range(&self, range: R) -> Range<'_, K, V> where T: Ord + ?Sized, @@ -908,7 +908,7 @@ mod tests { } #[test] - #[should_panic(expected = "FrozenMap entries must be sorted and unique")] + #[should_panic(expected = "FrozenMap entries must be unique and sorted")] fn test_from_unique_sorted_box_panics() { let _ = FrozenMap::from_unique_sorted_box(Box::from([(1, "a"), (1, "b")])); } diff --git a/turbopack/crates/turbo-frozenmap/src/set.rs b/turbopack/crates/turbo-frozenmap/src/set.rs index 1eb4fd3a0746..30e12dea888f 100644 --- a/turbopack/crates/turbo-frozenmap/src/set.rs +++ b/turbopack/crates/turbo-frozenmap/src/set.rs @@ -21,7 +21,7 @@ use crate::map::{self, FrozenMap}; /// # Construction /// /// If you're building a new set, and you don't expect many overlapping items, consider pushing -/// elements into a [`Vec`] and calling [`FrozenSet::from`] or using the [`FromIterator`] +/// items into a [`Vec`] and calling [`FrozenSet::from`] or using the [`FromIterator`] /// implementation via [`Iterator::collect`]. It is typically cheaper to collect into a [`Vec`] and /// sort the items once at the end than it is to maintain a temporary set data structure. /// @@ -30,7 +30,7 @@ use crate::map::{self, FrozenMap}; /// many common collections. You should prefer using a [`BTreeSet`], as it matches the sorted /// semantics of [`FrozenSet`] and avoids a sort operation during conversion. /// -/// Overlapping keys encountered during construction preserve the last overlapping entry, matching +/// Overlapping items encountered during construction preserve the last overlapping item, matching /// similar behavior for other sets in the standard library. /// /// Similar to the API of [`BTreeSet`], there are no convenience methods for constructing from a @@ -65,10 +65,7 @@ where /// that the iterator is sorted and has no overlapping items. /// /// Panics if the `items` are not unique and sorted. - pub fn from_unique_sorted_iter(items: impl IntoIterator) -> Self - where - T: Ord, - { + pub fn from_unique_sorted_iter(items: impl IntoIterator) -> Self { FrozenSet { map: FrozenMap::from_unique_sorted_box(items.into_iter().map(|t| (t, ())).collect()), } @@ -82,7 +79,7 @@ where /// # Correctness /// /// The caller must ensure that: - /// - The iterator yields elements in ascending order according to [`T: Ord`][Ord] + /// - The iterator yields items in ascending order according to [`T: Ord`][Ord] /// - There are no overlapping items /// /// If these invariants are not upheld, the set will behave incorrectly (e.g., @@ -100,8 +97,8 @@ where } impl FromIterator for FrozenSet { - /// Creates a [`FrozenSet`] from an iterator of elements. If there are overlapping elements, - /// only the last copy is kept. + /// Creates a [`FrozenSet`] from an iterator of items. If there are overlapping items, only the + /// last copy is kept. fn from_iter>(items: I) -> Self { FrozenSet { map: FrozenMap::from_iter(items.into_iter().map(|t| (t, ()))), @@ -112,7 +109,7 @@ impl FromIterator for FrozenSet { impl From> for FrozenSet { /// Creates a [`FrozenSet`] from a [`BTreeSet`]. /// - /// This is more efficient than [`From>`] because [`BTreeSet`] already iterates in + /// This is more efficient than `From>` because [`BTreeSet`] already iterates in /// sorted order, so no re-sorting is needed. fn from(set: BTreeSet) -> Self { if set.is_empty() { @@ -133,7 +130,7 @@ where { /// Creates a [`FrozenSet`] from a [`HashSet`]. /// - /// The elements are sorted during construction. + /// The items are sorted during construction. fn from(set: HashSet) -> Self { if set.is_empty() { return Self::new(); @@ -151,7 +148,7 @@ where { /// Creates a [`FrozenSet`] from an [`IndexSet`]. /// - /// The elements are sorted during construction. + /// The items are sorted during construction. fn from(set: IndexSet) -> Self { if set.is_empty() { return Self::new(); @@ -163,12 +160,12 @@ where } impl From<[T; N]> for FrozenSet { - /// Creates a [`FrozenSet`] from an array of elements. If there are overlapping elements, the - /// last copy is kept. + /// Creates a [`FrozenSet`] from an array of items. If there are overlapping items, the last + /// copy is kept. /// - /// The elements are sorted during construction. - fn from(elements: [T; N]) -> Self { - Self::from_iter(elements) + /// The items are sorted during construction. + fn from(items: [T; N]) -> Self { + Self::from_iter(items) } } @@ -201,14 +198,14 @@ impl FrozenSet { self.map.get_key_value(value).map(|(t, _)| t) } - /// Returns a reference to the first element in the set, if any. - /// This element is always the minimum of all elements in the set. + /// Returns a reference to the first element in the set, if any. This element is always the + /// minimum of all elements in the set. pub fn first(&self) -> Option<&T> { self.map.first_key_value().map(|(t, _)| t) } - /// Returns a reference to the last element in the set, if any. - /// This element is always the maximum of all elements in the set. + /// Returns a reference to the last element in the set, if any. This element is always the + /// maximum of all elements in the set. pub fn last(&self) -> Option<&T> { self.map.last_key_value().map(|(t, _)| t) } @@ -230,8 +227,8 @@ impl FrozenSet { } } - /// Returns `true` if `self` has no elements in common with `other`. - /// This is equivalent to checking for an empty intersection. + /// Returns `true` if `self` has no elements in common with `other`. This is equivalent to + /// checking for an empty intersection. pub fn is_disjoint(&self, other: &Self) -> bool where T: Ord, @@ -243,8 +240,8 @@ impl FrozenSet { } } - /// Returns `true` if the set is a subset of another, - /// i.e., `other` contains at least all the elements in `self`. + /// Returns `true` if the set is a subset of another, i.e., `other` contains at least all the + /// elements in `self`. pub fn is_subset(&self, other: &Self) -> bool where T: Ord, @@ -255,8 +252,8 @@ impl FrozenSet { self.iter().all(|v| other.contains(v)) } - /// Returns `true` if the set is a superset of another, - /// i.e., `self` contains at least all the elements in `other`. + /// Returns `true` if the set is a superset of another, i.e., `self` contains at least all the + /// elements in `other`. pub fn is_superset(&self, other: &Self) -> bool where T: Ord, @@ -297,8 +294,8 @@ impl IntoIterator for FrozenSet { } // These could be newtype wrappers (BTreeSet does this), but type aliases are simpler to implement. -type Iter<'a, T> = map::Keys<'a, T, ()>; -type IntoIter = map::IntoKeys; +pub type Iter<'a, T> = map::Keys<'a, T, ()>; +pub type IntoIter = map::IntoKeys; /// An iterator over a sub-range of elements in a [`FrozenSet`]. pub struct Range<'a, T> { @@ -481,7 +478,7 @@ mod tests { } #[test] - #[should_panic(expected = "FrozenMap entries must be sorted and unique")] + #[should_panic(expected = "FrozenMap entries must be unique and sorted")] fn test_from_unique_sorted_iter_panics() { let _ = FrozenSet::from_unique_sorted_iter([1, 1, 2]); }