From 5861201a1f26c4f34ef63d2f452f6cc995e30651 Mon Sep 17 00:00:00 2001 From: Hendrik van Antwerpen Date: Mon, 13 Mar 2023 13:02:58 +0100 Subject: [PATCH 01/12] Add interning arena --- stack-graphs/src/arena.rs | 64 +++++++++++++++++++++++++++++++++- stack-graphs/tests/it/arena.rs | 15 ++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/stack-graphs/src/arena.rs b/stack-graphs/src/arena.rs index 6feaa3f7d..b440ea96e 100644 --- a/stack-graphs/src/arena.rs +++ b/stack-graphs/src/arena.rs @@ -30,6 +30,8 @@ //! [`StackGraph`]: ../graph/struct.StackGraph.html use std::cell::Cell; +use std::collections::hash_map::Entry; +use std::collections::HashMap; use std::fmt::Debug; use std::hash::Hash; use std::hash::Hasher; @@ -195,7 +197,7 @@ impl Arena { pub fn get(&self, handle: Handle) -> &T { unsafe { std::mem::transmute(&self.items[handle.as_usize()]) } } - /// + /// Dereferences a handle to an instance owned by this arena, returning a mutable reference to /// it. pub fn get_mut(&mut self, handle: Handle) -> &mut T { @@ -222,6 +224,66 @@ impl Arena { } } +/// An interning arena allocation. Equal handles means equal elements. +pub struct InterningArena +where + T: Clone + Eq + Hash, +{ + arena: Arena, + index: HashMap>, +} + +impl InterningArena +where + T: Clone + Eq + Hash, +{ + /// Creates a new interning arena. + pub fn new() -> Self { + Self { + arena: Arena::new(), + index: HashMap::new(), + } + } + + /// Adds a new instance to this arena, returning a stable handle to it. + /// + /// Note that we deduplicate instances of `T`. If you add two instances that + /// have the same content, you will get identical handles for each one. + pub fn add(&mut self, item: T) -> Handle { + match self.index.entry(item) { + Entry::Occupied(entry) => *entry.get(), + Entry::Vacant(entry) => { + let handle = self.arena.add(entry.key().clone()); + entry.insert(handle); + handle + } + } + } + + /// Dereferences a handle to an instance owned by this arena, returning a reference to it. + pub fn get(&self, handle: Handle) -> &T { + self.arena.get(handle) + } + + /// Dereferences a handle to an instance owned by this arena, returning a mutable reference to + /// it. + pub fn get_mut(&mut self, handle: Handle) -> &mut T { + self.arena.get_mut(handle) + } + + /// Returns an iterator of all of the handles in this arena. (Note that this iterator does not + /// retain a reference to the arena!) + pub fn iter_handles(&self) -> impl Iterator> { + self.arena.iter_handles() + } + + /// Returns the number of instances stored in this arena. + #[inline(always)] + pub fn len(&self) -> usize { + self.arena.len() + } +} + //------------------------------------------------------------------------------------------------- // Supplemental arenas diff --git a/stack-graphs/tests/it/arena.rs b/stack-graphs/tests/it/arena.rs index 5af25d66e..7fe467f74 100644 --- a/stack-graphs/tests/it/arena.rs +++ b/stack-graphs/tests/it/arena.rs @@ -8,6 +8,7 @@ use stack_graphs::arena::Arena; use stack_graphs::arena::Deque; use stack_graphs::arena::DequeArena; +use stack_graphs::arena::InterningArena; use stack_graphs::arena::List; use stack_graphs::arena::ListArena; use stack_graphs::arena::ReversibleList; @@ -229,3 +230,17 @@ fn can_compare_deques() { deque10.ensure_backwards(&mut arena); assert_eq!(deque1.cmp(&mut arena, deque10), Ordering::Less); } + +#[test] +fn can_allocate_in_interning_arena() { + let mut arena = InterningArena::new(); + let hello1 = arena.add("hello".to_string()); + let hello2 = arena.add("hello".to_string()); + let there = arena.add("there".to_string()); + assert_eq!(hello1, hello2); + assert_ne!(hello1, there); + assert_ne!(hello2, there); + assert_eq!(arena.get(hello1), arena.get(hello2)); + assert_ne!(arena.get(hello1), arena.get(there)); + assert_ne!(arena.get(hello2), arena.get(there)); +} From 2acf77798c1e55844f43dce7ef228b0e098aca33 Mon Sep 17 00:00:00 2001 From: Hendrik van Antwerpen Date: Mon, 13 Mar 2023 13:34:15 +0100 Subject: [PATCH 02/12] Introduce arena trait --- stack-graphs/src/arena.rs | 136 +++++++++++++++++++++------------ stack-graphs/src/c.rs | 1 + stack-graphs/src/graph.rs | 17 +++-- stack-graphs/src/stitching.rs | 5 +- stack-graphs/tests/it/arena.rs | 9 ++- 5 files changed, 105 insertions(+), 63 deletions(-) diff --git a/stack-graphs/src/arena.rs b/stack-graphs/src/arena.rs index b440ea96e..4b014729e 100644 --- a/stack-graphs/src/arena.rs +++ b/stack-graphs/src/arena.rs @@ -40,6 +40,7 @@ use std::mem::MaybeUninit; use std::num::NonZeroU32; use std::ops::Index; use std::ops::IndexMut; +use std::ops::Range; use bitvec::vec::BitVec; use controlled_option::Niche; @@ -159,14 +160,62 @@ impl PartialOrd for Handle { unsafe impl Send for Handle {} unsafe impl Sync for Handle {} +pub struct Handles { + range: Range, + _phantom: PhantomData, +} + +impl Iterator for Handles { + type Item = Handle; + fn next(&mut self) -> Option { + self.range + .next() + .map(|index| Handle::new(unsafe { NonZeroU32::new_unchecked(index as u32) })) + } +} + +impl From> for Handles { + fn from(range: Range) -> Self { + Handles { + range, + _phantom: PhantomData, + } + } +} + +pub trait Arena { + /// Adds a new instance to this arena, returning a stable handle to it. + fn add(&mut self, item: T) -> Handle; + + /// Dereferences a handle to an instance owned by this arena, returning a reference to it. + fn get(&self, handle: Handle) -> &T; + + /// Dereferences a handle to an instance owned by this arena, returning a mutable reference to + /// it. + fn get_mut(&mut self, handle: Handle) -> &mut T; + + /// Returns an iterator of all of the handles in this arena. (Note that this iterator does not + /// retain a reference to the arena!) + fn iter_handles(&self) -> Handles; + + /// Returns a pointer to this arena's storage. + fn as_ptr(&self) -> *const T; + + /// Returns the number of instances stored in this arena. + fn len(&self) -> usize; +} + /// Manages the life cycle of instances of type `T`. You can allocate new instances of `T` from /// the arena. All of the instances managed by this arena will be dropped as a single operation /// when the arena itself is dropped. -pub struct Arena { +/// +/// Note that we do not deduplicate instances of `T` in any way. If you add two instances that +/// have the same content, you will get distinct handles for each one. +pub struct VecArena { items: Vec>, } -impl Drop for Arena { +impl Drop for VecArena { fn drop(&mut self) { unsafe { let items = std::mem::transmute::<_, &mut [T]>(&mut self.items[1..]) as *mut [T]; @@ -175,81 +224,71 @@ impl Drop for Arena { } } -impl Arena { +impl VecArena { /// Creates a new arena. - pub fn new() -> Arena { - Arena { + pub fn new() -> VecArena { + VecArena { items: vec![MaybeUninit::uninit()], } } +} - /// Adds a new instance to this arena, returning a stable handle to it. - /// - /// Note that we do not deduplicate instances of `T` in any way. If you add two instances that - /// have the same content, you will get distinct handles for each one. - pub fn add(&mut self, item: T) -> Handle { +impl Arena for VecArena { + fn add(&mut self, item: T) -> Handle { let index = self.items.len() as u32; self.items.push(MaybeUninit::new(item)); Handle::new(unsafe { NonZeroU32::new_unchecked(index) }) } - /// Dereferences a handle to an instance owned by this arena, returning a reference to it. - pub fn get(&self, handle: Handle) -> &T { + fn get(&self, handle: Handle) -> &T { unsafe { std::mem::transmute(&self.items[handle.as_usize()]) } } - /// Dereferences a handle to an instance owned by this arena, returning a mutable reference to - /// it. - pub fn get_mut(&mut self, handle: Handle) -> &mut T { + fn get_mut(&mut self, handle: Handle) -> &mut T { unsafe { std::mem::transmute(&mut self.items[handle.as_usize()]) } } - /// Returns an iterator of all of the handles in this arena. (Note that this iterator does not - /// retain a reference to the arena!) - pub fn iter_handles(&self) -> impl Iterator> { - (1..self.items.len()) - .into_iter() - .map(|index| Handle::new(unsafe { NonZeroU32::new_unchecked(index as u32) })) + fn iter_handles(&self) -> Handles { + (1..self.items.len()).into() } - /// Returns a pointer to this arena's storage. - pub(crate) fn as_ptr(&self) -> *const T { + fn as_ptr(&self) -> *const T { self.items.as_ptr() as *const T } - /// Returns the number of instances stored in this arena. #[inline(always)] - pub fn len(&self) -> usize { + fn len(&self) -> usize { self.items.len() } } /// An interning arena allocation. Equal handles means equal elements. -pub struct InterningArena +pub struct HashArena where T: Clone + Eq + Hash, { - arena: Arena, + arena: VecArena, index: HashMap>, } -impl InterningArena +impl HashArena where T: Clone + Eq + Hash, { /// Creates a new interning arena. pub fn new() -> Self { Self { - arena: Arena::new(), + arena: VecArena::new(), index: HashMap::new(), } } +} - /// Adds a new instance to this arena, returning a stable handle to it. - /// - /// Note that we deduplicate instances of `T`. If you add two instances that - /// have the same content, you will get identical handles for each one. - pub fn add(&mut self, item: T) -> Handle { +impl Arena for HashArena +where + T: Clone + Eq + Hash, +{ + fn add(&mut self, item: T) -> Handle { match self.index.entry(item) { Entry::Occupied(entry) => *entry.get(), Entry::Vacant(entry) => { @@ -260,26 +299,24 @@ where } } - /// Dereferences a handle to an instance owned by this arena, returning a reference to it. - pub fn get(&self, handle: Handle) -> &T { + fn get(&self, handle: Handle) -> &T { self.arena.get(handle) } - /// Dereferences a handle to an instance owned by this arena, returning a mutable reference to - /// it. - pub fn get_mut(&mut self, handle: Handle) -> &mut T { + fn get_mut(&mut self, handle: Handle) -> &mut T { self.arena.get_mut(handle) } - /// Returns an iterator of all of the handles in this arena. (Note that this iterator does not - /// retain a reference to the arena!) - pub fn iter_handles(&self) -> impl Iterator> { + fn iter_handles(&self) -> Handles { self.arena.iter_handles() } - /// Returns the number of instances stored in this arena. + fn as_ptr(&self) -> *const T { + self.arena.as_ptr() + } + #[inline(always)] - pub fn len(&self) -> usize { + fn len(&self) -> usize { self.arena.len() } } @@ -299,8 +336,9 @@ where /// ``` /// # use stack_graphs::arena::Arena; /// # use stack_graphs::arena::SupplementalArena; +/// # use stack_graphs::arena::VecArena; /// // We need an Arena to create handles. -/// let mut arena = Arena::::new(); +/// let mut arena = VecArena::::new(); /// let handle = arena.add(1); /// /// let mut supplemental = SupplementalArena::::new(); @@ -344,8 +382,8 @@ impl SupplementalArena { /// Creates a new, empty supplemental arena, preallocating enough space to store supplemental /// data for all of the instances that have already been allocated in a (regular) arena. - pub fn with_capacity(arena: &Arena) -> SupplementalArena { - let mut items = Vec::with_capacity(arena.items.len()); + pub fn with_capacity(arena: impl Arena) -> SupplementalArena { + let mut items = Vec::with_capacity(arena.len()); items[0] = MaybeUninit::uninit(); SupplementalArena { items, @@ -531,7 +569,7 @@ const EMPTY_LIST_HANDLE: NonZeroU32 = unsafe { NonZeroU32::new_unchecked(u32::MA // // (Note that the arena doesn't store `List` itself; it stores the `ListCell`s that the lists // are made of.) -pub type ListArena = Arena>; +pub type ListArena = VecArena>; impl List { /// Creates a new `ListArena` that will manage lists of this type. @@ -682,7 +720,7 @@ pub struct ReversibleListCell { // // (Note that the arena doesn't store `ReversibleList` itself; it stores the // `ReversibleListCell`s that the lists are made of.) -pub type ReversibleListArena = Arena>; +pub type ReversibleListArena = VecArena>; impl ReversibleList { /// Creates a new `ReversibleListArena` that will manage lists of this type. diff --git a/stack-graphs/src/c.rs b/stack-graphs/src/c.rs index 998d4cf2e..c0dd6177a 100644 --- a/stack-graphs/src/c.rs +++ b/stack-graphs/src/c.rs @@ -14,6 +14,7 @@ use std::sync::atomic::AtomicUsize; use libc::c_char; +use crate::arena::Arena; use crate::arena::Handle; use crate::graph::File; use crate::graph::InternedString; diff --git a/stack-graphs/src/graph.rs b/stack-graphs/src/graph.rs index 6eb0f8c34..4e9e57e1e 100644 --- a/stack-graphs/src/graph.rs +++ b/stack-graphs/src/graph.rs @@ -63,6 +63,7 @@ use smallvec::SmallVec; use crate::arena::Arena; use crate::arena::Handle; use crate::arena::SupplementalArena; +use crate::arena::VecArena; //------------------------------------------------------------------------------------------------- // String content @@ -1401,13 +1402,13 @@ impl StackGraph { /// Contains all of the nodes and edges that make up a stack graph. pub struct StackGraph { interned_strings: InternedStringArena, - pub(crate) symbols: Arena, + pub(crate) symbols: VecArena, symbol_handles: FxHashMap<&'static str, Handle>, - pub(crate) strings: Arena, + pub(crate) strings: VecArena, string_handles: FxHashMap<&'static str, Handle>, - pub(crate) files: Arena, + pub(crate) files: VecArena, file_handles: FxHashMap<&'static str, Handle>, - pub(crate) nodes: Arena, + pub(crate) nodes: VecArena, pub(crate) source_info: SupplementalArena, node_id_handles: NodeIDHandles, outgoing_edges: SupplementalArena>, @@ -1573,17 +1574,17 @@ impl StackGraph { impl Default for StackGraph { fn default() -> StackGraph { - let mut nodes = Arena::new(); + let mut nodes = VecArena::new(); nodes.add(RootNode::new().into()); nodes.add(JumpToNode::new().into()); StackGraph { interned_strings: InternedStringArena::new(), - symbols: Arena::new(), + symbols: VecArena::new(), symbol_handles: FxHashMap::default(), - strings: Arena::new(), + strings: VecArena::new(), string_handles: FxHashMap::default(), - files: Arena::new(), + files: VecArena::new(), file_handles: FxHashMap::default(), nodes, source_info: SupplementalArena::new(), diff --git a/stack-graphs/src/stitching.rs b/stack-graphs/src/stitching.rs index 5de928324..cd1a61084 100644 --- a/stack-graphs/src/stitching.rs +++ b/stack-graphs/src/stitching.rs @@ -48,6 +48,7 @@ use crate::arena::List; use crate::arena::ListArena; use crate::arena::ListCell; use crate::arena::SupplementalArena; +use crate::arena::VecArena; use crate::cycles::Appendables; use crate::cycles::AppendingCycleDetector; use crate::cycles::SimilarPathDetector; @@ -74,7 +75,7 @@ use crate::CancellationFlag; /// partial paths that are actually needed, placing them into a `Database` instance as they're /// needed. pub struct Database { - pub(crate) partial_paths: Arena, + pub(crate) partial_paths: VecArena, pub(crate) local_nodes: HandleSet, symbol_stack_keys: ListArena>, symbol_stack_key_cache: HashMap, @@ -86,7 +87,7 @@ impl Database { /// Creates a new, empty database. pub fn new() -> Database { Database { - partial_paths: Arena::new(), + partial_paths: VecArena::new(), local_nodes: HandleSet::new(), symbol_stack_keys: List::new_arena(), symbol_stack_key_cache: HashMap::new(), diff --git a/stack-graphs/tests/it/arena.rs b/stack-graphs/tests/it/arena.rs index 7fe467f74..8b7faac8d 100644 --- a/stack-graphs/tests/it/arena.rs +++ b/stack-graphs/tests/it/arena.rs @@ -8,16 +8,17 @@ use stack_graphs::arena::Arena; use stack_graphs::arena::Deque; use stack_graphs::arena::DequeArena; -use stack_graphs::arena::InterningArena; +use stack_graphs::arena::HashArena; use stack_graphs::arena::List; use stack_graphs::arena::ListArena; use stack_graphs::arena::ReversibleList; use stack_graphs::arena::ReversibleListArena; use stack_graphs::arena::SupplementalArena; +use stack_graphs::arena::VecArena; #[test] fn can_allocate_in_arena() { - let mut arena = Arena::new(); + let mut arena = VecArena::new(); let hello1 = arena.add("hello".to_string()); let hello2 = arena.add("hello".to_string()); let there = arena.add("there".to_string()); @@ -31,7 +32,7 @@ fn can_allocate_in_arena() { #[test] fn can_allocate_in_supplemental_arena() { - let mut arena = Arena::::new(); + let mut arena = VecArena::::new(); let h1 = arena.add(1); let h2 = arena.add(2); let h3 = arena.add(3); @@ -233,7 +234,7 @@ fn can_compare_deques() { #[test] fn can_allocate_in_interning_arena() { - let mut arena = InterningArena::new(); + let mut arena = HashArena::new(); let hello1 = arena.add("hello".to_string()); let hello2 = arena.add("hello".to_string()); let there = arena.add("there".to_string()); From 9f674479f093e8cbfddcf0ae649028a4bca8d31b Mon Sep 17 00:00:00 2001 From: Hendrik van Antwerpen Date: Mon, 13 Mar 2023 14:15:52 +0100 Subject: [PATCH 03/12] Support any Arena for ReversibleList --- stack-graphs/src/arena.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/stack-graphs/src/arena.rs b/stack-graphs/src/arena.rs index 4b014729e..fdf38f3ec 100644 --- a/stack-graphs/src/arena.rs +++ b/stack-graphs/src/arena.rs @@ -716,16 +716,12 @@ pub struct ReversibleListCell { reversed: Cell>>>, } -// An arena that's used to manage `ReversibleList` instances. -// -// (Note that the arena doesn't store `ReversibleList` itself; it stores the -// `ReversibleListCell`s that the lists are made of.) pub type ReversibleListArena = VecArena>; impl ReversibleList { - /// Creates a new `ReversibleListArena` that will manage lists of this type. + /// Creates a new arena that will manage lists of this type. pub fn new_arena() -> ReversibleListArena { - ReversibleListArena::new() + VecArena::new() } /// Returns whether this list is empty. @@ -765,12 +761,11 @@ impl ReversibleList { self.cells = cell.tail; Some(&cell.head) } +} +impl<'a, T: 'a> ReversibleList { /// Returns an iterator over the elements of this list. - pub fn iter<'a>( - mut self, - arena: &'a ReversibleListArena, - ) -> impl Iterator + 'a { + pub fn iter(mut self, arena: &'a ReversibleListArena) -> impl Iterator + 'a { std::iter::from_fn(move || self.pop_front(arena)) } } @@ -997,7 +992,7 @@ pub type DequeArena = ReversibleListArena; impl Deque { /// Creates a new `DequeArena` that will manage deques of this type. pub fn new_arena() -> DequeArena { - ReversibleList::new_arena() + VecArena::new() } /// Returns whether this deque is empty. From cb6f651ad53c7131b150aa8409208a7dec39195a Mon Sep 17 00:00:00 2001 From: Hendrik van Antwerpen Date: Mon, 13 Mar 2023 14:30:32 +0100 Subject: [PATCH 04/12] Support any Arena for Deque --- stack-graphs/src/arena.rs | 143 ++++++++++++++++++++------------- stack-graphs/src/partial.rs | 16 ++-- stack-graphs/tests/it/arena.rs | 23 +++--- 3 files changed, 107 insertions(+), 75 deletions(-) diff --git a/stack-graphs/src/arena.rs b/stack-graphs/src/arena.rs index fdf38f3ec..85605666d 100644 --- a/stack-graphs/src/arena.rs +++ b/stack-graphs/src/arena.rs @@ -716,11 +716,9 @@ pub struct ReversibleListCell { reversed: Cell>>>, } -pub type ReversibleListArena = VecArena>; - impl ReversibleList { /// Creates a new arena that will manage lists of this type. - pub fn new_arena() -> ReversibleListArena { + pub fn new_arena() -> impl Arena> { VecArena::new() } @@ -738,7 +736,7 @@ impl ReversibleList { } /// Returns whether we have already calculated the reversal of this list. - pub fn have_reversal(&self, arena: &ReversibleListArena) -> bool { + pub fn have_reversal(&self, arena: &impl Arena>) -> bool { if self.is_empty() { // The empty list is already reversed. return true; @@ -747,13 +745,13 @@ impl ReversibleList { } /// Pushes a new element onto the front of this list. - pub fn push_front(&mut self, arena: &mut ReversibleListArena, head: T) { + pub fn push_front(&mut self, arena: &mut impl Arena>, head: T) { self.cells = arena.add(ReversibleListCell::new(head, self.cells, None)); } /// Removes and returns the element at the front of this list. If the list is empty, returns /// `None`. - pub fn pop_front<'a>(&mut self, arena: &'a ReversibleListArena) -> Option<&'a T> { + pub fn pop_front<'a>(&mut self, arena: &'a impl Arena>) -> Option<&'a T> { if self.is_empty() { return None; } @@ -765,7 +763,10 @@ impl ReversibleList { impl<'a, T: 'a> ReversibleList { /// Returns an iterator over the elements of this list. - pub fn iter(mut self, arena: &'a ReversibleListArena) -> impl Iterator + 'a { + pub fn iter( + mut self, + arena: &'a impl Arena>, + ) -> impl Iterator + 'a { std::iter::from_fn(move || self.pop_front(arena)) } } @@ -777,7 +778,7 @@ where /// Reverses the list. Since we're already caching everything in an arena, we make sure to /// only calculate the reversal once, returning it as-is if you call this function multiple /// times. - pub fn reverse(&mut self, arena: &mut ReversibleListArena) { + pub fn reverse(&mut self, arena: &mut impl Arena>) { if self.is_empty() { return; } @@ -788,7 +789,7 @@ where /// Ensures that the reversal of this list is available. It can be useful to precalculate this /// when you have mutable access to the arena, so that you can then reverse and un-reverse the /// list later when you only have shared access to it. - pub fn ensure_reversal_available(&self, arena: &mut ReversibleListArena) { + pub fn ensure_reversal_available(&self, arena: &mut impl Arena>) { // First check to see if the list has already been reversed. if self.is_empty() { // The empty list is already reversed. @@ -807,7 +808,7 @@ where impl ReversibleList { /// Reverses the list, assuming that the reversal has already been computed. If it hasn't we /// return an error. - pub fn reverse_reused(&mut self, arena: &ReversibleListArena) -> Result<(), ()> { + pub fn reverse_reused(&mut self, arena: &impl Arena>) -> Result<(), ()> { if self.is_empty() { // The empty list is already reversed. return Ok(()); @@ -845,7 +846,7 @@ where { fn reverse( forwards: Handle>, - arena: &mut ReversibleListArena, + arena: &mut impl Arena>, ) -> Handle> { let mut reversed = ReversibleListCell::empty_handle(); let mut current = forwards; @@ -872,7 +873,7 @@ where impl ReversibleList { pub fn equals_with( mut self, - arena: &ReversibleListArena, + arena: &impl Arena>, mut other: ReversibleList, mut eq: F, ) -> bool @@ -891,7 +892,7 @@ impl ReversibleList { pub fn cmp_with( mut self, - arena: &ReversibleListArena, + arena: &impl Arena>, mut other: ReversibleList, mut cmp: F, ) -> std::cmp::Ordering @@ -915,7 +916,11 @@ impl ReversibleList where T: Eq, { - pub fn equals(self, arena: &ReversibleListArena, other: ReversibleList) -> bool { + pub fn equals( + self, + arena: &impl Arena>, + other: ReversibleList, + ) -> bool { self.equals_with(arena, other, |a, b| *a == *b) } } @@ -926,7 +931,7 @@ where { pub fn cmp( self, - arena: &ReversibleListArena, + arena: &impl Arena>, other: ReversibleList, ) -> std::cmp::Ordering { self.cmp_with(arena, other, |a, b| a.cmp(b)) @@ -948,6 +953,8 @@ impl Copy for ReversibleList {} //------------------------------------------------------------------------------------------------- // Arena-allocated deque +pub type DequeCell = ReversibleListCell; + /// An arena-allocated deque. /// /// Under the covers, this is implemented as a [`List`][]. Because these lists are singly-linked, @@ -986,12 +993,9 @@ impl std::ops::Not for DequeDirection { } } -// An arena that's used to manage `Deque` instances. -pub type DequeArena = ReversibleListArena; - impl Deque { - /// Creates a new `DequeArena` that will manage deques of this type. - pub fn new_arena() -> DequeArena { + /// Creates a new arena that will manage deques of this type. + pub fn new_arena() -> impl Arena> { VecArena::new() } @@ -1014,7 +1018,7 @@ impl Deque { } /// Returns whether we have already calculated the reversal of this deque. - pub fn have_reversal(&self, arena: &DequeArena) -> bool { + pub fn have_reversal(&self, arena: &impl Arena>) -> bool { self.list.have_reversal(arena) } @@ -1025,12 +1029,17 @@ impl Deque { fn is_forwards(&self) -> bool { matches!(self.direction, DequeDirection::Forwards) } +} +impl<'a, T: 'a> Deque { /// Returns an iterator over the contents of this deque, with no guarantee about the ordering of /// the elements. (By not caring about the ordering of the elements, you can call this method /// regardless of which direction the deque's elements are currently stored. And that, in /// turn, means that we only need shared access to the arena, and not mutable access to it.) - pub fn iter_unordered<'a>(&self, arena: &'a DequeArena) -> impl Iterator + 'a { + pub fn iter_unordered( + &self, + arena: &'a impl Arena>, + ) -> impl Iterator + 'a { self.list.iter(arena) } } @@ -1040,7 +1049,7 @@ where T: Clone, { /// Ensures that this deque has computed its backwards-facing list of elements. - pub fn ensure_backwards(&mut self, arena: &mut DequeArena) { + pub fn ensure_backwards(&mut self, arena: &mut impl Arena>) { if self.is_backwards() { return; } @@ -1049,7 +1058,7 @@ where } /// Ensures that this deque has computed its forwards-facing list of elements. - pub fn ensure_forwards(&mut self, arena: &mut DequeArena) { + pub fn ensure_forwards(&mut self, arena: &mut impl Arena>) { if self.is_forwards() { return; } @@ -1058,38 +1067,67 @@ where } /// Ensures that this deque has computed both directions of elements. - pub fn ensure_both_directions(&self, arena: &mut DequeArena) { + pub fn ensure_both_directions(&self, arena: &mut impl Arena>) { self.list.ensure_reversal_available(arena); } /// Pushes a new element onto the front of this deque. - pub fn push_front(&mut self, arena: &mut DequeArena, element: T) { + pub fn push_front(&mut self, arena: &mut impl Arena>, element: T) { self.ensure_forwards(arena); self.list.push_front(arena, element); } /// Pushes a new element onto the back of this deque. - pub fn push_back(&mut self, arena: &mut DequeArena, element: T) { + pub fn push_back(&mut self, arena: &mut impl Arena>, element: T) { self.ensure_backwards(arena); self.list.push_front(arena, element); } /// Removes and returns the element from the front of this deque. If the deque is empty, /// returns `None`. - pub fn pop_front<'a>(&mut self, arena: &'a mut DequeArena) -> Option<&'a T> { + pub fn pop_front<'a>(&mut self, arena: &'a mut impl Arena>) -> Option<&'a T> { self.ensure_forwards(arena); self.list.pop_front(arena) } /// Removes and returns the element from the back of this deque. If the deque is empty, /// returns `None`. - pub fn pop_back<'a>(&mut self, arena: &'a mut DequeArena) -> Option<&'a T> { + pub fn pop_back<'a>(&mut self, arena: &'a mut impl Arena>) -> Option<&'a T> { self.ensure_backwards(arena); self.list.pop_front(arena) } + /// Ensures that both deques are stored in the same direction. It doesn't matter _which_ + /// direction, as long as they're the same, so do the minimum amount of work to bring this + /// about. (In particular, if we've already calculated the reversal of one of the deques, + /// reverse that one.) + fn ensure_same_direction( + &mut self, + arena: &mut impl Arena>, + other: &mut Deque, + ) { + if self.direction == other.direction { + return; + } + if self.list.have_reversal(arena) { + self.list.reverse(arena); + self.direction = !self.direction; + } else { + other.list.reverse(arena); + other.direction = !other.direction; + } + } +} + +impl<'a, T: 'a> Deque +where + T: Clone, +{ /// Returns an iterator over the contents of this deque in a forwards direction. - pub fn iter<'a>(&self, arena: &'a mut DequeArena) -> impl Iterator + 'a { + pub fn iter( + &self, + arena: &'a mut impl Arena>, + ) -> impl Iterator + 'a { let mut list = self.list; if self.is_backwards() { list.reverse(arena); @@ -1098,9 +1136,9 @@ where } /// Returns an iterator over the contents of this deque in a backwards direction. - pub fn iter_reversed<'a>( + pub fn iter_reversed( &self, - arena: &'a mut DequeArena, + arena: &'a mut impl Arena>, ) -> impl Iterator + 'a { let mut list = self.list; if self.is_forwards() { @@ -1108,30 +1146,18 @@ where } list.iter(arena) } - - /// Ensures that both deques are stored in the same direction. It doesn't matter _which_ - /// direction, as long as they're the same, so do the minimum amount of work to bring this - /// about. (In particular, if we've already calculated the reversal of one of the deques, - /// reverse that one.) - fn ensure_same_direction(&mut self, arena: &mut DequeArena, other: &mut Deque) { - if self.direction == other.direction { - return; - } - if self.list.have_reversal(arena) { - self.list.reverse(arena); - self.direction = !self.direction; - } else { - other.list.reverse(arena); - other.direction = !other.direction; - } - } } impl Deque where T: Clone, { - pub fn equals_with(mut self, arena: &mut DequeArena, mut other: Deque, eq: F) -> bool + pub fn equals_with( + mut self, + arena: &mut impl Arena>, + mut other: Deque, + eq: F, + ) -> bool where F: FnMut(&T, &T) -> bool, { @@ -1141,7 +1167,7 @@ where pub fn cmp_with( mut self, - arena: &mut DequeArena, + arena: &mut impl Arena>, mut other: Deque, cmp: F, ) -> std::cmp::Ordering @@ -1160,7 +1186,7 @@ impl Deque where T: Clone + Eq, { - pub fn equals(self, arena: &mut DequeArena, other: Deque) -> bool { + pub fn equals(self, arena: &mut impl Arena>, other: Deque) -> bool { self.equals_with(arena, other, |a, b| *a == *b) } } @@ -1169,18 +1195,21 @@ impl Deque where T: Clone + Ord, { - pub fn cmp(self, arena: &mut DequeArena, other: Deque) -> std::cmp::Ordering { + pub fn cmp(self, arena: &mut impl Arena>, other: Deque) -> std::cmp::Ordering { self.cmp_with(arena, other, |a, b| a.cmp(b)) } } -impl Deque { +impl<'a, T: 'a> Deque { /// Returns an iterator over the contents of this deque in a forwards direction, assuming that /// we have already computed its forwards-facing list of elements via [`ensure_forwards`][]. /// Panics if we haven't already computed it. /// /// [`ensure_forwards`]: #method.ensure_forwards - pub fn iter_reused<'a>(&self, arena: &'a DequeArena) -> impl Iterator + 'a { + pub fn iter_reused( + &self, + arena: &'a impl Arena>, + ) -> impl Iterator + 'a { let mut list = self.list; if self.is_backwards() { list.reverse_reused(arena) @@ -1194,9 +1223,9 @@ impl Deque { /// Panics if we haven't already computed it. /// /// [`ensure_backwards`]: #method.ensure_backwards - pub fn iter_reversed_reused<'a>( + pub fn iter_reversed_reused( &self, - arena: &'a DequeArena, + arena: &'a impl Arena>, ) -> impl Iterator + 'a { let mut list = self.list; if self.is_forwards() { diff --git a/stack-graphs/src/partial.rs b/stack-graphs/src/partial.rs index a012c9bc0..ebd697996 100644 --- a/stack-graphs/src/partial.rs +++ b/stack-graphs/src/partial.rs @@ -44,10 +44,10 @@ use enumset::EnumSetType; use smallvec::SmallVec; use crate::arena::Deque; -use crate::arena::DequeArena; +use crate::arena::DequeCell; use crate::arena::Handle; use crate::arena::ReversibleList; -use crate::arena::ReversibleListArena; +use crate::arena::VecArena; use crate::cycles::Appendables; use crate::cycles::AppendingCycleDetector; use crate::graph::Edge; @@ -2672,17 +2672,17 @@ struct Join { /// Manages the state of a collection of partial paths built up as part of the partial-path-finding /// algorithm or path-stitching algorithm. pub struct PartialPaths { - pub(crate) partial_symbol_stacks: ReversibleListArena, - pub(crate) partial_scope_stacks: ReversibleListArena>, - pub(crate) partial_path_edges: DequeArena, + pub(crate) partial_symbol_stacks: VecArena>, + pub(crate) partial_scope_stacks: VecArena>>, + pub(crate) partial_path_edges: VecArena>, } impl PartialPaths { pub fn new() -> PartialPaths { PartialPaths { - partial_symbol_stacks: ReversibleList::new_arena(), - partial_scope_stacks: ReversibleList::new_arena(), - partial_path_edges: Deque::new_arena(), + partial_symbol_stacks: VecArena::new(), + partial_scope_stacks: VecArena::new(), + partial_path_edges: VecArena::new(), } } } diff --git a/stack-graphs/tests/it/arena.rs b/stack-graphs/tests/it/arena.rs index 8b7faac8d..18d3d5930 100644 --- a/stack-graphs/tests/it/arena.rs +++ b/stack-graphs/tests/it/arena.rs @@ -7,12 +7,12 @@ use stack_graphs::arena::Arena; use stack_graphs::arena::Deque; -use stack_graphs::arena::DequeArena; +use stack_graphs::arena::DequeCell; use stack_graphs::arena::HashArena; use stack_graphs::arena::List; use stack_graphs::arena::ListArena; use stack_graphs::arena::ReversibleList; -use stack_graphs::arena::ReversibleListArena; +use stack_graphs::arena::ReversibleListCell; use stack_graphs::arena::SupplementalArena; use stack_graphs::arena::VecArena; @@ -91,7 +91,10 @@ fn can_compare_lists() { #[test] fn can_create_reversible_lists() { - fn collect(list: &ReversibleList, arena: &ReversibleListArena) -> Vec { + fn collect( + list: &ReversibleList, + arena: &impl Arena>, + ) -> Vec { list.iter(arena).copied().collect() } @@ -148,13 +151,13 @@ fn can_compare_reversible_lists() { #[test] fn can_create_deques() { - fn collect(deque: &Deque, arena: &mut DequeArena) -> Vec { + fn collect(deque: &Deque, arena: &mut impl Arena>) -> Vec { deque.iter(arena).copied().collect() } - fn collect_reused(deque: &Deque, arena: &DequeArena) -> Vec { + fn collect_reused(deque: &Deque, arena: &impl Arena>) -> Vec { deque.iter_reused(arena).copied().collect() } - fn collect_rev(deque: &Deque, arena: &mut DequeArena) -> Vec { + fn collect_rev(deque: &Deque, arena: &mut impl Arena>) -> Vec { deque.iter_reversed(arena).copied().collect() } @@ -192,20 +195,20 @@ fn can_compare_deques() { let mut arena = Deque::new_arena(); // Build up deques in both directions so that our comparisons have to test the "reverse if // needed" logic. - let from_slice_forwards = |slice: &[u32], arena: &mut DequeArena| { + fn from_slice_forwards(slice: &[u32], arena: &mut impl Arena>) -> Deque { let mut deque = Deque::empty(); for element in slice.iter() { deque.push_back(arena, *element); } deque - }; - let from_slice_backwards = |slice: &[u32], arena: &mut DequeArena| { + } + fn from_slice_backwards(slice: &[u32], arena: &mut impl Arena>) -> Deque { let mut deque = Deque::empty(); for element in slice.iter().rev() { deque.push_front(arena, *element); } deque - }; + } let deque0 = from_slice_forwards(&[], &mut arena); let mut deque1 = from_slice_backwards(&[1], &mut arena); let deque2 = from_slice_forwards(&[2], &mut arena); From b7780985b1854d7c195941bf8320e0e7382d4de4 Mon Sep 17 00:00:00 2001 From: Hendrik van Antwerpen Date: Mon, 13 Mar 2023 14:38:04 +0100 Subject: [PATCH 05/12] Support any Arena for List --- stack-graphs/src/arena.rs | 33 +++++++++++++++++---------------- stack-graphs/src/cycles.rs | 5 +++-- stack-graphs/src/stitching.rs | 5 ++--- stack-graphs/tests/it/arena.rs | 4 ++-- 4 files changed, 24 insertions(+), 23 deletions(-) diff --git a/stack-graphs/src/arena.rs b/stack-graphs/src/arena.rs index 85605666d..45024a863 100644 --- a/stack-graphs/src/arena.rs +++ b/stack-graphs/src/arena.rs @@ -565,16 +565,10 @@ pub struct ListCell { const EMPTY_LIST_HANDLE: NonZeroU32 = unsafe { NonZeroU32::new_unchecked(u32::MAX) }; -// An arena that's used to manage `List` instances. -// -// (Note that the arena doesn't store `List` itself; it stores the `ListCell`s that the lists -// are made of.) -pub type ListArena = VecArena>; - impl List { - /// Creates a new `ListArena` that will manage lists of this type. - pub fn new_arena() -> ListArena { - ListArena::new() + /// Creates a new arena that will manage lists of this type. + pub fn new_arena() -> impl Arena> { + VecArena::new() } /// Returns whether this list is empty. @@ -600,7 +594,7 @@ impl List { } /// Pushes a new element onto the front of this list. - pub fn push_front(&mut self, arena: &mut ListArena, head: T) { + pub fn push_front(&mut self, arena: &mut impl Arena>, head: T) { self.cells = arena.add(ListCell { head, tail: self.cells, @@ -609,7 +603,7 @@ impl List { /// Removes and returns the element at the front of this list. If the list is empty, returns /// `None`. - pub fn pop_front<'a>(&mut self, arena: &'a ListArena) -> Option<&'a T> { + pub fn pop_front<'a>(&mut self, arena: &'a impl Arena>) -> Option<&'a T> { if self.is_empty() { return None; } @@ -617,15 +611,22 @@ impl List { self.cells = cell.tail; Some(&cell.head) } +} +impl<'a, T: 'a> List { /// Returns an iterator over the elements of this list. - pub fn iter<'a>(mut self, arena: &'a ListArena) -> impl Iterator + 'a { + pub fn iter(mut self, arena: &'a impl Arena>) -> impl Iterator + 'a { std::iter::from_fn(move || self.pop_front(arena)) } } impl List { - pub fn equals_with(mut self, arena: &ListArena, mut other: List, mut eq: F) -> bool + pub fn equals_with( + mut self, + arena: &impl Arena>, + mut other: List, + mut eq: F, + ) -> bool where F: FnMut(&T, &T) -> bool, { @@ -641,7 +642,7 @@ impl List { pub fn cmp_with( mut self, - arena: &ListArena, + arena: &impl Arena>, mut other: List, mut cmp: F, ) -> std::cmp::Ordering @@ -665,7 +666,7 @@ impl List where T: Eq, { - pub fn equals(self, arena: &ListArena, other: List) -> bool { + pub fn equals(self, arena: &impl Arena>, other: List) -> bool { self.equals_with(arena, other, |a, b| *a == *b) } } @@ -674,7 +675,7 @@ impl List where T: Ord, { - pub fn cmp(self, arena: &ListArena, other: List) -> std::cmp::Ordering { + pub fn cmp(self, arena: &impl Arena>, other: List) -> std::cmp::Ordering { self.cmp_with(arena, other, |a, b| a.cmp(b)) } } diff --git a/stack-graphs/src/cycles.rs b/stack-graphs/src/cycles.rs index eb4577c6a..43e1dbf76 100644 --- a/stack-graphs/src/cycles.rs +++ b/stack-graphs/src/cycles.rs @@ -35,7 +35,8 @@ use std::collections::HashMap; use crate::arena::Handle; use crate::arena::List; -use crate::arena::ListArena; +use crate::arena::ListCell; +use crate::arena::VecArena; use crate::graph::Edge; use crate::graph::Node; use crate::graph::StackGraph; @@ -148,7 +149,7 @@ pub struct AppendingCycleDetector { appendages: List, } -pub type Appendables = ListArena; +pub type Appendables = VecArena>; impl AppendingCycleDetector { pub fn new() -> Self { diff --git a/stack-graphs/src/stitching.rs b/stack-graphs/src/stitching.rs index cd1a61084..4865a69db 100644 --- a/stack-graphs/src/stitching.rs +++ b/stack-graphs/src/stitching.rs @@ -45,7 +45,6 @@ use crate::arena::Arena; use crate::arena::Handle; use crate::arena::HandleSet; use crate::arena::List; -use crate::arena::ListArena; use crate::arena::ListCell; use crate::arena::SupplementalArena; use crate::arena::VecArena; @@ -77,7 +76,7 @@ use crate::CancellationFlag; pub struct Database { pub(crate) partial_paths: VecArena, pub(crate) local_nodes: HandleSet, - symbol_stack_keys: ListArena>, + symbol_stack_keys: VecArena>>, symbol_stack_key_cache: HashMap, paths_by_start_node: SupplementalArena>>, root_paths_by_precondition: SupplementalArena>>, @@ -89,7 +88,7 @@ impl Database { Database { partial_paths: VecArena::new(), local_nodes: HandleSet::new(), - symbol_stack_keys: List::new_arena(), + symbol_stack_keys: VecArena::new(), symbol_stack_key_cache: HashMap::new(), paths_by_start_node: SupplementalArena::new(), root_paths_by_precondition: SupplementalArena::new(), diff --git a/stack-graphs/tests/it/arena.rs b/stack-graphs/tests/it/arena.rs index 18d3d5930..9f2066c25 100644 --- a/stack-graphs/tests/it/arena.rs +++ b/stack-graphs/tests/it/arena.rs @@ -10,7 +10,7 @@ use stack_graphs::arena::Deque; use stack_graphs::arena::DequeCell; use stack_graphs::arena::HashArena; use stack_graphs::arena::List; -use stack_graphs::arena::ListArena; +use stack_graphs::arena::ListCell; use stack_graphs::arena::ReversibleList; use stack_graphs::arena::ReversibleListCell; use stack_graphs::arena::SupplementalArena; @@ -47,7 +47,7 @@ fn can_allocate_in_supplemental_arena() { #[test] fn can_create_lists() { - fn collect(list: &List, arena: &ListArena) -> Vec { + fn collect(list: &List, arena: &impl Arena>) -> Vec { list.iter(arena).copied().collect() } From 0a00cc9b6b560eb2e691c617c84eb3ae59de0710 Mon Sep 17 00:00:00 2001 From: Hendrik van Antwerpen Date: Mon, 13 Mar 2023 14:51:26 +0100 Subject: [PATCH 06/12] Bring back nice arena types --- stack-graphs/src/arena.rs | 104 ++++++++++++++++----------------- stack-graphs/tests/it/arena.rs | 23 ++++---- 2 files changed, 61 insertions(+), 66 deletions(-) diff --git a/stack-graphs/src/arena.rs b/stack-graphs/src/arena.rs index 45024a863..7f7e63443 100644 --- a/stack-graphs/src/arena.rs +++ b/stack-graphs/src/arena.rs @@ -563,11 +563,15 @@ pub struct ListCell { tail: Handle>, } +pub trait ListArena: Arena> {} + +impl ListArena for A where A: Arena> {} + const EMPTY_LIST_HANDLE: NonZeroU32 = unsafe { NonZeroU32::new_unchecked(u32::MAX) }; impl List { /// Creates a new arena that will manage lists of this type. - pub fn new_arena() -> impl Arena> { + pub fn new_arena() -> impl ListArena { VecArena::new() } @@ -594,7 +598,7 @@ impl List { } /// Pushes a new element onto the front of this list. - pub fn push_front(&mut self, arena: &mut impl Arena>, head: T) { + pub fn push_front(&mut self, arena: &mut impl ListArena, head: T) { self.cells = arena.add(ListCell { head, tail: self.cells, @@ -603,7 +607,7 @@ impl List { /// Removes and returns the element at the front of this list. If the list is empty, returns /// `None`. - pub fn pop_front<'a>(&mut self, arena: &'a impl Arena>) -> Option<&'a T> { + pub fn pop_front<'a>(&mut self, arena: &'a impl ListArena) -> Option<&'a T> { if self.is_empty() { return None; } @@ -615,7 +619,7 @@ impl List { impl<'a, T: 'a> List { /// Returns an iterator over the elements of this list. - pub fn iter(mut self, arena: &'a impl Arena>) -> impl Iterator + 'a { + pub fn iter(mut self, arena: &'a impl ListArena) -> impl Iterator + 'a { std::iter::from_fn(move || self.pop_front(arena)) } } @@ -623,7 +627,7 @@ impl<'a, T: 'a> List { impl List { pub fn equals_with( mut self, - arena: &impl Arena>, + arena: &impl ListArena, mut other: List, mut eq: F, ) -> bool @@ -642,7 +646,7 @@ impl List { pub fn cmp_with( mut self, - arena: &impl Arena>, + arena: &impl ListArena, mut other: List, mut cmp: F, ) -> std::cmp::Ordering @@ -666,7 +670,7 @@ impl List where T: Eq, { - pub fn equals(self, arena: &impl Arena>, other: List) -> bool { + pub fn equals(self, arena: &impl ListArena, other: List) -> bool { self.equals_with(arena, other, |a, b| *a == *b) } } @@ -675,7 +679,7 @@ impl List where T: Ord, { - pub fn cmp(self, arena: &impl Arena>, other: List) -> std::cmp::Ordering { + pub fn cmp(self, arena: &impl ListArena, other: List) -> std::cmp::Ordering { self.cmp_with(arena, other, |a, b| a.cmp(b)) } } @@ -717,9 +721,13 @@ pub struct ReversibleListCell { reversed: Cell>>>, } +pub trait ReversibleListArena: Arena> {} + +impl ReversibleListArena for A where A: Arena> {} + impl ReversibleList { /// Creates a new arena that will manage lists of this type. - pub fn new_arena() -> impl Arena> { + pub fn new_arena() -> impl ReversibleListArena { VecArena::new() } @@ -737,7 +745,7 @@ impl ReversibleList { } /// Returns whether we have already calculated the reversal of this list. - pub fn have_reversal(&self, arena: &impl Arena>) -> bool { + pub fn have_reversal(&self, arena: &impl ReversibleListArena) -> bool { if self.is_empty() { // The empty list is already reversed. return true; @@ -746,13 +754,13 @@ impl ReversibleList { } /// Pushes a new element onto the front of this list. - pub fn push_front(&mut self, arena: &mut impl Arena>, head: T) { + pub fn push_front(&mut self, arena: &mut impl ReversibleListArena, head: T) { self.cells = arena.add(ReversibleListCell::new(head, self.cells, None)); } /// Removes and returns the element at the front of this list. If the list is empty, returns /// `None`. - pub fn pop_front<'a>(&mut self, arena: &'a impl Arena>) -> Option<&'a T> { + pub fn pop_front<'a>(&mut self, arena: &'a impl ReversibleListArena) -> Option<&'a T> { if self.is_empty() { return None; } @@ -766,7 +774,7 @@ impl<'a, T: 'a> ReversibleList { /// Returns an iterator over the elements of this list. pub fn iter( mut self, - arena: &'a impl Arena>, + arena: &'a impl ReversibleListArena, ) -> impl Iterator + 'a { std::iter::from_fn(move || self.pop_front(arena)) } @@ -779,7 +787,7 @@ where /// Reverses the list. Since we're already caching everything in an arena, we make sure to /// only calculate the reversal once, returning it as-is if you call this function multiple /// times. - pub fn reverse(&mut self, arena: &mut impl Arena>) { + pub fn reverse(&mut self, arena: &mut impl ReversibleListArena) { if self.is_empty() { return; } @@ -790,7 +798,7 @@ where /// Ensures that the reversal of this list is available. It can be useful to precalculate this /// when you have mutable access to the arena, so that you can then reverse and un-reverse the /// list later when you only have shared access to it. - pub fn ensure_reversal_available(&self, arena: &mut impl Arena>) { + pub fn ensure_reversal_available(&self, arena: &mut impl ReversibleListArena) { // First check to see if the list has already been reversed. if self.is_empty() { // The empty list is already reversed. @@ -809,7 +817,7 @@ where impl ReversibleList { /// Reverses the list, assuming that the reversal has already been computed. If it hasn't we /// return an error. - pub fn reverse_reused(&mut self, arena: &impl Arena>) -> Result<(), ()> { + pub fn reverse_reused(&mut self, arena: &impl ReversibleListArena) -> Result<(), ()> { if self.is_empty() { // The empty list is already reversed. return Ok(()); @@ -847,7 +855,7 @@ where { fn reverse( forwards: Handle>, - arena: &mut impl Arena>, + arena: &mut impl ReversibleListArena, ) -> Handle> { let mut reversed = ReversibleListCell::empty_handle(); let mut current = forwards; @@ -874,7 +882,7 @@ where impl ReversibleList { pub fn equals_with( mut self, - arena: &impl Arena>, + arena: &impl ReversibleListArena, mut other: ReversibleList, mut eq: F, ) -> bool @@ -893,7 +901,7 @@ impl ReversibleList { pub fn cmp_with( mut self, - arena: &impl Arena>, + arena: &impl ReversibleListArena, mut other: ReversibleList, mut cmp: F, ) -> std::cmp::Ordering @@ -917,11 +925,7 @@ impl ReversibleList where T: Eq, { - pub fn equals( - self, - arena: &impl Arena>, - other: ReversibleList, - ) -> bool { + pub fn equals(self, arena: &impl ReversibleListArena, other: ReversibleList) -> bool { self.equals_with(arena, other, |a, b| *a == *b) } } @@ -932,7 +936,7 @@ where { pub fn cmp( self, - arena: &impl Arena>, + arena: &impl ReversibleListArena, other: ReversibleList, ) -> std::cmp::Ordering { self.cmp_with(arena, other, |a, b| a.cmp(b)) @@ -956,6 +960,10 @@ impl Copy for ReversibleList {} pub type DequeCell = ReversibleListCell; +pub trait DequeArena: Arena> {} + +impl DequeArena for A where A: Arena> {} + /// An arena-allocated deque. /// /// Under the covers, this is implemented as a [`List`][]. Because these lists are singly-linked, @@ -996,7 +1004,7 @@ impl std::ops::Not for DequeDirection { impl Deque { /// Creates a new arena that will manage deques of this type. - pub fn new_arena() -> impl Arena> { + pub fn new_arena() -> impl DequeArena { VecArena::new() } @@ -1019,7 +1027,7 @@ impl Deque { } /// Returns whether we have already calculated the reversal of this deque. - pub fn have_reversal(&self, arena: &impl Arena>) -> bool { + pub fn have_reversal(&self, arena: &impl DequeArena) -> bool { self.list.have_reversal(arena) } @@ -1039,7 +1047,7 @@ impl<'a, T: 'a> Deque { /// turn, means that we only need shared access to the arena, and not mutable access to it.) pub fn iter_unordered( &self, - arena: &'a impl Arena>, + arena: &'a impl DequeArena, ) -> impl Iterator + 'a { self.list.iter(arena) } @@ -1050,7 +1058,7 @@ where T: Clone, { /// Ensures that this deque has computed its backwards-facing list of elements. - pub fn ensure_backwards(&mut self, arena: &mut impl Arena>) { + pub fn ensure_backwards(&mut self, arena: &mut impl DequeArena) { if self.is_backwards() { return; } @@ -1059,7 +1067,7 @@ where } /// Ensures that this deque has computed its forwards-facing list of elements. - pub fn ensure_forwards(&mut self, arena: &mut impl Arena>) { + pub fn ensure_forwards(&mut self, arena: &mut impl DequeArena) { if self.is_forwards() { return; } @@ -1073,27 +1081,27 @@ where } /// Pushes a new element onto the front of this deque. - pub fn push_front(&mut self, arena: &mut impl Arena>, element: T) { + pub fn push_front(&mut self, arena: &mut impl DequeArena, element: T) { self.ensure_forwards(arena); self.list.push_front(arena, element); } /// Pushes a new element onto the back of this deque. - pub fn push_back(&mut self, arena: &mut impl Arena>, element: T) { + pub fn push_back(&mut self, arena: &mut impl DequeArena, element: T) { self.ensure_backwards(arena); self.list.push_front(arena, element); } /// Removes and returns the element from the front of this deque. If the deque is empty, /// returns `None`. - pub fn pop_front<'a>(&mut self, arena: &'a mut impl Arena>) -> Option<&'a T> { + pub fn pop_front<'a>(&mut self, arena: &'a mut impl DequeArena) -> Option<&'a T> { self.ensure_forwards(arena); self.list.pop_front(arena) } /// Removes and returns the element from the back of this deque. If the deque is empty, /// returns `None`. - pub fn pop_back<'a>(&mut self, arena: &'a mut impl Arena>) -> Option<&'a T> { + pub fn pop_back<'a>(&mut self, arena: &'a mut impl DequeArena) -> Option<&'a T> { self.ensure_backwards(arena); self.list.pop_front(arena) } @@ -1102,11 +1110,7 @@ where /// direction, as long as they're the same, so do the minimum amount of work to bring this /// about. (In particular, if we've already calculated the reversal of one of the deques, /// reverse that one.) - fn ensure_same_direction( - &mut self, - arena: &mut impl Arena>, - other: &mut Deque, - ) { + fn ensure_same_direction(&mut self, arena: &mut impl DequeArena, other: &mut Deque) { if self.direction == other.direction { return; } @@ -1125,10 +1129,7 @@ where T: Clone, { /// Returns an iterator over the contents of this deque in a forwards direction. - pub fn iter( - &self, - arena: &'a mut impl Arena>, - ) -> impl Iterator + 'a { + pub fn iter(&self, arena: &'a mut impl DequeArena) -> impl Iterator + 'a { let mut list = self.list; if self.is_backwards() { list.reverse(arena); @@ -1139,7 +1140,7 @@ where /// Returns an iterator over the contents of this deque in a backwards direction. pub fn iter_reversed( &self, - arena: &'a mut impl Arena>, + arena: &'a mut impl DequeArena, ) -> impl Iterator + 'a { let mut list = self.list; if self.is_forwards() { @@ -1155,7 +1156,7 @@ where { pub fn equals_with( mut self, - arena: &mut impl Arena>, + arena: &mut impl DequeArena, mut other: Deque, eq: F, ) -> bool @@ -1168,7 +1169,7 @@ where pub fn cmp_with( mut self, - arena: &mut impl Arena>, + arena: &mut impl DequeArena, mut other: Deque, cmp: F, ) -> std::cmp::Ordering @@ -1187,7 +1188,7 @@ impl Deque where T: Clone + Eq, { - pub fn equals(self, arena: &mut impl Arena>, other: Deque) -> bool { + pub fn equals(self, arena: &mut impl DequeArena, other: Deque) -> bool { self.equals_with(arena, other, |a, b| *a == *b) } } @@ -1196,7 +1197,7 @@ impl Deque where T: Clone + Ord, { - pub fn cmp(self, arena: &mut impl Arena>, other: Deque) -> std::cmp::Ordering { + pub fn cmp(self, arena: &mut impl DequeArena, other: Deque) -> std::cmp::Ordering { self.cmp_with(arena, other, |a, b| a.cmp(b)) } } @@ -1207,10 +1208,7 @@ impl<'a, T: 'a> Deque { /// Panics if we haven't already computed it. /// /// [`ensure_forwards`]: #method.ensure_forwards - pub fn iter_reused( - &self, - arena: &'a impl Arena>, - ) -> impl Iterator + 'a { + pub fn iter_reused(&self, arena: &'a impl DequeArena) -> impl Iterator + 'a { let mut list = self.list; if self.is_backwards() { list.reverse_reused(arena) @@ -1226,7 +1224,7 @@ impl<'a, T: 'a> Deque { /// [`ensure_backwards`]: #method.ensure_backwards pub fn iter_reversed_reused( &self, - arena: &'a impl Arena>, + arena: &'a impl DequeArena, ) -> impl Iterator + 'a { let mut list = self.list; if self.is_forwards() { diff --git a/stack-graphs/tests/it/arena.rs b/stack-graphs/tests/it/arena.rs index 9f2066c25..6364deb7d 100644 --- a/stack-graphs/tests/it/arena.rs +++ b/stack-graphs/tests/it/arena.rs @@ -7,12 +7,12 @@ use stack_graphs::arena::Arena; use stack_graphs::arena::Deque; -use stack_graphs::arena::DequeCell; +use stack_graphs::arena::DequeArena; use stack_graphs::arena::HashArena; use stack_graphs::arena::List; -use stack_graphs::arena::ListCell; +use stack_graphs::arena::ListArena; use stack_graphs::arena::ReversibleList; -use stack_graphs::arena::ReversibleListCell; +use stack_graphs::arena::ReversibleListArena; use stack_graphs::arena::SupplementalArena; use stack_graphs::arena::VecArena; @@ -47,7 +47,7 @@ fn can_allocate_in_supplemental_arena() { #[test] fn can_create_lists() { - fn collect(list: &List, arena: &impl Arena>) -> Vec { + fn collect(list: &List, arena: &impl ListArena) -> Vec { list.iter(arena).copied().collect() } @@ -91,10 +91,7 @@ fn can_compare_lists() { #[test] fn can_create_reversible_lists() { - fn collect( - list: &ReversibleList, - arena: &impl Arena>, - ) -> Vec { + fn collect(list: &ReversibleList, arena: &impl ReversibleListArena) -> Vec { list.iter(arena).copied().collect() } @@ -151,13 +148,13 @@ fn can_compare_reversible_lists() { #[test] fn can_create_deques() { - fn collect(deque: &Deque, arena: &mut impl Arena>) -> Vec { + fn collect(deque: &Deque, arena: &mut impl DequeArena) -> Vec { deque.iter(arena).copied().collect() } - fn collect_reused(deque: &Deque, arena: &impl Arena>) -> Vec { + fn collect_reused(deque: &Deque, arena: &impl DequeArena) -> Vec { deque.iter_reused(arena).copied().collect() } - fn collect_rev(deque: &Deque, arena: &mut impl Arena>) -> Vec { + fn collect_rev(deque: &Deque, arena: &mut impl DequeArena) -> Vec { deque.iter_reversed(arena).copied().collect() } @@ -195,14 +192,14 @@ fn can_compare_deques() { let mut arena = Deque::new_arena(); // Build up deques in both directions so that our comparisons have to test the "reverse if // needed" logic. - fn from_slice_forwards(slice: &[u32], arena: &mut impl Arena>) -> Deque { + fn from_slice_forwards(slice: &[u32], arena: &mut impl DequeArena) -> Deque { let mut deque = Deque::empty(); for element in slice.iter() { deque.push_back(arena, *element); } deque } - fn from_slice_backwards(slice: &[u32], arena: &mut impl Arena>) -> Deque { + fn from_slice_backwards(slice: &[u32], arena: &mut impl DequeArena) -> Deque { let mut deque = Deque::empty(); for element in slice.iter().rev() { deque.push_front(arena, *element); From d7e108e41872f17295c00751d223a0836086094d Mon Sep 17 00:00:00 2001 From: Hendrik van Antwerpen Date: Mon, 13 Mar 2023 19:10:15 +0100 Subject: [PATCH 07/12] Add traits to various arena types --- stack-graphs/src/arena.rs | 20 +++++++++++-- stack-graphs/src/partial.rs | 58 +++++++++++++++++++++++++++++++++---- 2 files changed, 71 insertions(+), 7 deletions(-) diff --git a/stack-graphs/src/arena.rs b/stack-graphs/src/arena.rs index 7f7e63443..613f6e474 100644 --- a/stack-graphs/src/arena.rs +++ b/stack-graphs/src/arena.rs @@ -546,7 +546,7 @@ impl Default for HandleSet { /// linked list implementation _should_ be cache-friendly, since the individual cells are allocated /// out of an arena. #[repr(C)] -#[derive(Niche)] +#[derive(Eq, Hash, Niche, PartialEq)] pub struct List { // The value of this handle will be EMPTY_LIST_HANDLE if the list is empty. For an // Option>, the value will be zero (via the Option optimization) if the list @@ -557,6 +557,7 @@ pub struct List { #[doc(hidden)] #[repr(C)] +#[derive(Clone, Copy, Eq, Hash, PartialEq)] pub struct ListCell { head: T, // The value of this handle will be EMPTY_LIST_HANDLE if this is the last element of the list. @@ -707,7 +708,7 @@ impl Copy for List {} /// /// [`List`]: struct.List.html #[repr(C)] -#[derive(Niche)] +#[derive(Eq, Hash, Niche, PartialEq)] pub struct ReversibleList { #[niche] cells: Handle>, @@ -715,12 +716,26 @@ pub struct ReversibleList { #[repr(C)] #[doc(hidden)] +#[derive(Clone, Eq)] pub struct ReversibleListCell { head: T, tail: Handle>, reversed: Cell>>>, } +impl Hash for ReversibleListCell { + fn hash(&self, state: &mut H) { + self.head.hash(state); + self.tail.hash(state); + } +} + +impl PartialEq for ReversibleListCell { + fn eq(&self, other: &Self) -> bool { + self.head == other.head && self.tail == other.tail + } +} + pub trait ReversibleListArena: Arena> {} impl ReversibleListArena for A where A: Arena> {} @@ -980,6 +995,7 @@ impl DequeArena for A where A: Arena> {} /// /// [`List`]: struct.List.html #[repr(C)] +#[derive(Eq, Hash, PartialEq)] pub struct Deque { list: ReversibleList, direction: DequeDirection, diff --git a/stack-graphs/src/partial.rs b/stack-graphs/src/partial.rs index ebd697996..4d996e7b9 100644 --- a/stack-graphs/src/partial.rs +++ b/stack-graphs/src/partial.rs @@ -158,8 +158,8 @@ where /// Represents an unknown list of scoped symbols. #[repr(transparent)] -#[derive(Clone, Copy, Debug, Eq, Hash, Niche, Ord, PartialEq, PartialOrd)] -pub struct SymbolStackVariable(#[niche] NonZeroU32); +#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] +pub struct SymbolStackVariable(NonZeroU32); impl SymbolStackVariable { pub fn new(variable: u32) -> Option { @@ -218,13 +218,37 @@ impl TryFrom for SymbolStackVariable { } } +impl Niche for SymbolStackVariable { + type Output = u32; + + #[inline] + fn none() -> Self::Output { + 0 + } + + #[inline] + fn is_none(value: &Self::Output) -> bool { + *value == 0 + } + + #[inline] + fn into_some(value: Self) -> Self::Output { + value.as_u32() + } + + #[inline] + fn from_some(value: Self::Output) -> Self { + Self(unsafe { NonZeroU32::new_unchecked(value) }) + } +} + //------------------------------------------------------------------------------------------------- // Scope stack variables /// Represents an unknown list of exported scopes. #[repr(transparent)] -#[derive(Clone, Copy, Debug, Eq, Hash, Niche, Ord, PartialEq, PartialOrd)] -pub struct ScopeStackVariable(#[niche] NonZeroU32); +#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] +pub struct ScopeStackVariable(NonZeroU32); impl ScopeStackVariable { pub fn new(variable: u32) -> Option { @@ -289,6 +313,30 @@ impl TryFrom for ScopeStackVariable { } } +impl Niche for ScopeStackVariable { + type Output = u32; + + #[inline] + fn none() -> Self::Output { + 0 + } + + #[inline] + fn is_none(value: &Self::Output) -> bool { + *value == 0 + } + + #[inline] + fn into_some(value: Self) -> Self::Output { + value.as_u32() + } + + #[inline] + fn from_some(value: Self::Output) -> Self { + Self(unsafe { NonZeroU32::new_unchecked(value) }) + } +} + //------------------------------------------------------------------------------------------------- // Partial symbol stacks @@ -855,7 +903,7 @@ impl DisplayWithPartialPaths for PartialSymbolStack { /// A pattern that might match against a scope stack. Consists of a (possibly empty) list of /// exported scopes, along with an optional scope stack variable. #[repr(C)] -#[derive(Clone, Copy, Niche)] +#[derive(Clone, Copy, Eq, Hash, Niche, PartialEq)] pub struct PartialScopeStack { #[niche] scopes: ReversibleList>, From 296a72f81fa3384af8065c001052a99da13eb086 Mon Sep 17 00:00:00 2001 From: Hendrik van Antwerpen Date: Tue, 14 Mar 2023 11:26:47 +0100 Subject: [PATCH 08/12] Replace derived Niche implementation with explicit trait-friendly ones --- stack-graphs/src/arena.rs | 154 +++++++++++++++++++++++++++++++++- stack-graphs/src/partial.rs | 159 +++++++++++++++++++++++++++++++++--- 2 files changed, 296 insertions(+), 17 deletions(-) diff --git a/stack-graphs/src/arena.rs b/stack-graphs/src/arena.rs index 613f6e474..a658732ae 100644 --- a/stack-graphs/src/arena.rs +++ b/stack-graphs/src/arena.rs @@ -43,6 +43,7 @@ use std::ops::IndexMut; use std::ops::Range; use bitvec::vec::BitVec; +use controlled_option::ControlledOption; use controlled_option::Niche; use crate::utils::cmp_option; @@ -546,12 +547,11 @@ impl Default for HandleSet { /// linked list implementation _should_ be cache-friendly, since the individual cells are allocated /// out of an arena. #[repr(C)] -#[derive(Eq, Hash, Niche, PartialEq)] +#[derive(Eq, Hash, PartialEq)] pub struct List { // The value of this handle will be EMPTY_LIST_HANDLE if the list is empty. For an // Option>, the value will be zero (via the Option optimization) if the list // is None. - #[niche] cells: Handle>, } @@ -697,6 +697,53 @@ impl Clone for List { impl Copy for List {} +#[repr(C)] +#[derive(Eq, Hash, PartialEq)] +pub struct NicheList { + cells: ControlledOption>>, +} + +impl Niche for List { + type Output = NicheList; + + #[inline] + fn none() -> Self::Output { + NicheList { + cells: ControlledOption::none(), + } + } + + #[inline] + fn is_none(value: &Self::Output) -> bool { + value.cells.is_none() + } + + #[inline] + fn into_some(value: Self) -> Self::Output { + NicheList { + cells: ControlledOption::from(value.cells), + } + } + + #[inline] + fn from_some(value: Self::Output) -> Self { + List { + cells: value + .cells + .into_option() + .expect("Niche::from_some called on none value of ControlledOption>"), + } + } +} + +impl Clone for NicheList { + fn clone(&self) -> Self { + Self { cells: self.cells } + } +} + +impl Copy for NicheList {} + //------------------------------------------------------------------------------------------------- // Reversible arena-allocated list @@ -708,9 +755,8 @@ impl Copy for List {} /// /// [`List`]: struct.List.html #[repr(C)] -#[derive(Eq, Hash, Niche, PartialEq)] +#[derive(Eq, Hash, PartialEq)] pub struct ReversibleList { - #[niche] cells: Handle>, } @@ -970,6 +1016,52 @@ impl Clone for ReversibleList { impl Copy for ReversibleList {} +#[repr(C)] +#[derive(Eq, Hash, PartialEq)] +pub struct NicheReversibleList { + cells: ControlledOption>>, +} + +impl Niche for ReversibleList { + type Output = NicheReversibleList; + + #[inline] + fn none() -> Self::Output { + NicheReversibleList { + cells: ControlledOption::none(), + } + } + + #[inline] + fn is_none(value: &Self::Output) -> bool { + value.cells.is_none() + } + + #[inline] + fn into_some(value: Self) -> Self::Output { + NicheReversibleList { + cells: ControlledOption::from(value.cells), + } + } + + #[inline] + fn from_some(value: Self::Output) -> Self { + ReversibleList { + cells: value.cells.into_option().expect( + "Niche::from_some called on none value of ControlledOption>", + ), + } + } +} + +impl Clone for NicheReversibleList { + fn clone(&self) -> Self { + Self { cells: self.cells } + } +} + +impl Copy for NicheReversibleList {} + //------------------------------------------------------------------------------------------------- // Arena-allocated deque @@ -1265,3 +1357,57 @@ impl Clone for Deque { } impl Copy for Deque {} + +#[repr(C)] +#[derive(Eq, Hash, PartialEq)] +pub struct NicheDeque { + list: ControlledOption>, + direction: DequeDirection, +} + +impl Niche for Deque { + type Output = NicheDeque; + + #[inline] + fn none() -> Self::Output { + NicheDeque { + list: ControlledOption::none(), + direction: DequeDirection::Forwards, + } + } + + #[inline] + fn is_none(value: &Self::Output) -> bool { + value.list.is_none() + } + + #[inline] + fn into_some(value: Self) -> Self::Output { + NicheDeque { + list: ControlledOption::from(value.list), + direction: value.direction, + } + } + + #[inline] + fn from_some(value: Self::Output) -> Self { + Deque { + list: value + .list + .into_option() + .expect("Niche::from_some called on none value of ControlledOption>"), + direction: value.direction, + } + } +} + +impl Clone for NicheDeque { + fn clone(&self) -> Self { + Self { + list: self.list, + direction: self.direction, + } + } +} + +impl Copy for NicheDeque {} diff --git a/stack-graphs/src/partial.rs b/stack-graphs/src/partial.rs index 4d996e7b9..11e7c9746 100644 --- a/stack-graphs/src/partial.rs +++ b/stack-graphs/src/partial.rs @@ -36,6 +36,7 @@ use std::collections::VecDeque; use std::convert::TryFrom; use std::fmt::Display; +use std::hash::Hash; use std::num::NonZeroU32; use controlled_option::ControlledOption; @@ -305,14 +306,6 @@ impl Into for ScopeStackVariable { } } -impl TryFrom for ScopeStackVariable { - type Error = (); - fn try_from(value: u32) -> Result { - let value = NonZeroU32::new(value).ok_or(())?; - Ok(ScopeStackVariable(value)) - } -} - impl Niche for ScopeStackVariable { type Output = u32; @@ -337,12 +330,20 @@ impl Niche for ScopeStackVariable { } } +impl TryFrom for ScopeStackVariable { + type Error = (); + fn try_from(value: u32) -> Result { + let value = NonZeroU32::new(value).ok_or(())?; + Ok(ScopeStackVariable(value)) + } +} + //------------------------------------------------------------------------------------------------- // Partial symbol stacks /// A symbol with an unknown, but possibly empty, list of exported scopes attached to it. #[repr(C)] -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Eq, Hash, PartialEq)] pub struct PartialScopedSymbol { pub symbol: Handle, // Note that not having an attached scope list is _different_ than having an empty attached @@ -490,9 +491,8 @@ impl DisplayWithPartialPaths for PartialScopedSymbol { /// A pattern that might match against a symbol stack. Consists of a (possibly empty) list of /// partial scoped symbols, along with an optional symbol stack variable. #[repr(C)] -#[derive(Clone, Copy, Niche)] +#[derive(Clone, Copy)] pub struct PartialSymbolStack { - #[niche] symbols: ReversibleList, length: u32, variable: ControlledOption, @@ -897,15 +897,60 @@ impl DisplayWithPartialPaths for PartialSymbolStack { } } +#[repr(C)] +#[derive(Clone, Copy, Eq, Hash, PartialEq)] +pub struct NichePartialSymbolStack { + symbols: ControlledOption>, + length: u32, + variable: ControlledOption, +} + +impl Niche for PartialSymbolStack { + type Output = NichePartialSymbolStack; + + #[inline] + fn none() -> Self::Output { + NichePartialSymbolStack { + symbols: ControlledOption::none(), + length: 0, + variable: ControlledOption::none(), + } + } + + #[inline] + fn is_none(value: &Self::Output) -> bool { + value.symbols.is_none() + } + + #[inline] + fn into_some(value: Self) -> Self::Output { + NichePartialSymbolStack { + symbols: ControlledOption::from(value.symbols), + length: value.length, + variable: value.variable, + } + } + + #[inline] + fn from_some(value: Self::Output) -> Self { + PartialSymbolStack { + symbols: value.symbols.into_option().expect( + "Niche::from_some called on none value of ControlledOption", + ), + length: value.length, + variable: value.variable, + } + } +} + //------------------------------------------------------------------------------------------------- // Partial scope stacks /// A pattern that might match against a scope stack. Consists of a (possibly empty) list of /// exported scopes, along with an optional scope stack variable. #[repr(C)] -#[derive(Clone, Copy, Eq, Hash, Niche, PartialEq)] +#[derive(Clone, Copy, Eq, Hash, PartialEq)] pub struct PartialScopeStack { - #[niche] scopes: ReversibleList>, length: u32, variable: ControlledOption, @@ -1242,6 +1287,52 @@ impl DisplayWithPartialPaths for PartialScopeStack { } } +#[repr(C)] +#[derive(Clone, Copy, Eq, Hash, PartialEq)] +pub struct NichePartialScopeStack { + scopes: ControlledOption>>, + length: u32, + variable: ControlledOption, +} + +impl Niche for PartialScopeStack { + type Output = NichePartialScopeStack; + + #[inline] + fn none() -> Self::Output { + NichePartialScopeStack { + scopes: ControlledOption::none(), + length: 0, + variable: ControlledOption::none(), + } + } + + #[inline] + fn is_none(value: &Self::Output) -> bool { + value.scopes.is_none() + } + + #[inline] + fn into_some(value: Self) -> Self::Output { + NichePartialScopeStack { + scopes: ControlledOption::from(value.scopes), + length: value.length, + variable: value.variable, + } + } + + #[inline] + fn from_some(value: Self::Output) -> Self { + PartialScopeStack { + scopes: value.scopes.into_option().expect( + "Niche::from_some called on none value of ControlledOption", + ), + length: value.length, + variable: value.variable, + } + } +} + //------------------------------------------------------------------------------------------------- // Partial symbol bindings @@ -1645,6 +1736,48 @@ impl DisplayWithPartialPaths for PartialPathEdgeList { } } +#[repr(C)] +#[derive(Clone, Copy, Eq, Hash, PartialEq)] +pub struct NichePartialPathEdgeList { + edges: ControlledOption>, + length: u32, +} + +impl Niche for PartialPathEdgeList { + type Output = NichePartialPathEdgeList; + + #[inline] + fn none() -> Self::Output { + NichePartialPathEdgeList { + edges: ControlledOption::none(), + length: 0, + } + } + + #[inline] + fn is_none(value: &Self::Output) -> bool { + value.edges.is_none() + } + + #[inline] + fn into_some(value: Self) -> Self::Output { + NichePartialPathEdgeList { + edges: ControlledOption::from(value.edges), + length: value.length, + } + } + + #[inline] + fn from_some(value: Self::Output) -> Self { + PartialPathEdgeList { + edges: value.edges.into_option().expect( + "Niche::from_some called on none value of ControlledOption", + ), + length: value.length, + } + } +} + //------------------------------------------------------------------------------------------------- // Partial paths From 1f2cb0a95519f2a8a7a597052bc6846c2f73b4b2 Mon Sep 17 00:00:00 2001 From: Hendrik van Antwerpen Date: Tue, 14 Mar 2023 11:49:36 +0100 Subject: [PATCH 09/12] Run arena tests against both implementations --- stack-graphs/tests/it/arena.rs | 301 +++++++++++++++++---------------- 1 file changed, 158 insertions(+), 143 deletions(-) diff --git a/stack-graphs/tests/it/arena.rs b/stack-graphs/tests/it/arena.rs index 6364deb7d..1e611f73d 100644 --- a/stack-graphs/tests/it/arena.rs +++ b/stack-graphs/tests/it/arena.rs @@ -30,6 +30,20 @@ fn can_allocate_in_arena() { assert_ne!(arena.get(hello2), arena.get(there)); } +#[test] +fn can_allocate_in_interning_arena() { + let mut arena = HashArena::new(); + let hello1 = arena.add("hello".to_string()); + let hello2 = arena.add("hello".to_string()); + let there = arena.add("there".to_string()); + assert_eq!(hello1, hello2); + assert_ne!(hello1, there); + assert_ne!(hello2, there); + assert_eq!(arena.get(hello1), arena.get(hello2)); + assert_ne!(arena.get(hello1), arena.get(there)); + assert_ne!(arena.get(hello2), arena.get(there)); +} + #[test] fn can_allocate_in_supplemental_arena() { let mut arena = VecArena::::new(); @@ -50,43 +64,48 @@ fn can_create_lists() { fn collect(list: &List, arena: &impl ListArena) -> Vec { list.iter(arena).copied().collect() } - - let mut arena = List::new_arena(); - let mut list = List::empty(); - assert_eq!(collect(&list, &arena), vec![] as Vec); - list.push_front(&mut arena, 1); - assert_eq!(collect(&list, &arena), vec![1]); - list.push_front(&mut arena, 2); - list.push_front(&mut arena, 3); - assert_eq!(collect(&list, &arena), vec![3, 2, 1]); + fn run(mut arena: impl ListArena) { + let mut list = List::empty(); + assert_eq!(collect(&list, &arena), vec![] as Vec); + list.push_front(&mut arena, 1); + assert_eq!(collect(&list, &arena), vec![1]); + list.push_front(&mut arena, 2); + list.push_front(&mut arena, 3); + assert_eq!(collect(&list, &arena), vec![3, 2, 1]); + } + run(VecArena::new()); + run(HashArena::new()); } #[test] fn can_compare_lists() { - use std::cmp::Ordering; - let mut arena = List::new_arena(); - let mut from_slice = |slice: &[u32]| { - let mut list = List::empty(); - for element in slice.iter().rev() { - list.push_front(&mut arena, *element); - } - list - }; - let list0 = from_slice(&[]); - let list1 = from_slice(&[1]); - let list2 = from_slice(&[2]); - let list12 = from_slice(&[1, 2]); - assert!(list0.equals(&arena, list0)); - assert_eq!(list0.cmp(&arena, list0), Ordering::Equal); - assert!(!list0.equals(&arena, list1)); - assert_eq!(list0.cmp(&arena, list1), Ordering::Less); - assert!(list1.equals(&arena, list1)); - assert_eq!(list1.cmp(&arena, list1), Ordering::Equal); - assert!(!list1.equals(&arena, list2)); - assert_eq!(list1.cmp(&arena, list2), Ordering::Less); - assert!(list2.equals(&arena, list2)); - assert_eq!(list2.cmp(&arena, list12), Ordering::Greater); - assert_eq!(list1.cmp(&arena, list12), Ordering::Less); + fn run(mut arena: impl ListArena) { + use std::cmp::Ordering; + let mut from_slice = |slice: &[u32]| { + let mut list = List::empty(); + for element in slice.iter().rev() { + list.push_front(&mut arena, *element); + } + list + }; + let list0 = from_slice(&[]); + let list1 = from_slice(&[1]); + let list2 = from_slice(&[2]); + let list12 = from_slice(&[1, 2]); + assert!(list0.equals(&arena, list0)); + assert_eq!(list0.cmp(&arena, list0), Ordering::Equal); + assert!(!list0.equals(&arena, list1)); + assert_eq!(list0.cmp(&arena, list1), Ordering::Less); + assert!(list1.equals(&arena, list1)); + assert_eq!(list1.cmp(&arena, list1), Ordering::Equal); + assert!(!list1.equals(&arena, list2)); + assert_eq!(list1.cmp(&arena, list2), Ordering::Less); + assert!(list2.equals(&arena, list2)); + assert_eq!(list2.cmp(&arena, list12), Ordering::Greater); + assert_eq!(list1.cmp(&arena, list12), Ordering::Less); + } + run(VecArena::new()); + run(HashArena::new()); } #[test] @@ -94,56 +113,61 @@ fn can_create_reversible_lists() { fn collect(list: &ReversibleList, arena: &impl ReversibleListArena) -> Vec { list.iter(arena).copied().collect() } - - let mut arena = ReversibleList::new_arena(); - let mut list = ReversibleList::empty(); - assert_eq!(collect(&list, &arena), vec![] as Vec); - list.push_front(&mut arena, 1); - assert_eq!(collect(&list, &arena), vec![1]); - list.push_front(&mut arena, 2); - list.push_front(&mut arena, 3); - assert_eq!(collect(&list, &arena), vec![3, 2, 1]); - list.reverse(&mut arena); - assert_eq!(collect(&list, &arena), vec![1, 2, 3]); - list.push_front(&mut arena, 4); - list.push_front(&mut arena, 5); - assert_eq!(collect(&list, &arena), vec![5, 4, 1, 2, 3]); - list.reverse(&mut arena); - assert_eq!(collect(&list, &arena), vec![3, 2, 1, 4, 5]); - // Verify that we stash away the re-reversal so that we don't have to recompute it. - assert!(list.have_reversal(&arena)); + fn run(mut arena: impl ReversibleListArena) { + let mut list = ReversibleList::empty(); + assert_eq!(collect(&list, &arena), vec![] as Vec); + list.push_front(&mut arena, 1); + assert_eq!(collect(&list, &arena), vec![1]); + list.push_front(&mut arena, 2); + list.push_front(&mut arena, 3); + assert_eq!(collect(&list, &arena), vec![3, 2, 1]); + list.reverse(&mut arena); + assert_eq!(collect(&list, &arena), vec![1, 2, 3]); + list.push_front(&mut arena, 4); + list.push_front(&mut arena, 5); + assert_eq!(collect(&list, &arena), vec![5, 4, 1, 2, 3]); + list.reverse(&mut arena); + assert_eq!(collect(&list, &arena), vec![3, 2, 1, 4, 5]); + // Verify that we stash away the re-reversal so that we don't have to recompute it. + assert!(list.have_reversal(&arena)); + } + run(VecArena::new()); + run(HashArena::new()); } #[test] fn can_compare_reversible_lists() { use std::cmp::Ordering; - let mut arena = ReversibleList::new_arena(); - let mut from_slice = |slice: &[u32]| { - let mut list = ReversibleList::empty(); - for element in slice.iter().rev() { - list.push_front(&mut arena, *element); - } - list - }; - let list0 = from_slice(&[]); - let list1 = from_slice(&[1]); - let list2 = from_slice(&[2]); - let list12 = from_slice(&[1, 2]); - assert!(list0.equals(&arena, list0)); - assert_eq!(list0.cmp(&arena, list0), Ordering::Equal); - assert!(!list0.equals(&arena, list1)); - assert_eq!(list0.cmp(&arena, list1), Ordering::Less); - assert!(list1.equals(&arena, list1)); - assert_eq!(list1.cmp(&arena, list1), Ordering::Equal); - assert!(!list1.equals(&arena, list2)); - assert_eq!(list1.cmp(&arena, list2), Ordering::Less); - assert!(list2.equals(&arena, list2)); - assert_eq!(list2.cmp(&arena, list12), Ordering::Greater); - assert_eq!(list1.cmp(&arena, list12), Ordering::Less); - let mut list21 = list12; - list21.reverse(&mut arena); - assert_eq!(list2.cmp(&arena, list21), Ordering::Less); - assert_eq!(list1.cmp(&arena, list21), Ordering::Less); + fn run(mut arena: impl ReversibleListArena) { + let mut from_slice = |slice: &[u32]| { + let mut list = ReversibleList::empty(); + for element in slice.iter().rev() { + list.push_front(&mut arena, *element); + } + list + }; + let list0 = from_slice(&[]); + let list1 = from_slice(&[1]); + let list2 = from_slice(&[2]); + let list12 = from_slice(&[1, 2]); + assert!(list0.equals(&arena, list0)); + assert_eq!(list0.cmp(&arena, list0), Ordering::Equal); + assert!(!list0.equals(&arena, list1)); + assert_eq!(list0.cmp(&arena, list1), Ordering::Less); + assert!(list1.equals(&arena, list1)); + assert_eq!(list1.cmp(&arena, list1), Ordering::Equal); + assert!(!list1.equals(&arena, list2)); + assert_eq!(list1.cmp(&arena, list2), Ordering::Less); + assert!(list2.equals(&arena, list2)); + assert_eq!(list2.cmp(&arena, list12), Ordering::Greater); + assert_eq!(list1.cmp(&arena, list12), Ordering::Less); + let mut list21 = list12; + list21.reverse(&mut arena); + assert_eq!(list2.cmp(&arena, list21), Ordering::Less); + assert_eq!(list1.cmp(&arena, list21), Ordering::Less); + } + run(VecArena::new()); + run(HashArena::new()); } #[test] @@ -157,39 +181,40 @@ fn can_create_deques() { fn collect_rev(deque: &Deque, arena: &mut impl DequeArena) -> Vec { deque.iter_reversed(arena).copied().collect() } - - let mut arena = Deque::new_arena(); - let mut deque = Deque::empty(); - assert_eq!(collect(&deque, &mut arena), vec![] as Vec); - assert_eq!(collect_rev(&deque, &mut arena), vec![] as Vec); - deque.push_front(&mut arena, 1); - assert_eq!(collect(&deque, &mut arena), vec![1]); - assert_eq!(collect_rev(&deque, &mut arena), vec![1]); - deque.push_front(&mut arena, 2); - deque.push_front(&mut arena, 3); - assert_eq!(collect(&deque, &mut arena), vec![3, 2, 1]); - assert_eq!(collect_rev(&deque, &mut arena), vec![1, 2, 3]); - deque.push_back(&mut arena, 4); - assert_eq!(collect(&deque, &mut arena), vec![3, 2, 1, 4]); - assert_eq!(collect_rev(&deque, &mut arena), vec![4, 1, 2, 3]); - deque.push_back(&mut arena, 5); - deque.push_back(&mut arena, 6); - assert_eq!(collect(&deque, &mut arena), vec![3, 2, 1, 4, 5, 6]); - assert_eq!(collect_rev(&deque, &mut arena), vec![6, 5, 4, 1, 2, 3]); - deque.push_front(&mut arena, 7); - deque.push_front(&mut arena, 8); - assert_eq!(collect(&deque, &mut arena), vec![8, 7, 3, 2, 1, 4, 5, 6]); - assert_eq!(collect_reused(&deque, &arena), vec![8, 7, 3, 2, 1, 4, 5, 6]); - assert_eq!( - collect_rev(&deque, &mut arena), - vec![6, 5, 4, 1, 2, 3, 7, 8] - ); + fn run(mut arena: impl ReversibleListArena) { + let mut deque = Deque::empty(); + assert_eq!(collect(&deque, &mut arena), vec![] as Vec); + assert_eq!(collect_rev(&deque, &mut arena), vec![] as Vec); + deque.push_front(&mut arena, 1); + assert_eq!(collect(&deque, &mut arena), vec![1]); + assert_eq!(collect_rev(&deque, &mut arena), vec![1]); + deque.push_front(&mut arena, 2); + deque.push_front(&mut arena, 3); + assert_eq!(collect(&deque, &mut arena), vec![3, 2, 1]); + assert_eq!(collect_rev(&deque, &mut arena), vec![1, 2, 3]); + deque.push_back(&mut arena, 4); + assert_eq!(collect(&deque, &mut arena), vec![3, 2, 1, 4]); + assert_eq!(collect_rev(&deque, &mut arena), vec![4, 1, 2, 3]); + deque.push_back(&mut arena, 5); + deque.push_back(&mut arena, 6); + assert_eq!(collect(&deque, &mut arena), vec![3, 2, 1, 4, 5, 6]); + assert_eq!(collect_rev(&deque, &mut arena), vec![6, 5, 4, 1, 2, 3]); + deque.push_front(&mut arena, 7); + deque.push_front(&mut arena, 8); + assert_eq!(collect(&deque, &mut arena), vec![8, 7, 3, 2, 1, 4, 5, 6]); + assert_eq!(collect_reused(&deque, &arena), vec![8, 7, 3, 2, 1, 4, 5, 6]); + assert_eq!( + collect_rev(&deque, &mut arena), + vec![6, 5, 4, 1, 2, 3, 7, 8] + ); + } + run(VecArena::new()); + run(HashArena::new()); } #[test] fn can_compare_deques() { use std::cmp::Ordering; - let mut arena = Deque::new_arena(); // Build up deques in both directions so that our comparisons have to test the "reverse if // needed" logic. fn from_slice_forwards(slice: &[u32], arena: &mut impl DequeArena) -> Deque { @@ -206,42 +231,32 @@ fn can_compare_deques() { } deque } - let deque0 = from_slice_forwards(&[], &mut arena); - let mut deque1 = from_slice_backwards(&[1], &mut arena); - let deque2 = from_slice_forwards(&[2], &mut arena); - let mut deque10 = from_slice_backwards(&[1, 0], &mut arena); - let deque12 = from_slice_backwards(&[1, 2], &mut arena); - assert!(deque0.equals(&mut arena, deque0)); - assert_eq!(deque0.cmp(&mut arena, deque0), Ordering::Equal); - assert!(!deque0.equals(&mut arena, deque1)); - assert_eq!(deque0.cmp(&mut arena, deque1), Ordering::Less); - assert!(deque1.equals(&mut arena, deque1)); - assert_eq!(deque1.cmp(&mut arena, deque1), Ordering::Equal); - assert!(!deque1.equals(&mut arena, deque2)); - assert_eq!(deque1.cmp(&mut arena, deque2), Ordering::Less); - assert!(deque2.equals(&mut arena, deque2)); - assert_eq!(deque2.cmp(&mut arena, deque12), Ordering::Greater); - assert_eq!(deque1.cmp(&mut arena, deque12), Ordering::Less); - - // We should get the same result regardless of which direction the deques are pointing. - deque1.ensure_forwards(&mut arena); - deque10.ensure_forwards(&mut arena); - assert_eq!(deque1.cmp(&mut arena, deque10), Ordering::Less); - deque1.ensure_backwards(&mut arena); - deque10.ensure_backwards(&mut arena); - assert_eq!(deque1.cmp(&mut arena, deque10), Ordering::Less); -} + fn run(mut arena: impl ReversibleListArena) { + let deque0 = from_slice_forwards(&[], &mut arena); + let mut deque1 = from_slice_backwards(&[1], &mut arena); + let deque2 = from_slice_forwards(&[2], &mut arena); + let mut deque10 = from_slice_backwards(&[1, 0], &mut arena); + let deque12 = from_slice_backwards(&[1, 2], &mut arena); + assert!(deque0.equals(&mut arena, deque0)); + assert_eq!(deque0.cmp(&mut arena, deque0), Ordering::Equal); + assert!(!deque0.equals(&mut arena, deque1)); + assert_eq!(deque0.cmp(&mut arena, deque1), Ordering::Less); + assert!(deque1.equals(&mut arena, deque1)); + assert_eq!(deque1.cmp(&mut arena, deque1), Ordering::Equal); + assert!(!deque1.equals(&mut arena, deque2)); + assert_eq!(deque1.cmp(&mut arena, deque2), Ordering::Less); + assert!(deque2.equals(&mut arena, deque2)); + assert_eq!(deque2.cmp(&mut arena, deque12), Ordering::Greater); + assert_eq!(deque1.cmp(&mut arena, deque12), Ordering::Less); -#[test] -fn can_allocate_in_interning_arena() { - let mut arena = HashArena::new(); - let hello1 = arena.add("hello".to_string()); - let hello2 = arena.add("hello".to_string()); - let there = arena.add("there".to_string()); - assert_eq!(hello1, hello2); - assert_ne!(hello1, there); - assert_ne!(hello2, there); - assert_eq!(arena.get(hello1), arena.get(hello2)); - assert_ne!(arena.get(hello1), arena.get(there)); - assert_ne!(arena.get(hello2), arena.get(there)); + // We should get the same result regardless of which direction the deques are pointing. + deque1.ensure_forwards(&mut arena); + deque10.ensure_forwards(&mut arena); + assert_eq!(deque1.cmp(&mut arena, deque10), Ordering::Less); + deque1.ensure_backwards(&mut arena); + deque10.ensure_backwards(&mut arena); + assert_eq!(deque1.cmp(&mut arena, deque10), Ordering::Less); + } + run(VecArena::new()); + run(HashArena::new()); } From b4b84ea3bc36f6b33398d2ae6cf07753e3c6652a Mon Sep 17 00:00:00 2001 From: Hendrik van Antwerpen Date: Tue, 14 Mar 2023 12:00:28 +0100 Subject: [PATCH 10/12] Switch to HashArena for PartialPaths --- stack-graphs/src/partial.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/stack-graphs/src/partial.rs b/stack-graphs/src/partial.rs index 11e7c9746..790daedd6 100644 --- a/stack-graphs/src/partial.rs +++ b/stack-graphs/src/partial.rs @@ -47,8 +47,8 @@ use smallvec::SmallVec; use crate::arena::Deque; use crate::arena::DequeCell; use crate::arena::Handle; +use crate::arena::HashArena; use crate::arena::ReversibleList; -use crate::arena::VecArena; use crate::cycles::Appendables; use crate::cycles::AppendingCycleDetector; use crate::graph::Edge; @@ -2853,17 +2853,17 @@ struct Join { /// Manages the state of a collection of partial paths built up as part of the partial-path-finding /// algorithm or path-stitching algorithm. pub struct PartialPaths { - pub(crate) partial_symbol_stacks: VecArena>, - pub(crate) partial_scope_stacks: VecArena>>, - pub(crate) partial_path_edges: VecArena>, + pub(crate) partial_symbol_stacks: HashArena>, + pub(crate) partial_scope_stacks: HashArena>>, + pub(crate) partial_path_edges: HashArena>, } impl PartialPaths { pub fn new() -> PartialPaths { PartialPaths { - partial_symbol_stacks: VecArena::new(), - partial_scope_stacks: VecArena::new(), - partial_path_edges: VecArena::new(), + partial_symbol_stacks: HashArena::new(), + partial_scope_stacks: HashArena::new(), + partial_path_edges: HashArena::new(), } } } From 5f9f4ae10fd1f68f182a220db50b20667497978c Mon Sep 17 00:00:00 2001 From: Hendrik van Antwerpen Date: Tue, 14 Mar 2023 12:02:06 +0100 Subject: [PATCH 11/12] Use HashArena for symbol stack keys --- stack-graphs/src/stitching.rs | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/stack-graphs/src/stitching.rs b/stack-graphs/src/stitching.rs index 4865a69db..7987f7abc 100644 --- a/stack-graphs/src/stitching.rs +++ b/stack-graphs/src/stitching.rs @@ -35,7 +35,6 @@ //! [`Database`]: struct.Database.html //! [`PathStitcher`]: struct.PathStitcher.html -use std::collections::HashMap; use std::collections::VecDeque; #[cfg(feature = "copious-debugging")] use std::fmt::Display; @@ -44,6 +43,7 @@ use std::ops::Index; use crate::arena::Arena; use crate::arena::Handle; use crate::arena::HandleSet; +use crate::arena::HashArena; use crate::arena::List; use crate::arena::ListCell; use crate::arena::SupplementalArena; @@ -76,8 +76,7 @@ use crate::CancellationFlag; pub struct Database { pub(crate) partial_paths: VecArena, pub(crate) local_nodes: HandleSet, - symbol_stack_keys: VecArena>>, - symbol_stack_key_cache: HashMap, + symbol_stack_keys: HashArena>>, paths_by_start_node: SupplementalArena>>, root_paths_by_precondition: SupplementalArena>>, } @@ -88,8 +87,7 @@ impl Database { Database { partial_paths: VecArena::new(), local_nodes: HandleSet::new(), - symbol_stack_keys: VecArena::new(), - symbol_stack_key_cache: HashMap::new(), + symbol_stack_keys: HashArena::new(), paths_by_start_node: SupplementalArena::new(), root_paths_by_precondition: SupplementalArena::new(), } @@ -362,18 +360,7 @@ impl SymbolStackKey { /// Pushes a new symbol onto the back of this symbol stack key. fn push_back(&mut self, db: &mut Database, symbol: Handle) { - let cache_key = SymbolStackCacheKey { - head: symbol, - tail: self.back_handle(), - }; - if let Some(handle) = db.symbol_stack_key_cache.get(&cache_key) { - self.symbols = List::from_handle(*handle); - return; - } - // push_front because we store the key's symbols in reverse order. self.symbols.push_front(&mut db.symbol_stack_keys, symbol); - let handle = self.back_handle(); - db.symbol_stack_key_cache.insert(cache_key, handle); } /// Pops a symbol from the back of this symbol stack key. From 1caa5ee2d40d4a20bcde87f81a70d880068f39b5 Mon Sep 17 00:00:00 2001 From: Hendrik van Antwerpen Date: Fri, 17 Mar 2023 15:07:58 +0100 Subject: [PATCH 12/12] Use handle equality --- stack-graphs/src/arena.rs | 20 ++++++++++++++ stack-graphs/src/partial.rs | 52 +++++++++++-------------------------- 2 files changed, 35 insertions(+), 37 deletions(-) diff --git a/stack-graphs/src/arena.rs b/stack-graphs/src/arena.rs index a658732ae..383716e46 100644 --- a/stack-graphs/src/arena.rs +++ b/stack-graphs/src/arena.rs @@ -204,6 +204,10 @@ pub trait Arena { /// Returns the number of instances stored in this arena. fn len(&self) -> usize; + + /// Returns whether the values associated with the arena are equal, if this can be determined from the + /// handles alone. Otherwise, no value is returned. + fn try_equals(&self, lhs: Handle, rhs: Handle) -> Option; } /// Manages the life cycle of instances of type `T`. You can allocate new instances of `T` from @@ -261,6 +265,11 @@ impl Arena for VecArena { fn len(&self) -> usize { self.items.len() } + + #[inline] + fn try_equals(&self, _lhs: Handle, _rhs: Handle) -> Option { + None + } } /// An interning arena allocation. Equal handles means equal elements. @@ -320,6 +329,11 @@ where fn len(&self) -> usize { self.arena.len() } + + #[inline] + fn try_equals(&self, lhs: Handle, rhs: Handle) -> Option { + Some(lhs == rhs) + } } //------------------------------------------------------------------------------------------------- @@ -672,6 +686,9 @@ where T: Eq, { pub fn equals(self, arena: &impl ListArena, other: List) -> bool { + if let Some(equals) = arena.try_equals(self.cells, other.cells) { + return equals; + } self.equals_with(arena, other, |a, b| *a == *b) } } @@ -987,6 +1004,9 @@ where T: Eq, { pub fn equals(self, arena: &impl ReversibleListArena, other: ReversibleList) -> bool { + if let Some(equals) = arena.try_equals(self.cells, other.cells) { + return equals; + } self.equals_with(arena, other, |a, b| *a == *b) } } diff --git a/stack-graphs/src/partial.rs b/stack-graphs/src/partial.rs index 790daedd6..b6ccb0356 100644 --- a/stack-graphs/src/partial.rs +++ b/stack-graphs/src/partial.rs @@ -491,7 +491,7 @@ impl DisplayWithPartialPaths for PartialScopedSymbol { /// A pattern that might match against a symbol stack. Consists of a (possibly empty) list of /// partial scoped symbols, along with an optional symbol stack variable. #[repr(C)] -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Eq, Hash, PartialEq)] pub struct PartialSymbolStack { symbols: ReversibleList, length: u32, @@ -791,24 +791,13 @@ impl PartialSymbolStack { unreachable!(); } - pub fn equals(mut self, partials: &mut PartialPaths, mut other: PartialSymbolStack) -> bool { - while let Some(self_symbol) = self.pop_front(partials) { - if let Some(other_symbol) = other.pop_front(partials) { - if !self_symbol.equals(partials, &other_symbol) { - return false; - } - } else { - return false; - } - } - if !other.symbols.is_empty() { - return false; - } - equals_option( - self.variable.into_option(), - other.variable.into_option(), - |a, b| a == b, - ) + pub fn equals(self, _partials: &mut PartialPaths, other: PartialSymbolStack) -> bool { + self.symbols == other.symbols + && equals_option( + self.variable.into_option(), + other.variable.into_option(), + |a, b| a == b, + ) } pub fn cmp( @@ -1204,11 +1193,8 @@ impl PartialScopeStack { self.variable.into_option() } - pub fn equals(self, partials: &mut PartialPaths, other: PartialScopeStack) -> bool { - self.scopes - .equals_with(&mut partials.partial_scope_stacks, other.scopes, |a, b| { - *a == *b - }) + pub fn equals(self, _partials: &mut PartialPaths, other: PartialScopeStack) -> bool { + self.scopes == other.scopes && equals_option( self.variable.into_option(), other.variable.into_option(), @@ -1852,21 +1838,13 @@ impl PartialPath { self.edges.shadows(partials, other.edges) } - pub fn equals(&self, partials: &mut PartialPaths, other: &PartialPath) -> bool { + pub fn equals(&self, _partials: &mut PartialPaths, other: &PartialPath) -> bool { self.start_node == other.start_node && self.end_node == other.end_node - && self - .symbol_stack_precondition - .equals(partials, other.symbol_stack_precondition) - && self - .symbol_stack_postcondition - .equals(partials, other.symbol_stack_postcondition) - && self - .scope_stack_precondition - .equals(partials, other.scope_stack_precondition) - && self - .scope_stack_postcondition - .equals(partials, other.scope_stack_postcondition) + && self.symbol_stack_precondition == other.symbol_stack_precondition + && self.symbol_stack_postcondition == other.symbol_stack_postcondition + && self.scope_stack_precondition == other.scope_stack_precondition + && self.scope_stack_postcondition == other.scope_stack_postcondition } pub fn cmp(