diff --git a/tree_arena/src/tree_arena_safe.rs b/tree_arena/src/tree_arena_safe.rs index c325c19476..b8c7a3d1ff 100644 --- a/tree_arena/src/tree_arena_safe.rs +++ b/tree_arena/src/tree_arena_safe.rs @@ -77,7 +77,6 @@ pub struct ArenaMut<'arena, T> { pub struct ArenaRefList<'arena, T> { parent_id: Option, children: &'arena HashMap>, - parents_map: ArenaMapRef<'arena>, } /// A handle giving mutable access to a set of arena items. @@ -87,16 +86,15 @@ pub struct ArenaRefList<'arena, T> { pub struct ArenaMutList<'arena, T> { parent_id: Option, children: &'arena mut HashMap>, - parents_map: ArenaMapMut<'arena>, } -/// A shared reference to the parent father map +/// A shared reference to the parent map #[derive(Clone, Copy, Debug)] pub struct ArenaMapRef<'arena> { parents_map: &'arena HashMap>, } -/// A mutable reference to the parent father map +/// A mutable reference to the parent map #[derive(Debug)] pub struct ArenaMapMut<'arena> { parents_map: &'arena mut HashMap>, @@ -130,14 +128,16 @@ impl TreeArena { } /// Returns a handle giving access to the roots of the tree. - pub fn roots(&self) -> ArenaRefList<'_, T> { - ArenaRefList { - parent_id: None, - children: &self.roots, - parents_map: ArenaMapRef { + pub fn roots(&self) -> (ArenaRefList<'_, T>, ArenaMapRef<'_>) { + ( + ArenaRefList { + parent_id: None, + children: &self.roots, + }, + ArenaMapRef { parents_map: &self.parents_map, }, - } + ) } /// An iterator visiting all root ids in arbitrary order. @@ -149,14 +149,16 @@ impl TreeArena { /// /// Using [`insert`](ArenaMutList::insert) on this handle /// will add a new root to the tree. - pub fn roots_mut(&mut self) -> ArenaMutList<'_, T> { - ArenaMutList { - parent_id: None, - children: &mut self.roots, - parents_map: ArenaMapMut { + pub fn roots_mut(&mut self) -> (ArenaMutList<'_, T>, ArenaMapMut<'_>) { + ( + ArenaMutList { + parent_id: None, + children: &mut self.roots, + }, + ArenaMapMut { parents_map: &mut self.parents_map, }, - } + ) } /// Find an item in the tree. @@ -166,8 +168,9 @@ impl TreeArena { /// ## Complexity /// /// O(Depth). In future implementations, this will be O(1). - pub fn find(&self, id: impl Into) -> Option> { - self.roots().find_inner(id.into()) + pub fn find(&self, id: impl Into) -> (Option>, ArenaMapRef<'_>) { + let (roots, map) = self.roots(); + (roots.find_inner(map, id.into()), map) } /// Find an item in the tree. @@ -177,8 +180,12 @@ impl TreeArena { /// ## Complexity /// /// O(Depth). In future implementations, this will be O(1). - pub fn find_mut(&mut self, id: impl Into) -> Option> { - self.roots_mut().find_mut_inner(id.into()) + pub fn find_mut( + &mut self, + id: impl Into, + ) -> (Option>, ArenaMapMut<'_>) { + let (roots, map) = self.roots_mut(); + (roots.find_mut_inner(map.reborrow(), id.into()), map) } /// Construct the path of items from the given item to the root of the tree. @@ -205,6 +212,11 @@ impl TreeArena { pub fn reparent(&mut self, child: impl Into, new_parent: impl Into) { let child_id = child.into(); let new_parent_id = new_parent.into(); + let old_parent_id = self + .parents_map + .get(&child_id) + .unwrap_or_else(|| panic!("no node found for child id #{child_id}")) + .unwrap(); assert_ne!( child_id, new_parent_id, @@ -219,56 +231,51 @@ impl TreeArena { "reparenting of root nodes is currently not supported" ); - let old_parent_id = self - .parents_map - .get(&child_id) - .unwrap_or_else(|| panic!("no node found for child id #{child_id}")) + let mut roots = ArenaMutList { + parent_id: None, + children: &mut self.roots, + }; + let mut parents_map = ArenaMapMut { + parents_map: &mut self.parents_map, + }; + + let mut parent_node = roots + .reborrow_mut() + .find_mut(parents_map.reborrow(), old_parent_id) .unwrap(); - let child_node = self - .find_mut(old_parent_id) - .unwrap() - .children + let value = parent_node .children - .remove(&child_id) + .remove(parents_map.reborrow_mut(), child_id) .unwrap(); - self.parents_map.insert(child_id, Some(new_parent_id)); - self.find_mut(new_parent_id) - .unwrap_or_else(|| panic!("no node found for new_parent id #{new_parent_id}")) - .children + + let mut new_parent_node = roots + .find_mut(parents_map.reborrow(), new_parent_id) + .unwrap(); + let _ = new_parent_node .children - .insert(child_id, child_node); + .insert(parents_map, child_id, value); } } impl TreeNode { - fn arena_ref<'arena>( - &'arena self, - parent_id: Option, - parents_map: &'arena HashMap>, - ) -> ArenaRef<'arena, T> { + fn arena_ref<'arena>(&'arena self, parent_id: Option) -> ArenaRef<'arena, T> { ArenaRef { parent_id, item: &self.item, children: ArenaRefList { parent_id: Some(self.id), children: &self.children, - parents_map: ArenaMapRef { parents_map }, }, } } - fn arena_mut<'arena>( - &'arena mut self, - parent_id: Option, - parents_map: &'arena mut HashMap>, - ) -> ArenaMut<'arena, T> { + fn arena_mut<'arena>(&'arena mut self, parent_id: Option) -> ArenaMut<'arena, T> { ArenaMut { parent_id, item: &mut self.item, children: ArenaMutList { parent_id: Some(self.id), children: &mut self.children, - parents_map: ArenaMapMut { parents_map }, }, } } @@ -319,28 +326,36 @@ impl ArenaMut<'_, T> { impl<'arena, T> ArenaRefList<'arena, T> { /// Returns `true` if the list has an element with the given id. - pub fn has(self, id: impl Into) -> bool { + pub fn has(self, _parents_map: ArenaMapRef<'_>, id: impl Into) -> bool { let id = id.into(); self.children.contains_key(&id) } /// Get a handle to the element of the list with the given id. - pub fn item(&self, id: impl Into) -> Option> { + pub fn item( + &self, + _parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { let id = id.into(); self.children .get(&id) - .map(|child| child.arena_ref(self.parent_id, self.parents_map.parents_map)) + .map(|child| child.arena_ref(self.parent_id)) } /// Get a handle to the element of the list with the given id. /// /// This is the same as [`item`](Self::item), except it consumes /// self. This is sometimes necessary to accommodate the borrow checker. - pub fn into_item(self, id: impl Into) -> Option> { + pub fn into_item( + self, + _parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { let id = id.into(); self.children .get(&id) - .map(|child| child.arena_ref(self.parent_id, self.parents_map.parents_map)) + .map(|child| child.arena_ref(self.parent_id)) } /// Find an arena item among the list's items and their descendants. @@ -350,15 +365,19 @@ impl<'arena, T> ArenaRefList<'arena, T> { /// ## Complexity /// /// O(Depth). In future implementations, this will be O(1). - pub fn find(self, id: impl Into) -> Option> { - self.find_inner(id.into()) + pub fn find( + self, + parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { + self.find_inner(parents_map, id.into()) } - fn find_inner(self, id: NodeId) -> Option> { - let parent_id = self.parents_map.parents_map.get(&id)?; + fn find_inner(self, parents_map: ArenaMapRef<'_>, id: NodeId) -> Option> { + let parent_id = parents_map.parents_map.get(&id)?; let id_path = if let Some(parent_id) = parent_id { - self.parents_map.get_id_path(*parent_id, self.parent_id) + parents_map.get_id_path(*parent_id, self.parent_id) } else { Vec::new() }; @@ -371,53 +390,89 @@ impl<'arena, T> ArenaRefList<'arena, T> { } let node = node_children.get(&id)?; - Some(node.arena_ref(*parent_id, self.parents_map.parents_map)) + Some(node.arena_ref(*parent_id)) } } impl<'arena, T> ArenaMutList<'arena, T> { /// Returns `true` if the list has an element with the given id. - pub fn has(&self, id: impl Into) -> bool { + pub fn has(&self, _parents_map: ArenaMapRef<'_>, id: impl Into) -> bool { let id = id.into(); self.children.contains_key(&id) } /// Get a shared handle to the element of the list with the given id. - pub fn item(&self, id: impl Into) -> Option> { + pub fn item( + &self, + _parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { let id = id.into(); self.children .get(&id) - .map(|child| child.arena_ref(self.parent_id, self.parents_map.parents_map)) + .map(|child| child.arena_ref(self.parent_id)) } /// Get a mutable handle to the element of the list with the given id. - pub fn item_mut(&mut self, id: impl Into) -> Option> { + pub fn item_mut( + &mut self, + _parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { let id = id.into(); self.children .get_mut(&id) - .map(|child| child.arena_mut(self.parent_id, self.parents_map.parents_map)) + .map(|child| child.arena_mut(self.parent_id)) + } + + /// Get several mutable handles to the elements of the list with the given id. + /// + /// Returns an array of length `N` with the results of each query. + /// For soundness, at most one mutable reference will be returned to any value. + /// + /// # Panics + /// + /// Panics if any ids overlap. + pub fn item_mut_disjoint( + &mut self, + _parents_map: ArenaMapRef<'_>, + ids: [impl Into; N], + ) -> [Option>; N] { + let ids = ids.map(|id| id.into()); + let ids = ids.each_ref(); + self.children + .get_many_mut(ids) + .map(|option| option.map(|child| child.arena_mut(self.parent_id))) } /// Get a shared handle to the element of the list with the given id. /// /// This is the same as [`item`](Self::item), except it consumes /// self. This is sometimes necessary to accommodate the borrow checker. - pub fn into_item(self, id: impl Into) -> Option> { + pub fn into_item( + self, + _parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { let id = id.into(); self.children .get(&id) - .map(|child| child.arena_ref(self.parent_id, self.parents_map.parents_map)) + .map(|child| child.arena_ref(self.parent_id)) } /// Get a mutable handle to the element of the list with the given id. /// /// This is the same as [`item_mut`](Self::item_mut), except it consumes /// self. This is sometimes necessary to accommodate the borrow checker. - pub fn into_item_mut(self, id: impl Into) -> Option> { + pub fn into_item_mut( + self, + _parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { let id = id.into(); self.children .get_mut(&id) - .map(|child| child.arena_mut(self.parent_id, self.parents_map.parents_map)) + .map(|child| child.arena_mut(self.parent_id)) } // TODO - Remove the child_id argument once creation of widgets is figured out. @@ -434,15 +489,18 @@ impl<'arena, T> ArenaMutList<'arena, T> { /// # Panics /// /// If the arena already contains an item with the given id. - pub fn insert(&mut self, child_id: impl Into, value: T) -> ArenaMut<'_, T> { + pub fn insert( + &mut self, + parents_map: ArenaMapMut<'_>, + child_id: impl Into, + value: T, + ) -> ArenaMut<'_, T> { let child_id = child_id.into(); assert!( - !self.parents_map.parents_map.contains_key(&child_id), + !parents_map.parents_map.contains_key(&child_id), "Key already present" ); - self.parents_map - .parents_map - .insert(child_id, self.parent_id); + parents_map.parents_map.insert(child_id, self.parent_id); self.children.insert( child_id, @@ -456,7 +514,7 @@ impl<'arena, T> ArenaMutList<'arena, T> { self.children .get_mut(&child_id) .unwrap() - .arena_mut(self.parent_id, self.parents_map.parents_map) + .arena_mut(self.parent_id) } // TODO - How to handle when a subtree is removed? @@ -470,7 +528,11 @@ impl<'arena, T> ArenaMutList<'arena, T> { /// /// This will also silently remove any recursive grandchildren of this item. #[must_use] - pub fn remove(&mut self, child_id: impl Into) -> Option { + pub fn remove( + &mut self, + parents_map: ArenaMapMut<'_>, + child_id: impl Into, + ) -> Option { let child_id = child_id.into(); let child = self.children.remove(&child_id)?; @@ -484,17 +546,29 @@ impl<'arena, T> ArenaMutList<'arena, T> { parents_map.remove(&node.id); } - remove_children_from_map(&child, self.parents_map.parents_map); + remove_children_from_map(&child, parents_map.parents_map); Some(child.item) } + pub fn reparent( + &mut self, + mut parents_map: ArenaMapMut<'_>, + mut from: ArenaMutList<'_, T>, + id: impl Into, + ) { + let id = id.into(); + let Some(child) = from.remove(parents_map.reborrow_mut(), id) else { + return; + }; + let _ = self.insert(parents_map, id, child); + } + /// Returns a shared handle equivalent to this one. pub fn reborrow(&self) -> ArenaRefList<'_, T> { ArenaRefList { parent_id: self.parent_id, children: &*self.children, - parents_map: self.parents_map.reborrow(), } } @@ -505,7 +579,6 @@ impl<'arena, T> ArenaMutList<'arena, T> { ArenaMutList { parent_id: self.parent_id, children: &mut *self.children, - parents_map: self.parents_map.reborrow_mut(), } } @@ -516,8 +589,12 @@ impl<'arena, T> ArenaMutList<'arena, T> { /// ## Complexity /// /// O(Depth). - pub fn find(&self, id: impl Into) -> Option> { - self.reborrow().find(id) + pub fn find( + &self, + parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { + self.reborrow().find(parents_map, id) } /// Find an arena item among the list's items and their descendants. @@ -527,15 +604,23 @@ impl<'arena, T> ArenaMutList<'arena, T> { /// ## Complexity /// /// O(Depth). - pub fn find_mut(self, id: impl Into) -> Option> { - self.find_mut_inner(id.into()) + pub fn find_mut( + self, + parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { + self.find_mut_inner(parents_map, id.into()) } - fn find_mut_inner(self, id: NodeId) -> Option> { - let parent_id = self.parents_map.parents_map.get(&id)?; + fn find_mut_inner( + self, + parents_map: ArenaMapRef<'_>, + id: NodeId, + ) -> Option> { + let parent_id = parents_map.parents_map.get(&id)?; let id_path = if let Some(parent_id) = parent_id { - self.parents_map.get_id_path(*parent_id, self.parent_id) + parents_map.get_id_path(*parent_id, self.parent_id) } else { Vec::new() }; @@ -548,7 +633,7 @@ impl<'arena, T> ArenaMutList<'arena, T> { } let node = node_children.get_mut(&id)?; - Some(node.arena_mut(*parent_id, &mut *self.parents_map.parents_map)) + Some(node.arena_mut(*parent_id)) } /// No-op. Added for parity with unsafe implementation. @@ -574,7 +659,7 @@ impl ArenaMapRef<'_> { let mut path = Vec::new(); if !self.parents_map.contains_key(&id) { - return path; + return Vec::new(); } let mut current_id = Some(id); @@ -589,8 +674,9 @@ impl ArenaMapRef<'_> { } } + // We've gone all the way to the root without finding start_id. if current_id != start_id { - path.clear(); + return Vec::new(); } path diff --git a/tree_arena/src/tree_arena_unsafe.rs b/tree_arena/src/tree_arena_unsafe.rs index 976947e521..08ab740dee 100644 --- a/tree_arena/src/tree_arena_unsafe.rs +++ b/tree_arena/src/tree_arena_unsafe.rs @@ -19,8 +19,6 @@ struct TreeNode { struct DataMap { /// The items in the tree items: HashMap>>>, - /// The parent of each node, or `None` if it is the root - parents: HashMap>, } /// A container type for a tree of items. @@ -35,6 +33,8 @@ pub struct TreeArena { data_map: DataMap, /// The roots of the tree roots: Vec, + /// The parent of each node, or `None` if it is the root + parents: HashMap>, } /// A reference type giving shared access to an arena item and its children. @@ -97,6 +97,20 @@ pub struct ArenaMutList<'arena, T> { child_arr: &'arena mut Vec, } +/// A shared reference to the parent map +#[derive(Clone, Copy, Debug)] +pub struct ArenaMapRef<'arena> { + parents_map: &'arena HashMap>, +} + +/// A mutable reference to the parent map +#[derive(Debug)] +pub struct ArenaMapMut<'arena> { + parents_map: &'arena mut HashMap>, +} + +// --- + impl Clone for ArenaRef<'_, Item> { fn clone(&self) -> Self { *self @@ -117,7 +131,6 @@ impl DataMap { fn new() -> Self { Self { items: HashMap::new(), - parents: HashMap::new(), } } @@ -138,8 +151,8 @@ impl DataMap { /// Returns a shared reference to the item if present. /// /// Time Complexity O(1) - fn find_inner(&self, id: NodeId) -> Option> { - let parent_id = *self.parents.get(&id)?; + fn find_inner(&self, parents_map: ArenaMapRef<'_>, id: NodeId) -> Option> { + let parent_id = *parents_map.parents_map.get(&id)?; let TreeNode { item, .. } = self.find_node(id)?; @@ -160,8 +173,12 @@ impl DataMap { /// Returns a mutable reference to the item if present. /// /// Time Complexity O(1) - fn find_mut_inner(&mut self, id: NodeId) -> Option> { - let parent_id = *self.parents.get(&id)?; + fn find_mut_inner( + &mut self, + parents_map: ArenaMapRef<'_>, + id: NodeId, + ) -> Option> { + let parent_id = *parents_map.parents_map.get(&id)?; let node_cell = self.items.get(&id)?; // SAFETY @@ -187,39 +204,6 @@ impl DataMap { children, }) } - - /// Construct the path of items from the given item to the root of the tree. - /// - /// The path is in order from the bottom to the top, starting at the given item and ending at - /// the root. - /// - /// If `start_id` is Some, the path ends just before that id instead; `start_id` is not included. - /// - /// If there is no path from `start_id` to id, returns the empty vector. - fn get_id_path(&self, id: NodeId, start_id: Option) -> Vec { - let mut path = Vec::new(); - - if !self.parents.contains_key(&id) { - return path; - } - - let mut current_id = Some(id); - while let Some(current) = current_id { - path.push(current); - current_id = *self.parents.get(¤t).unwrap(); - if current_id == start_id { - break; - } - } - - // current_id was the last parent node - // as such if current id is not start_id - // we have gone to the root and we empty the vec - if current_id != start_id { - path.clear(); - } - path - } } impl TreeArena { @@ -227,16 +211,22 @@ impl TreeArena { pub fn new() -> Self { Self { data_map: DataMap::new(), + parents: HashMap::new(), roots: Vec::new(), } } /// Returns a handle whose children are the roots, if any, of the tree. - pub fn roots(&self) -> ArenaRefList<'_, T> { - ArenaRefList { - parent_arena: &self.data_map, - parent_id: None, - } + pub fn roots(&self) -> (ArenaRefList<'_, T>, ArenaMapRef<'_>) { + ( + ArenaRefList { + parent_arena: &self.data_map, + parent_id: None, + }, + ArenaMapRef { + parents_map: &self.parents, + }, + ) } /// An iterator visiting all root ids in arbitrary order. @@ -248,14 +238,19 @@ impl TreeArena { /// /// Using [`insert`](ArenaMutList::insert) on this handle /// will add a new root to the tree. - pub fn roots_mut(&mut self) -> ArenaMutList<'_, T> { + pub fn roots_mut(&mut self) -> (ArenaMutList<'_, T>, ArenaMapMut<'_>) { // safe as the roots are derived from the arena itself (same as safety for find for non root nodes) let roots = &mut self.roots; - ArenaMutList { - parent_arena: &mut self.data_map, - parent_id: None, - child_arr: roots, - } + ( + ArenaMutList { + parent_arena: &mut self.data_map, + parent_id: None, + child_arr: roots, + }, + ArenaMapMut { + parents_map: &mut self.parents, + }, + ) } /// Find an item in the tree. @@ -265,16 +260,21 @@ impl TreeArena { /// ## Complexity /// /// O(1). - pub fn find(&self, id: impl Into) -> Option> { - self.data_map.find_inner(id.into()) + pub fn find(&self, id: impl Into) -> (Option>, ArenaMapRef<'_>) { + let (roots, map) = self.roots(); + (roots.find(map, id.into()), map) } /// Find an item in the tree. /// /// Returns a mutable reference to the item if present. - pub fn find_mut(&mut self, id: impl Into) -> Option> { + pub fn find_mut( + &mut self, + id: impl Into, + ) -> (Option>, ArenaMapMut<'_>) { // safe as derived from the arena itself and has assoc lifetime with the arena - self.data_map.find_mut_inner(id.into()) + let (roots, map) = self.roots_mut(); + (roots.find_mut(map.reborrow(), id.into()), map) } /// Construct the path of items from the given item to the root of the tree. @@ -284,7 +284,10 @@ impl TreeArena { /// /// If the id is not in the tree, returns an empty vector. pub fn get_id_path(&self, id: impl Into) -> Vec { - self.data_map.get_id_path(id.into(), None) + let parents_map = ArenaMapRef { + parents_map: &self.parents, + }; + parents_map.get_id_path(id.into(), None) } /// Moves the given child (along with all its children) to the new parent. @@ -319,29 +322,34 @@ impl TreeArena { ); let old_parent_id = self - .data_map .parents .get(&child_id) .unwrap_or_else(|| panic!("no node found for child id #{child_id}")) .unwrap(); + let arena_map = ArenaMapRef { + parents_map: &self.parents, + }; + // Remove child from old parent's children. - self.find_mut(old_parent_id) + self.data_map + .find_mut_inner(arena_map, old_parent_id) .unwrap() .children .child_arr .retain(|i| *i != child_id); - // Update parent reference. - self.data_map.parents.insert(child_id, Some(new_parent_id)); - // Add child to new parent's children. // Safe because we checked that the new parent exists and is not a child of the to-be-reparented node - self.find_mut(new_parent_id) + self.data_map + .find_mut_inner(arena_map, new_parent_id) .unwrap_or_else(|| panic!("no node found for child id #{new_parent_id}")) .children .child_arr .push(child_id); + + // Update parent reference. + self.parents.insert(child_id, Some(new_parent_id)); } } @@ -377,30 +385,27 @@ impl<'arena, T> ArenaRefList<'arena, T> { /// Check if id is a descendant of self /// O(depth) and the limiting factor for find methods /// not from the root - fn is_descendant(&self, id: NodeId) -> bool { - if self.parent_arena.items.contains_key(&id) { - // the id of the parent - let parent_id = self.parent_id; - - // The arena is derived from the root, and the id is in the tree - if parent_id.is_none() { - return true; - } - - // iff the path is empty, there is no path from id to self - !self.parent_arena.get_id_path(id, parent_id).is_empty() - } else { + fn is_descendant(&self, parents_map: ArenaMapRef<'_>, id: NodeId) -> bool { + if !self.parent_arena.items.contains_key(&id) { // if the id is not in the tree, it is not a descendant - false + return false; + } + + // The arena is derived from the root, and the id is in the tree + if self.parent_id.is_none() { + return true; } + + // iff the path is empty, there is no path from id to self + !parents_map.get_id_path(id, self.parent_id).is_empty() } /// Returns `true` if the list has an element with the given id. - pub fn has(&self, id: impl Into) -> bool { + pub fn has(&self, parents_map: ArenaMapRef<'_>, id: impl Into) -> bool { let child_id = id.into(); let parent_id = self.parent_id; - self.parent_arena - .parents + parents_map + .parents_map .get(&child_id) .map(|parent| *parent == parent_id) // check if the parent of child is the same as the parent of the arena .unwrap_or_default() @@ -409,10 +414,14 @@ impl<'arena, T> ArenaRefList<'arena, T> { /// Get a shared handle to the element of the list with the given id. /// /// Return a new [`ArenaRef`] - pub fn item(&self, id: impl Into) -> Option> { + pub fn item( + &self, + parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { let id = id.into(); - if self.has(id) { - self.parent_arena.find_inner(id) + if self.has(parents_map, id) { + self.parent_arena.find_inner(parents_map, id) } else { None } @@ -422,10 +431,14 @@ impl<'arena, T> ArenaRefList<'arena, T> { /// /// This is the same as [`item`](Self::item), except it consumes the /// handle. This is sometimes necessary to accommodate the borrow checker. - pub fn into_item(self, id: impl Into) -> Option> { + pub fn into_item( + self, + parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { let id = id.into(); - if self.has(id) { - self.parent_arena.find_inner(id) + if self.has(parents_map, id) { + self.parent_arena.find_inner(parents_map, id) } else { None } @@ -438,12 +451,16 @@ impl<'arena, T> ArenaRefList<'arena, T> { /// ## Complexity /// /// O(Depth). except access from root which is O(1). - pub fn find(self, id: impl Into) -> Option> { + pub fn find( + self, + parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { // the id to search for let id: NodeId = id.into(); - if self.is_descendant(id) { - self.parent_arena.find_inner(id) + if self.is_descendant(parents_map, id) { + self.parent_arena.find_inner(parents_map, id) } else { None } @@ -483,23 +500,27 @@ impl<'arena, T> ArenaMutList<'arena, T> { /// Check if id is a descendant of self /// O(depth) and the limiting factor for find methods /// not from the root - fn is_descendant(&self, id: NodeId) -> bool { - self.reborrow().is_descendant(id) + fn is_descendant(&self, parents_map: ArenaMapRef<'_>, id: NodeId) -> bool { + self.reborrow().is_descendant(parents_map, id) } /// Returns `true` if the list has an element with the given id. - pub fn has(&self, id: impl Into) -> bool { - self.reborrow().has(id) + pub fn has(&self, parents_map: ArenaMapRef<'_>, id: impl Into) -> bool { + self.reborrow().has(parents_map, id) } /// Get a shared handle to the element of the list with the given id. /// /// Returns a tuple of a mutable reference to the child and a handle to access /// its children. - pub fn item(&self, id: impl Into) -> Option> { + pub fn item( + &self, + parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { let id = id.into(); - if self.has(id) { - self.parent_arena.find_inner(id) + if self.has(parents_map, id) { + self.parent_arena.find_inner(parents_map, id) } else { None } @@ -509,11 +530,15 @@ impl<'arena, T> ArenaMutList<'arena, T> { /// /// Returns a tuple of a mutable reference to the child and a handle to access /// its children. - pub fn item_mut(&mut self, id: impl Into) -> Option> { + pub fn item_mut( + &mut self, + parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { let id = id.into(); - if self.has(id) { + if self.has(parents_map, id) { // safe as we check the node is a direct child node - self.parent_arena.find_mut_inner(id) + self.parent_arena.find_mut_inner(parents_map, id) } else { None } @@ -523,10 +548,14 @@ impl<'arena, T> ArenaMutList<'arena, T> { /// /// This is the same as [`item`](Self::item), except it consumes the /// handle. This is sometimes necessary to accommodate the borrow checker. - pub fn into_item(self, id: impl Into) -> Option> { + pub fn into_item( + self, + parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { let id = id.into(); - if self.has(id) { - self.parent_arena.find_inner(id) + if self.has(parents_map, id) { + self.parent_arena.find_inner(parents_map, id) } else { None } @@ -536,11 +565,15 @@ impl<'arena, T> ArenaMutList<'arena, T> { /// /// This is the same as [`item_mut`](Self::item_mut), except it consumes /// the handle. This is sometimes necessary to accommodate the borrow checker. - pub fn into_item_mut(self, id: impl Into) -> Option> { + pub fn into_item_mut( + self, + parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { let id = id.into(); - if self.has(id) { + if self.has(parents_map, id) { // safe as we check the node is a direct child node - self.parent_arena.find_mut_inner(id) + self.parent_arena.find_mut_inner(parents_map, id) } else { None } @@ -560,14 +593,19 @@ impl<'arena, T> ArenaMutList<'arena, T> { /// # Panics /// /// If the arena already contains an item with the given id. - pub fn insert(&mut self, child_id: impl Into, value: T) -> ArenaMut<'_, T> { + pub fn insert( + &mut self, + parents_map: ArenaMapMut<'_>, + child_id: impl Into, + value: T, + ) -> ArenaMut<'_, T> { let child_id: NodeId = child_id.into(); assert!( - !self.parent_arena.parents.contains_key(&child_id), + !parents_map.parents_map.contains_key(&child_id), "Key already present" ); - self.parent_arena.parents.insert(child_id, self.parent_id); + parents_map.parents_map.insert(child_id, self.parent_id); self.child_arr.push(child_id); @@ -580,7 +618,9 @@ impl<'arena, T> ArenaMutList<'arena, T> { .items .insert(child_id, Box::new(UnsafeCell::new(node))); - self.parent_arena.find_mut_inner(child_id).unwrap() + self.parent_arena + .find_mut_inner(parents_map.reborrow(), child_id) + .unwrap() } // TODO - How to handle when a subtree is removed? @@ -595,22 +635,31 @@ impl<'arena, T> ArenaMutList<'arena, T> { /// /// This will also silently remove any recursive grandchildren of this item. #[must_use] - pub fn remove(&mut self, child_id: impl Into) -> Option { + pub fn remove( + &mut self, + parents_map: ArenaMapMut<'_>, + child_id: impl Into, + ) -> Option { let child_id: NodeId = child_id.into(); - if self.has(child_id) { - fn remove_children(id: NodeId, data_map: &mut DataMap) -> T { - let node = data_map.items.remove(&id).unwrap().into_inner(); - for child_id in node.children.into_iter() { - remove_children(child_id, data_map); - } - data_map.parents.remove(&id); - node.item + + if !self.has(parents_map.reborrow(), child_id) { + return None; + } + + fn remove_children( + id: NodeId, + mut parents_map: ArenaMapMut<'_>, + data_map: &mut DataMap, + ) -> T { + let node = data_map.items.remove(&id).unwrap().into_inner(); + for child_id in node.children.into_iter() { + remove_children(child_id, parents_map.reborrow_mut(), data_map); } - self.child_arr.retain(|i| *i != child_id); - Some(remove_children(child_id, self.parent_arena)) - } else { - None + parents_map.parents_map.remove(&id); + node.item } + self.child_arr.retain(|i| *i != child_id); + Some(remove_children(child_id, parents_map, self.parent_arena)) } /// Returns a shared handle equivalent to this one. @@ -639,8 +688,12 @@ impl<'arena, T> ArenaMutList<'arena, T> { /// ## Complexity /// /// O(Depth). except access from root which is O(1). - pub fn find(&self, id: impl Into) -> Option> { - self.reborrow().find(id) + pub fn find( + &self, + parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { + self.reborrow().find(parents_map, id) } /// Find an arena item among the list's items and their descendants. @@ -650,11 +703,15 @@ impl<'arena, T> ArenaMutList<'arena, T> { /// ## Complexity /// /// O(Depth). except access from root which is O(1). - pub fn find_mut(self, id: impl Into) -> Option> { + pub fn find_mut( + self, + parents_map: ArenaMapRef<'_>, + id: impl Into, + ) -> Option> { let id = id.into(); - if self.is_descendant(id) { + if self.is_descendant(parents_map, id) { // safe as we check the node is a descendant - self.parent_arena.find_mut_inner(id) + self.parent_arena.find_mut_inner(parents_map, id) } else { None } @@ -681,3 +738,70 @@ impl<'arena, T> ArenaMutList<'arena, T> { } } } + +impl ArenaMapRef<'_> { + /// Construct the path of items from the given item to the root of the tree. + /// + /// The path is in order from the bottom to the top, starting at the given item and ending at + /// the root. + /// + /// If `start_id` is Some, the path ends just before that id instead; `start_id` is not included. + /// + /// If there is no path from `start_id` to id, returns an empty vector. + pub fn get_id_path(self, id: NodeId, start_id: Option) -> Vec { + let mut path = Vec::new(); + + if !self.parents_map.contains_key(&id) { + return Vec::new(); + } + + let mut current_id = Some(id); + while let Some(current) = current_id { + path.push(current); + current_id = *self + .parents_map + .get(¤t) + .expect("All ids in the tree should have a parent in the parent map"); + if current_id == start_id { + break; + } + } + + // We've gone all the way to the root without finding start_id. + if current_id != start_id { + return Vec::new(); + } + + path + } +} + +impl ArenaMapMut<'_> { + /// Returns a shared handle equivalent to this one. + pub fn reborrow(&self) -> ArenaMapRef<'_> { + ArenaMapRef { + parents_map: self.parents_map, + } + } + + /// Returns a mutable handle equivalent to this one. + /// + /// This is sometimes useful to work with the borrow checker. + pub fn reborrow_mut(&mut self) -> ArenaMapMut<'_> { + ArenaMapMut { + parents_map: self.parents_map, + } + } + + /// Construct the path of items from the given item to the root of the tree. + /// + /// The path is in order from the bottom to the top, starting at the given item and ending at + /// the root. + /// + /// If `start_id` is Some, the path ends just before that id instead; `start_id` is not included. + /// + /// If there is no path from `start_id` to id, returns an empty vector. + pub fn get_id_path(&self, id: NodeId, start_id: Option) -> Vec { + self.reborrow().get_id_path(id, start_id) + } +} diff --git a/tree_arena/tests/basic_tests.rs b/tree_arena/tests/basic_tests.rs index 4eaa9c2edc..12ae8359df 100644 --- a/tree_arena/tests/basic_tests.rs +++ b/tree_arena/tests/basic_tests.rs @@ -8,35 +8,36 @@ use tree_arena::*; #[test] fn arena_insertions() { let mut tree: TreeArena = TreeArena::new(); - let mut roots = tree.roots_mut(); + let (mut roots, mut map) = tree.roots_mut(); // - roots.insert(1_u64, 'a'); - roots.insert(2_u64, 'b'); - assert!(roots.item(1_u64).is_some()); + roots.insert(map.reborrow_mut(), 1_u64, 'a'); + roots.insert(map.reborrow_mut(), 2_u64, 'b'); + assert!(roots.item(map.reborrow(), 1_u64).is_some()); // >-- 1(a) // // >-- 2(b) - let mut child_1 = roots.item_mut(1_u64).unwrap(); - child_1.children.insert(3_u64, 'c'); - assert!(child_1.children.item(3_u64).is_some()); + let mut child_1 = roots.item_mut(map.reborrow(), 1_u64).unwrap(); + child_1.children.insert(map.reborrow_mut(), 3_u64, 'c'); + assert!(child_1.children.item(map.reborrow(), 3_u64).is_some()); // >-- 1(a) -- 3(c) // // >-- 2(b) - let mut child_3 = child_1.children.item_mut(3_u64).unwrap(); - child_3.children.insert(4_u64, 'd'); + let mut child_3 = child_1.children.item_mut(map.reborrow(), 3_u64).unwrap(); + child_3.children.insert(map.reborrow_mut(), 4_u64, 'd'); // >-- 1(a) -- 3(c) -- 4(d) // // >-- 2(b) - let child_2 = tree.find(2_u64).expect("No child 2 found"); - let child_4 = child_2.children.find(4_u64); + let (child_2, map) = tree.find(2_u64); + let child_2 = child_2.expect("No child 2 found"); + let child_4 = child_2.children.find(map, 4_u64); assert!( child_4.is_none(), "Child 4 should not be descended from Child 2" @@ -46,32 +47,35 @@ fn arena_insertions() { #[test] fn arena_item_removal() { let mut tree: TreeArena = TreeArena::new(); - let mut roots = tree.roots_mut(); + let (mut roots, mut map) = tree.roots_mut(); // - roots.insert(1_u64, 'a'); - roots.insert(2_u64, 'b'); + roots.insert(map.reborrow_mut(), 1_u64, 'a'); + roots.insert(map.reborrow_mut(), 2_u64, 'b'); // >-- 1(a) // // >-- 2(b) - let mut child_1 = roots.item_mut(1_u64).unwrap(); + let mut child_1 = roots.item_mut(map.reborrow(), 1_u64).unwrap(); let child_1_item = child_1.item; - let mut child_3 = child_1.children.insert(3_u64, 'c'); + let mut child_3 = child_1.children.insert(map.reborrow_mut(), 3_u64, 'c'); // >-- 1(a) -- 3(c) // // >-- 2(b) - child_3.children.insert(4_u64, 'd'); + child_3.children.insert(map.reborrow_mut(), 4_u64, 'd'); // >-- 1(a) -- 3(c) -- 4(d) // // >-- 2(b) - let child_3_removed = child_1.children.remove(3_u64).expect("No child 3 found"); + let child_3_removed = child_1 + .children + .remove(map.reborrow_mut(), 3_u64) + .expect("No child 3 found"); assert_eq!(child_3_removed, 'c', "Expect removal of node 3"); // Force a realloc to surface potential UAFs. @@ -84,28 +88,28 @@ fn arena_item_removal() { // Check that the borrow of child_1.item is still valid. *child_1_item = 'X'; - assert!(child_1.children.find(3_u64).is_none()); - assert!(child_1.children.remove(3_u64).is_none()); + assert!(child_1.children.find(map.reborrow(), 3_u64).is_none()); + assert!(child_1.children.remove(map.reborrow_mut(), 3_u64).is_none()); - assert!(tree.find(4_u64).is_none()); + assert!(tree.find(4_u64).0.is_none()); } #[test] #[should_panic(expected = "Key already present")] fn arena_duplicate_insertion() { let mut tree: TreeArena = TreeArena::new(); - let mut roots = tree.roots_mut(); - roots.insert(1_u64, 'a'); - roots.insert(1_u64, 'b'); + let (mut roots, mut map) = tree.roots_mut(); + roots.insert(map.reborrow_mut(), 1_u64, 'a'); + roots.insert(map.reborrow_mut(), 1_u64, 'b'); } #[test] fn arena_mutate_parent_and_child_at_once() { let mut tree: TreeArena = TreeArena::new(); - let mut roots = tree.roots_mut(); + let (mut roots, mut map) = tree.roots_mut(); - let mut node_1 = roots.insert(1_u64, 'a'); - let mut node_2 = node_1.children.insert(2_u64, 'b'); + let mut node_1 = roots.insert(map.reborrow_mut(), 1_u64, 'a'); + let mut node_2 = node_1.children.insert(map.reborrow_mut(), 2_u64, 'b'); // >-- 1(a) -- 2(b) @@ -125,24 +129,24 @@ fn arena_mutate_parent_and_child_at_once() { #[test] fn mem_swap() { let mut tree_p: TreeArena = TreeArena::new(); - let mut roots_p = tree_p.roots_mut(); + let (mut roots_p, mut map_p) = tree_p.roots_mut(); - let mut node_p1 = roots_p.insert(1_u64, 'a'); - node_p1.children.insert(2_u64, 'b'); - let mut node_p3 = node_p1.children.insert(3_u64, 'c'); - node_p3.children.insert(4_u64, 'd'); + let mut node_p1 = roots_p.insert(map_p.reborrow_mut(), 1_u64, 'a'); + node_p1.children.insert(map_p.reborrow_mut(), 2_u64, 'b'); + let mut node_p3 = node_p1.children.insert(map_p.reborrow_mut(), 3_u64, 'c'); + node_p3.children.insert(map_p.reborrow_mut(), 4_u64, 'd'); // P: >-- 1(a) -- 2(b) // | // |- 3(c) -- 4(d) let mut tree_q: TreeArena = TreeArena::new(); - let mut roots_q = tree_q.roots_mut(); + let (mut roots_q, mut map_q) = tree_q.roots_mut(); - let mut node_q4 = roots_q.insert(4_u64, 'e'); - node_q4.children.insert(3_u64, 'f'); - let mut node_q2 = node_q4.children.insert(2_u64, 'g'); - node_q2.children.insert(1_u64, 'h'); + let mut node_q4 = roots_q.insert(map_q.reborrow_mut(), 4_u64, 'e'); + node_q4.children.insert(map_q.reborrow_mut(), 3_u64, 'f'); + let mut node_q2 = node_q4.children.insert(map_q.reborrow_mut(), 2_u64, 'g'); + node_q2.children.insert(map_q.reborrow_mut(), 1_u64, 'h'); // Q: >-- 4(e) -- 3(f) // | @@ -158,15 +162,19 @@ fn mem_swap() { // but now has the id '4' and access to the children of node Q4. assert_eq!(node_p1.id(), 4_u64); assert_eq!(node_p1.item, &'a'); - assert_eq!(node_p1.children.item(2_u64).unwrap().item, &'g',); + assert_eq!( + node_p1.children.item(map_p.reborrow(), 2_u64).unwrap().item, + &'g', + ); } #[test] fn root_ids() { let mut arena = TreeArena::new(); - arena.roots_mut().insert(3_u64, '0'); - arena.roots_mut().insert(4_u64, '0'); - arena.roots_mut().insert(5_u64, '0'); + let (mut roots, mut map) = arena.roots_mut(); + roots.insert(map.reborrow_mut(), 3_u64, '0'); + roots.insert(map.reborrow_mut(), 4_u64, '0'); + roots.insert(map.reborrow_mut(), 5_u64, '0'); assert_eq!(sorted(arena.root_ids()), vec![3, 4, 5]); } @@ -183,9 +191,9 @@ fn child_ids() { Node(2, '2', vec![]), ]), ); - assert_eq!(sorted(arena.find(0_u64).unwrap().child_ids()), vec![1, 2]); - assert_eq!(sorted(arena.find(2_u64).unwrap().child_ids()), vec![]); - assert_eq!(sorted(arena.find(3_u64).unwrap().child_ids()), vec![]); + assert_eq!(sorted(arena.find(0_u64).0.unwrap().child_ids()), vec![1, 2]); + assert_eq!(sorted(arena.find(2_u64).0.unwrap().child_ids()), vec![]); + assert_eq!(sorted(arena.find(3_u64).0.unwrap().child_ids()), vec![]); } #[test] @@ -379,32 +387,41 @@ fn reparent_root() { struct Node(u64, T, Vec>); fn add_tree(arena: &mut TreeArena, root: Node) { - let mut roots = arena.roots_mut(); - let root_mut = roots.insert(root.0, root.1); - add_children(root.2, root_mut); + let (mut roots, mut map) = arena.roots_mut(); + let root_mut = roots.insert(map.reborrow_mut(), root.0, root.1); + add_children(root.2, root_mut, map); } -fn add_children(children: Vec>, mut parent_mut: ArenaMut<'_, T>) { +fn add_children( + children: Vec>, + mut parent_mut: ArenaMut<'_, T>, + mut parents_map: ArenaMapMut<'_>, +) { for child in children { - let child_mut = parent_mut.children.insert(child.0, child.1); - add_children(child.2, child_mut); + let child_mut = parent_mut + .children + .insert(parents_map.reborrow_mut(), child.0, child.1); + add_children(child.2, child_mut, parents_map.reborrow_mut()); } } fn to_nodes(arena: &TreeArena) -> Vec> { sorted(arena.root_ids()) .into_iter() - .map(|root_id| to_node(arena.find(root_id).unwrap())) + .map(|root_id| { + let (node, map) = arena.find(root_id); + to_node(node.unwrap(), map) + }) .collect() } -fn to_node(a: ArenaRef<'_, T>) -> Node { +fn to_node(a: ArenaRef<'_, T>, parents_map: ArenaMapRef<'_>) -> Node { Node( a.id(), *a.item, sorted(a.child_ids()) .iter() - .map(|id| to_node(a.children.find(*id).unwrap())) + .map(|id| to_node(a.children.find(parents_map, *id).unwrap(), parents_map)) .collect(), ) }