diff --git a/indextree/src/arena.rs b/indextree/src/arena.rs index b28212c..f656416 100644 --- a/indextree/src/arena.rs +++ b/indextree/src/arena.rs @@ -8,12 +8,7 @@ use alloc::vec::Vec; #[cfg(not(feature = "std"))] -use core::{ - mem, - num::NonZeroUsize, - ops::{Index, IndexMut}, - slice, -}; +use core::{mem, num::NonZeroUsize}; #[cfg(feature = "par_iter")] use rayon::prelude::*; @@ -22,12 +17,7 @@ use rayon::prelude::*; use serde::{Deserialize, Serialize}; #[cfg(feature = "std")] -use std::{ - mem, - num::NonZeroUsize, - ops::{Index, IndexMut}, - slice, -}; +use std::{mem, num::NonZeroUsize}; use crate::{Node, NodeId, node::NodeData}; @@ -35,7 +25,7 @@ use crate::{Node, NodeId, node::NodeData}; #[cfg_attr(feature = "deser", derive(Deserialize, Serialize))] /// An `Arena` structure containing certain [`Node`]s. pub struct Arena { - nodes: Vec>, + pub(crate) nodes: Vec>, first_free_slot: Option, last_free_slot: Option, } @@ -84,7 +74,7 @@ impl Arena { /// let node = arena.get(foo).unwrap(); /// /// let node_id = arena.get_node_id(node).unwrap(); - /// assert_eq!(*arena[node_id].get(), "foo"); + /// assert_eq!(arena.get(node_id).map(|node| *node.get()), Some("foo")); /// ``` pub fn get_node_id(&self, node: &Node) -> Option { let nodes_range = self.nodes.as_ptr_range(); @@ -128,7 +118,7 @@ impl Arena { let index0 = index.get() - 1; // we use 1 based indexing. self.nodes .get(index0) - .filter(|n| !n.is_removed()) + .filter(|node| !node.is_removed()) .map(|node| NodeId::from_non_zero_usize(index, node.stamp)) } @@ -141,11 +131,11 @@ impl Arena { /// # Examples /// /// ``` - /// # use indextree::Arena; + /// # use indextree::{Arena, Node}; /// let mut arena = Arena::new(); /// let foo = arena.new_node("foo"); /// - /// assert_eq!(*arena[foo].get(), "foo"); + /// assert_eq!(arena.get(foo).map(Node::get), Some(&"foo")); /// ``` pub fn new_node(&mut self, data: T) -> NodeId { let (index, stamp) = if let Some(index) = self.pop_front_free_node() { @@ -167,11 +157,9 @@ impl Arena { /// Returns the number of nodes in the arena, including removed nodes. /// /// Removed nodes are still counted because they remain in the - /// internal storage. Use [`iter()`] with [`Node::is_removed()`] - /// to count only live nodes. + /// internal storage. Use [`count()`] to count only live nodes. /// - /// [`iter()`]: Arena::iter - /// [`Node::is_removed()`]: crate::Node::is_removed + /// [`count()`]: Arena::count /// /// # Examples /// @@ -180,17 +168,36 @@ impl Arena { /// let mut arena = Arena::new(); /// let foo = arena.new_node("foo"); /// let _bar = arena.new_node("bar"); - /// assert_eq!(arena.count(), 2); + /// assert_eq!(arena.len(), 2); /// /// foo.remove(&mut arena); /// // The removed node is still counted. + /// assert_eq!(arena.len(), 2); + /// ``` + #[inline] + pub fn len(&self) -> usize { + self.nodes.len() + } + + /// Returns the number of non-removed nodes in the arena. + /// + /// # Examples + /// + /// ``` + /// # use indextree::Arena; + /// let mut arena = Arena::new(); + /// let foo = arena.new_node("foo"); + /// let _bar = arena.new_node("bar"); /// assert_eq!(arena.count(), 2); + /// + /// foo.remove(&mut arena); + /// assert_eq!(arena.count(), 1); /// ``` pub fn count(&self) -> usize { - self.nodes.len() + self.iter().count() } - /// Returns `true` if arena has no nodes, `false` otherwise. + /// Returns `true` if the arena has no nodes, `false` otherwise. /// /// # Examples /// @@ -205,13 +212,14 @@ impl Arena { /// foo.remove(&mut arena); /// assert!(!arena.is_empty()); /// ``` + #[inline] pub fn is_empty(&self) -> bool { - self.count() == 0 + self.nodes.is_empty() } /// Returns a reference to the node with the given id if in the arena. /// - /// Returns `None` if not available. + /// Returns `None` if the node does not exist or has been removed. /// /// # Examples /// @@ -238,13 +246,15 @@ impl Arena { /// assert!(another_arena.get(bar).is_none()); /// ``` pub fn get(&self, id: NodeId) -> Option<&Node> { - self.nodes.get(id.index0()) + self.nodes + .get(id.index0()) + .filter(|node| !node.is_removed()) } /// Returns a mutable reference to the node with the given id if in the /// arena. /// - /// Returns `None` if not available. + /// Returns `None` if the node does not exist or has been removed. /// /// # Examples /// @@ -258,14 +268,13 @@ impl Arena { /// assert_eq!(arena.get(foo).map(|node| *node.get()), Some("FOO!")); /// ``` pub fn get_mut(&mut self, id: NodeId) -> Option<&mut Node> { - self.nodes.get_mut(id.index0()) + self.nodes + .get_mut(id.index0()) + .filter(|node| !node.is_removed()) } /// Returns an iterator of all nodes in the arena in storage-order. /// - /// Note that this iterator returns also removed elements, which can be - /// tested with the [`is_removed()`] method on the node. - /// /// # Examples /// /// ``` @@ -288,21 +297,17 @@ impl Arena { /// bar.remove(&mut arena); /// /// let mut iter = arena.iter(); - /// assert_eq!(iter.next().map(|node| (*node.get(), node.is_removed())), Some(("foo", false))); - /// assert_eq!(iter.next().map_or(false, |node| node.is_removed()), true); - /// assert_eq!(iter.next().map(|node| (*node.get(), node.is_removed())), None); + /// assert_eq!(iter.next().map(|node| *node.get()), Some("foo")); + /// assert_eq!(iter.next(), None); /// ``` - /// - /// [`is_removed()`]: Node::is_removed - pub fn iter(&self) -> slice::Iter<'_, Node> { - self.nodes.iter() + pub fn iter(&self) -> impl Iterator> { + self.nodes.iter().filter(|node| !node.is_removed()) } /// Returns an iterator of [`NodeId`]s of all non-removed nodes in /// the arena in storage-order. /// - /// Unlike [`iter()`], this skips removed nodes and yields `NodeId`s - /// instead of `&Node`. + /// Unlike [`iter()`], yields `NodeId`s instead of `&Node`. /// /// # Examples /// @@ -331,9 +336,6 @@ impl Arena { /// Returns a mutable iterator of all nodes in the arena in storage-order. /// - /// Note that this iterator returns also removed elements, which can be - /// tested with the [`is_removed()`] method on the node. - /// /// # Example /// /// ``` @@ -351,9 +353,8 @@ impl Arena { /// let node_refs = arena.iter().map(|i| i.get().clone()).collect::>(); /// assert_eq!(node_refs, vec![5, 6]); /// ``` - /// [`is_removed()`]: Node::is_removed - pub fn iter_mut(&mut self) -> slice::IterMut<'_, Node> { - self.nodes.iter_mut() + pub fn iter_mut(&mut self) -> impl Iterator> { + self.nodes.iter_mut().filter(|node| !node.is_removed()) } /// Clears all the nodes in the arena, but retains its allocated capacity. @@ -372,22 +373,12 @@ impl Arena { self.last_free_slot = None; } - /// Returns a slice of the inner nodes collection. - /// - /// The slice contains all nodes in storage order, including removed - /// nodes. Use [`Node::is_removed()`] to filter them out. - /// - /// [`Node::is_removed()`]: crate::Node::is_removed - pub fn as_slice(&self) -> &[Node] { - self.nodes.as_slice() - } - pub(crate) fn free_node(&mut self, id: NodeId) { - let node = &mut self[id]; + let node = &mut self.nodes[id.index0()]; node.data = NodeData::NextFree(None); node.stamp.as_removed(); let stamp = node.stamp; - if stamp.reuseable() { + if stamp.reusable() { if let Some(index) = self.last_free_slot { let new_last = id.index0(); self.nodes[index].data = NodeData::NextFree(Some(new_last)); @@ -407,7 +398,7 @@ impl Arena { if let NodeData::NextFree(next_free) = self.nodes[index].data { self.first_free_slot = next_free; } else { - unreachable!("A data node considered as a freed node"); + unreachable!("a data node considered as a freed node"); } if self.first_free_slot.is_none() { self.last_free_slot = None; @@ -425,9 +416,6 @@ impl Arena { /// Requires the `par_iter` feature. Uses [rayon](https://docs.rs/rayon) /// for data parallelism across all nodes in storage order. /// - /// Note that this iterator returns also removed elements, which can be - /// tested with the [`is_removed()`] method on the node. - /// /// # Examples /// /// ``` @@ -441,10 +429,8 @@ impl Arena { /// let sum: i64 = arena.par_iter().map(|node| *node.get()).sum(); /// assert_eq!(sum, 6); /// ``` - /// - /// [`is_removed()`]: Node::is_removed - pub fn par_iter(&self) -> rayon::slice::Iter<'_, Node> { - self.nodes.par_iter() + pub fn par_iter(&self) -> impl ParallelIterator> { + self.nodes.par_iter().filter(|node| !node.is_removed()) } } @@ -458,20 +444,6 @@ impl Default for Arena { } } -impl Index for Arena { - type Output = Node; - - fn index(&self, node: NodeId) -> &Node { - &self.nodes[node.index0()] - } -} - -impl IndexMut for Arena { - fn index_mut(&mut self, node: NodeId) -> &mut Node { - &mut self.nodes[node.index0()] - } -} - #[test] fn reuse_node() { let mut arena = Arena::new(); diff --git a/indextree/src/debug_pretty_print.rs b/indextree/src/debug_pretty_print.rs index 8c4c453..81961d4 100644 --- a/indextree/src/debug_pretty_print.rs +++ b/indextree/src/debug_pretty_print.rs @@ -11,6 +11,7 @@ use alloc::vec::Vec; use std::vec::Vec; use crate::{ + Node, arena::Arena, id::NodeId, traverse::{NodeEdge, Traverse}, @@ -238,7 +239,7 @@ pub struct DebugPrettyPrint<'a, T> { impl<'a, T> DebugPrettyPrint<'a, T> { /// Creates a new `DebugPrettyPrint` object for the node. #[inline] - pub(crate) fn new(id: &'a NodeId, arena: &'a Arena) -> Self { + pub(crate) const fn new(id: &'a NodeId, arena: &'a Arena) -> Self { Self { id, arena } } } @@ -251,8 +252,7 @@ impl fmt::Display for DebugPrettyPrint<'_, T> { // Print the first (root) node. traverser.next(); - { - let data = self.arena[*self.id].get(); + if let Some(data) = self.arena.get(*self.id).map(Node::get) { if is_alternate { write!(writer, "{data:#}")? } else { @@ -262,11 +262,12 @@ impl fmt::Display for DebugPrettyPrint<'_, T> { // Print the descendants. while let Some(id) = prepare_next_node_printing(&mut writer, &mut traverser)? { - let data = traverser.arena()[id].get(); - if is_alternate { - write!(writer, "{data:#}")? - } else { - write!(writer, "{data}")? + if let Some(data) = traverser.arena().get(id).map(Node::get) { + if is_alternate { + write!(writer, "{data:#}")? + } else { + write!(writer, "{data}")? + } } } @@ -282,8 +283,7 @@ impl fmt::Debug for DebugPrettyPrint<'_, T> { // Print the first (root) node. traverser.next(); - { - let data = self.arena[*self.id].get(); + if let Some(data) = self.arena.get(*self.id).map(Node::get) { if is_alternate { write!(writer, "{data:#?}")? } else { @@ -293,11 +293,12 @@ impl fmt::Debug for DebugPrettyPrint<'_, T> { // Print the descendants. while let Some(id) = prepare_next_node_printing(&mut writer, &mut traverser)? { - let data = traverser.arena()[id].get(); - if is_alternate { - write!(writer, "{data:#?}")? - } else { - write!(writer, "{data:?}")? + if let Some(data) = traverser.arena().get(id).map(Node::get) { + if is_alternate { + write!(writer, "{data:#?}")? + } else { + write!(writer, "{data:?}")? + } } } @@ -325,7 +326,9 @@ fn prepare_next_node_printing( } } }; - let is_last_sibling = traverser.arena()[id].next_sibling().is_none(); + let is_last_sibling = traverser.arena().nodes[id.index0()] + .next_sibling() + .is_none(); writer.open_item(is_last_sibling)?; return Ok(Some(id)); diff --git a/indextree/src/id.rs b/indextree/src/id.rs index c58529d..2af50fd 100644 --- a/indextree/src/id.rs +++ b/indextree/src/id.rs @@ -14,7 +14,7 @@ use serde::{Deserialize, Serialize}; use std::{fmt, num::NonZeroUsize}; use crate::{ - Ancestors, Arena, Children, Descendants, FollowingSiblings, NodeError, PrecedingSiblings, + Ancestors, Arena, Children, Descendants, FollowingSiblings, Node, NodeError, PrecedingSiblings, Predecessors, ReverseTraverse, Traverse, debug_pretty_print::DebugPrettyPrint, relations::{insert_last_unchecked, insert_with_neighbors}, @@ -48,7 +48,8 @@ pub(crate) struct NodeStamp(i16); impl NodeStamp { /// Returns `true` if this stamp represents a removed node (negative /// value). - pub fn is_removed(self) -> bool { + #[inline] + pub const fn is_removed(self) -> bool { self.0.is_negative() } @@ -56,7 +57,7 @@ impl NodeStamp { /// /// The resulting negative value differs from the live stamp, allowing /// `NodeId::is_removed` to detect stale references via inequality. - pub fn as_removed(&mut self) { + pub const fn as_removed(&mut self) { debug_assert!(!self.is_removed()); self.0 = if self.0 < i16::MAX { -self.0 - 1 @@ -69,7 +70,7 @@ impl NodeStamp { /// /// A slot becomes permanently unreusable once its generation nears /// `i16::MIN`, preventing stamp value collisions after many cycles. - pub fn reuseable(self) -> bool { + pub const fn reusable(self) -> bool { debug_assert!(self.is_removed()); self.0 > i16::MIN + 1 } @@ -78,8 +79,8 @@ impl NodeStamp { /// /// Negates the value back to positive, producing a new generation /// that differs from all previous stamps for this slot. - pub fn reuse(&mut self) -> Self { - debug_assert!(self.reuseable()); + pub const fn reuse(&mut self) -> Self { + debug_assert!(self.reusable()); self.0 = -self.0; *self } @@ -105,20 +106,23 @@ impl From for usize { impl NodeId { /// Returns zero-based index. - pub(crate) fn index0(self) -> usize { + pub(crate) const fn index0(self) -> usize { // This is totally safe because `self.index1 >= 1` is guaranteed by // `NonZeroUsize` type. self.index1.get() - 1 } /// Creates a new `NodeId` from the given one-based index. - pub(crate) fn from_non_zero_usize(index1: NonZeroUsize, stamp: NodeStamp) -> Self { + pub(crate) const fn from_non_zero_usize(index1: NonZeroUsize, stamp: NodeStamp) -> Self { NodeId { index1, stamp } } - /// Return if the `Node` of NodeId point to is removed. + /// Return if the [`Node`] this [`NodeId`] points to is removed. pub fn is_removed(self, arena: &Arena) -> bool { - arena[self].stamp != self.stamp + arena + .nodes + .get(self.index0()) + .is_none_or(|node| node.stamp != self.stamp) } /// Returns the ID of the parent node, unless this node is the root of the @@ -147,7 +151,7 @@ impl NodeId { /// assert_eq!(n1_3.parent(&arena), Some(n1)); /// ``` pub fn parent(self, arena: &Arena) -> Option { - arena[self].parent() + arena.get(self).and_then(Node::parent) } /// Returns an iterator of IDs of this node and its ancestors. @@ -575,7 +579,7 @@ impl NodeId { /// # Examples /// /// ``` - /// # use indextree::{Arena, NodeEdge}; + /// # use indextree::{Arena, Node, NodeEdge}; /// # let mut arena = Arena::new(); /// # let n1 = arena.new_node("1"); /// # let n1_1 = arena.new_node("1_1"); @@ -605,9 +609,9 @@ impl NodeId { /// // `-- (implicit) /// // `-- 1_2 * /// - /// assert!(arena[n1_2].parent().is_none()); - /// assert!(arena[n1_2].previous_sibling().is_none()); - /// assert!(arena[n1_2].next_sibling().is_none()); + /// assert!(arena.get(n1_2).and_then(Node::parent).is_none()); + /// assert!(arena.get(n1_2).and_then(Node::previous_sibling).is_none()); + /// assert!(arena.get(n1_2).and_then(Node::next_sibling).is_none()); /// /// let mut iter = n1.descendants(&arena); /// assert_eq!(iter.next(), Some(n1)); @@ -624,7 +628,7 @@ impl NodeId { // Ensure the node is surely detached. debug_assert!( - arena[self].is_detached(), + arena.nodes[self.index0()].is_detached(), "The node should be successfully detached" ); } @@ -708,15 +712,21 @@ impl NodeId { if new_child == self { return Err(NodeError::AppendSelf); } - if arena[self].is_removed() || arena[new_child].is_removed() { + if arena.get(self).is_none() || arena.get(new_child).is_none() { return Err(NodeError::Removed); } if self.ancestors(arena).any(|ancestor| new_child == ancestor) { return Err(NodeError::AppendAncestor); } new_child.detach(arena); - insert_with_neighbors(arena, new_child, Some(self), arena[self].last_child, None) - .expect("Should never fail: `new_child` is not `self` and they are not removed"); + insert_with_neighbors( + arena, + new_child, + Some(self), + arena.get(self).and_then(Node::last_child), + None, + ) + .expect("Should never fail: `new_child` is not `self` and they are not removed"); Ok(()) } @@ -900,14 +910,20 @@ impl NodeId { if new_child == self { return Err(NodeError::PrependSelf); } - if arena[self].is_removed() || arena[new_child].is_removed() { + if arena.get(self).is_none() || arena.get(new_child).is_none() { return Err(NodeError::Removed); } if self.ancestors(arena).any(|ancestor| new_child == ancestor) { return Err(NodeError::PrependAncestor); } - insert_with_neighbors(arena, new_child, Some(self), None, arena[self].first_child) - .expect("Should never fail: `new_child` is not `self` and they are not removed"); + insert_with_neighbors( + arena, + new_child, + Some(self), + None, + arena.get(self).and_then(Node::first_child), + ) + .expect("Should never fail: `new_child` is not `self` and they are not removed"); Ok(()) } @@ -994,13 +1010,16 @@ impl NodeId { if new_sibling == self { return Err(NodeError::InsertAfterSelf); } - if arena[self].is_removed() || arena[new_sibling].is_removed() { + if arena.get(self).is_none() || arena.get(new_sibling).is_none() { return Err(NodeError::Removed); } new_sibling.detach(arena); let (next_sibling, parent) = { - let current = &arena[self]; - (current.next_sibling, current.parent) + let current = arena.get(self); + ( + current.and_then(Node::next_sibling), + current.and_then(Node::parent), + ) }; insert_with_neighbors(arena, new_sibling, parent, Some(self), next_sibling) .expect("Should never fail: `new_sibling` is not `self` and they are not removed"); @@ -1090,13 +1109,16 @@ impl NodeId { if new_sibling == self { return Err(NodeError::InsertBeforeSelf); } - if arena[self].is_removed() || arena[new_sibling].is_removed() { + if arena.get(self).is_none() || arena.get(new_sibling).is_none() { return Err(NodeError::Removed); } new_sibling.detach(arena); let (previous_sibling, parent) = { - let current = &arena[self]; - (current.previous_sibling, current.parent) + let current = arena.get(self); + ( + current.and_then(Node::previous_sibling), + current.and_then(Node::parent), + ) }; insert_with_neighbors(arena, new_sibling, parent, previous_sibling, Some(self)) .expect("Should never fail: `new_sibling` is not `self` and they are not removed"); @@ -1161,22 +1183,32 @@ impl NodeId { pub fn remove(self, arena: &mut Arena) { debug_assert_triangle_nodes!( arena, - arena[self].parent, - arena[self].previous_sibling, + arena.nodes[self.index0()].parent, + arena.nodes[self.index0()].previous_sibling, Some(self) ); debug_assert_triangle_nodes!( arena, - arena[self].parent, + arena.nodes[self.index0()].parent, Some(self), - arena[self].next_sibling + arena.nodes[self.index0()].next_sibling + ); + debug_assert_triangle_nodes!( + arena, + Some(self), + None, + arena.nodes[self.index0()].first_child + ); + debug_assert_triangle_nodes!( + arena, + Some(self), + arena.nodes[self.index0()].last_child, + None ); - debug_assert_triangle_nodes!(arena, Some(self), None, arena[self].first_child); - debug_assert_triangle_nodes!(arena, Some(self), arena[self].last_child, None); // Retrieve needed values. let (parent, previous_sibling, next_sibling, first_child, last_child) = { - let node = &arena[self]; + let node = &arena.nodes[self.index0()]; ( node.parent, node.previous_sibling, @@ -1195,7 +1227,7 @@ impl NodeId { .expect("Should never fail: neighbors and children must be consistent"); } arena.free_node(self); - debug_assert!(arena[self].is_detached()); + debug_assert!(arena.nodes[self.index0()].is_detached()); } /// Removes a node and its descendants from the arena. @@ -1245,12 +1277,21 @@ impl NodeId { let mut cursor = Some(self); while let Some(id) = cursor { arena.free_node(id); - let node = &arena[id]; + let node = &arena.nodes[id.index0()]; cursor = node.first_child.or(node.next_sibling).or_else(|| { - id.ancestors(arena) // traverse ancestors upwards - .skip(1) // skip the starting node itself - .find(|n| arena[*n].next_sibling.is_some()) // first ancestor with a sibling - .and_then(|n| arena[n].next_sibling) // the sibling is the new cursor + let mut ancestor = node.parent; + + while let Some(parent) = ancestor { + let parent_node = &arena.nodes[parent.index0()]; + + if let Some(sibling) = parent_node.next_sibling { + return Some(sibling); + } + + ancestor = parent_node.parent; + } + + None }); } } @@ -1264,7 +1305,7 @@ impl NodeId { /// # Examples /// /// ``` - /// # use indextree::Arena; + /// # use indextree::{Arena, Node}; /// # let mut arena = Arena::new(); /// # let n1 = arena.new_node("1"); /// # let n1_1 = arena.new_node("1_1"); @@ -1293,15 +1334,15 @@ impl NodeId { /// // `-- 1_3 /// /// assert_eq!(n1.children(&arena).count(), 0); - /// assert!(!arena[n1_1].is_removed()); - /// assert!(arena[n1_1].parent().is_none()); + /// assert!(arena.get(n1_1).is_some()); + /// assert!(arena.get(n1_1).and_then(Node::parent).is_none()); /// // 1_2's subtree is preserved - /// assert_eq!(arena[n1_2_1].parent(), Some(n1_2)); + /// assert_eq!(arena.get(n1_2_1).and_then(Node::parent), Some(n1_2)); /// ``` pub fn detach_children(self, arena: &mut Arena) { - let mut child_opt = arena[self].first_child; + let mut child_opt = arena.get(self).and_then(Node::first_child); while let Some(child) = child_opt { - let next = arena[child].next_sibling; + let next = arena.get(child).and_then(Node::next_sibling); child.detach(arena); child_opt = next; } @@ -1348,9 +1389,9 @@ impl NodeId { /// /// [`remove_subtree`]: NodeId::remove_subtree pub fn remove_children(self, arena: &mut Arena) { - let mut child_opt = arena[self].first_child; + let mut child_opt = arena.get(self).and_then(Node::first_child); while let Some(child) = child_opt { - let next = arena[child].next_sibling; + let next = arena.get(child).and_then(Node::next_sibling); child.remove_subtree(arena); child_opt = next; } diff --git a/indextree/src/node.rs b/indextree/src/node.rs index f647006..0883d6c 100644 --- a/indextree/src/node.rs +++ b/indextree/src/node.rs @@ -42,28 +42,20 @@ pub struct Node { impl Node { /// Returns a reference to the node data. - /// - /// # Panics - /// - /// Panics if the node has been removed. pub fn get(&self) -> &T { if let NodeData::Data(ref data) = self.data { data } else { - panic!("attempted to access a freed node") + unreachable!("&Node will always be a non-removed node"); } } /// Returns a mutable reference to the node data. - /// - /// # Panics - /// - /// Panics if the node has been removed. pub fn get_mut(&mut self) -> &mut T { if let NodeData::Data(ref mut data) = self.data { data } else { - panic!("attempted to access a freed node") + unreachable!("&Node will always be a non-removed node"); } } @@ -99,7 +91,7 @@ impl Node { /// # Examples /// /// ``` - /// # use indextree::Arena; + /// # use indextree::{Arena, Node}; /// # let mut arena = Arena::new(); /// # let n1 = arena.new_node("1"); /// # let n1_1 = arena.new_node("1_1"); @@ -113,12 +105,13 @@ impl Node { /// // |-- 1_1 /// // |-- 1_2 /// // `-- 1_3 - /// assert_eq!(arena[n1].parent(), None); - /// assert_eq!(arena[n1_1].parent(), Some(n1)); - /// assert_eq!(arena[n1_2].parent(), Some(n1)); - /// assert_eq!(arena[n1_3].parent(), Some(n1)); + /// assert_eq!(arena.get(n1).and_then(Node::parent), None); + /// assert_eq!(arena.get(n1_1).and_then(Node::parent), Some(n1)); + /// assert_eq!(arena.get(n1_2).and_then(Node::parent), Some(n1)); + /// assert_eq!(arena.get(n1_3).and_then(Node::parent), Some(n1)); /// ``` - pub fn parent(&self) -> Option { + #[inline] + pub const fn parent(&self) -> Option { self.parent } @@ -127,7 +120,7 @@ impl Node { /// # Examples /// /// ``` - /// # use indextree::Arena; + /// # use indextree::{Arena, Node}; /// # let mut arena = Arena::new(); /// # let n1 = arena.new_node("1"); /// # let n1_1 = arena.new_node("1_1"); @@ -141,12 +134,13 @@ impl Node { /// // |-- 1_1 /// // |-- 1_2 /// // `-- 1_3 - /// assert_eq!(arena[n1].first_child(), Some(n1_1)); - /// assert_eq!(arena[n1_1].first_child(), None); - /// assert_eq!(arena[n1_2].first_child(), None); - /// assert_eq!(arena[n1_3].first_child(), None); + /// assert_eq!(arena.get(n1).and_then(Node::first_child), Some(n1_1)); + /// assert_eq!(arena.get(n1_1).and_then(Node::first_child), None); + /// assert_eq!(arena.get(n1_2).and_then(Node::first_child), None); + /// assert_eq!(arena.get(n1_3).and_then(Node::first_child), None); /// ``` - pub fn first_child(&self) -> Option { + #[inline] + pub const fn first_child(&self) -> Option { self.first_child } @@ -155,7 +149,7 @@ impl Node { /// # Examples /// /// ``` - /// # use indextree::Arena; + /// # use indextree::{Arena, Node}; /// # let mut arena = Arena::new(); /// # let n1 = arena.new_node("1"); /// # let n1_1 = arena.new_node("1_1"); @@ -169,12 +163,13 @@ impl Node { /// // |-- 1_1 /// // |-- 1_2 /// // `-- 1_3 - /// assert_eq!(arena[n1].last_child(), Some(n1_3)); - /// assert_eq!(arena[n1_1].last_child(), None); - /// assert_eq!(arena[n1_2].last_child(), None); - /// assert_eq!(arena[n1_3].last_child(), None); + /// assert_eq!(arena.get(n1).and_then(Node::last_child), Some(n1_3)); + /// assert_eq!(arena.get(n1_1).and_then(Node::last_child), None); + /// assert_eq!(arena.get(n1_2).and_then(Node::last_child), None); + /// assert_eq!(arena.get(n1_3).and_then(Node::last_child), None); /// ``` - pub fn last_child(&self) -> Option { + #[inline] + pub const fn last_child(&self) -> Option { self.last_child } @@ -184,7 +179,7 @@ impl Node { /// # Examples /// /// ``` - /// # use indextree::Arena; + /// # use indextree::{Arena, Node}; /// # let mut arena = Arena::new(); /// # let n1 = arena.new_node("1"); /// # let n1_1 = arena.new_node("1_1"); @@ -198,17 +193,17 @@ impl Node { /// // |-- 1_1 /// // |-- 1_2 /// // `-- 1_3 - /// assert_eq!(arena[n1].previous_sibling(), None); - /// assert_eq!(arena[n1_1].previous_sibling(), None); - /// assert_eq!(arena[n1_2].previous_sibling(), Some(n1_1)); - /// assert_eq!(arena[n1_3].previous_sibling(), Some(n1_2)); + /// assert_eq!(arena.get(n1).and_then(Node::previous_sibling), None); + /// assert_eq!(arena.get(n1_1).and_then(Node::previous_sibling), None); + /// assert_eq!(arena.get(n1_2).and_then(Node::previous_sibling), Some(n1_1)); + /// assert_eq!(arena.get(n1_3).and_then(Node::previous_sibling), Some(n1_2)); /// ``` /// /// Note that newly created nodes are independent toplevel nodes, and they /// are not siblings by default. /// /// ``` - /// # use indextree::Arena; + /// # use indextree::{Arena, Node}; /// let mut arena = Arena::new(); /// let n1 = arena.new_node("1"); /// let n2 = arena.new_node("2"); @@ -217,18 +212,19 @@ impl Node { /// // | `-- 1 /// // `-- (implicit) /// // `-- 2 - /// assert_eq!(arena[n1].previous_sibling(), None); - /// assert_eq!(arena[n2].previous_sibling(), None); + /// assert_eq!(arena.get(n1).and_then(Node::previous_sibling), None); + /// assert_eq!(arena.get(n2).and_then(Node::previous_sibling), None); /// /// n1.insert_after(n2, &mut arena); /// // arena /// // `-- (implicit) /// // |-- 1 /// // `-- 2 - /// assert_eq!(arena[n1].previous_sibling(), None); - /// assert_eq!(arena[n2].previous_sibling(), Some(n1)); + /// assert_eq!(arena.get(n1).and_then(Node::previous_sibling), None); + /// assert_eq!(arena.get(n2).and_then(Node::previous_sibling), Some(n1)); /// ``` - pub fn previous_sibling(&self) -> Option { + #[inline] + pub const fn previous_sibling(&self) -> Option { self.previous_sibling } @@ -238,7 +234,7 @@ impl Node { /// # Examples /// /// ``` - /// # use indextree::Arena; + /// # use indextree::{Arena, Node}; /// # let mut arena = Arena::new(); /// # let n1 = arena.new_node("1"); /// # let n1_1 = arena.new_node("1_1"); @@ -252,17 +248,17 @@ impl Node { /// // |-- 1_1 /// // |-- 1_2 /// // `-- 1_3 - /// assert_eq!(arena[n1].next_sibling(), None); - /// assert_eq!(arena[n1_1].next_sibling(), Some(n1_2)); - /// assert_eq!(arena[n1_2].next_sibling(), Some(n1_3)); - /// assert_eq!(arena[n1_3].next_sibling(), None); + /// assert_eq!(arena.get(n1).and_then(Node::next_sibling), None); + /// assert_eq!(arena.get(n1_1).and_then(Node::next_sibling), Some(n1_2)); + /// assert_eq!(arena.get(n1_2).and_then(Node::next_sibling), Some(n1_3)); + /// assert_eq!(arena.get(n1_3).and_then(Node::next_sibling), None); /// ``` /// /// Note that newly created nodes are independent toplevel nodes, and they /// are not siblings by default. /// /// ``` - /// # use indextree::Arena; + /// # use indextree::{Arena, Node}; /// let mut arena = Arena::new(); /// let n1 = arena.new_node("1"); /// let n2 = arena.new_node("2"); @@ -271,18 +267,19 @@ impl Node { /// // | `-- 1 /// // `-- (implicit) /// // `-- 2 - /// assert_eq!(arena[n1].next_sibling(), None); - /// assert_eq!(arena[n2].next_sibling(), None); + /// assert_eq!(arena.get(n1).and_then(Node::next_sibling), None); + /// assert_eq!(arena.get(n2).and_then(Node::next_sibling), None); /// /// n1.insert_after(n2, &mut arena); /// // arena /// // `-- (implicit) /// // |-- 1 /// // `-- 2 - /// assert_eq!(arena[n1].next_sibling(), Some(n2)); - /// assert_eq!(arena[n2].next_sibling(), None); + /// assert_eq!(arena.get(n1).and_then(Node::next_sibling), Some(n2)); + /// assert_eq!(arena.get(n2).and_then(Node::next_sibling), None); /// ``` - pub fn next_sibling(&self) -> Option { + #[inline] + pub const fn next_sibling(&self) -> Option { self.next_sibling } @@ -291,7 +288,7 @@ impl Node { /// # Examples /// /// ``` - /// # use indextree::Arena; + /// # use indextree::{Arena, Node}; /// # let mut arena = Arena::new(); /// # let n1 = arena.new_node("1"); /// # let n1_1 = arena.new_node("1_1"); @@ -305,27 +302,28 @@ impl Node { /// // |-- 1_1 /// // |-- 1_2 * /// // `-- 1_3 - /// assert_eq!(arena[n1_1].next_sibling(), Some(n1_2)); - /// assert_eq!(arena[n1_2].parent(), Some(n1)); - /// assert!(!arena[n1_2].is_removed()); - /// assert_eq!(arena[n1_3].previous_sibling(), Some(n1_2)); + /// assert_eq!(arena.get(n1_1).and_then(Node::next_sibling), Some(n1_2)); + /// assert_eq!(arena.get(n1_2).and_then(Node::parent), Some(n1)); + /// assert!(arena.get(n1_2).is_some()); + /// assert_eq!(arena.get(n1_3).and_then(Node::previous_sibling), Some(n1_2)); /// /// n1_2.remove(&mut arena); /// // arena /// // `-- 1 /// // |-- 1_1 /// // `-- 1_3 - /// assert_eq!(arena[n1_1].next_sibling(), Some(n1_3)); - /// assert_eq!(arena[n1_2].parent(), None); - /// assert!(arena[n1_2].is_removed()); - /// assert_eq!(arena[n1_3].previous_sibling(), Some(n1_1)); + /// assert_eq!(arena.get(n1_1).and_then(Node::next_sibling), Some(n1_3)); + /// assert_eq!(arena.get(n1_2).and_then(Node::parent), None); + /// assert!(arena.get(n1_2).is_none()); + /// assert_eq!(arena.get(n1_3).and_then(Node::previous_sibling), Some(n1_1)); /// ``` - pub fn is_removed(&self) -> bool { + #[inline] + pub const fn is_removed(&self) -> bool { self.stamp.is_removed() } /// Checks if the node is detached. - pub(crate) fn is_detached(&self) -> bool { + pub(crate) const fn is_detached(&self) -> bool { self.parent.is_none() && self.previous_sibling.is_none() && self.next_sibling.is_none() } } diff --git a/indextree/src/relations.rs b/indextree/src/relations.rs index 2c33295..8a4c6f6 100644 --- a/indextree/src/relations.rs +++ b/indextree/src/relations.rs @@ -4,7 +4,7 @@ //! inserted, detached, or transplanted within the tree. use crate::{ - Arena, NodeId, + Arena, Node, NodeId, error::ConsistencyError, siblings_range::{DetachedSiblingsRange, SiblingsRange}, }; @@ -31,7 +31,7 @@ pub(crate) fn assert_triangle_nodes( previous: Option, next: Option, ) { - if let Some(previous_node) = previous.map(|id| &arena[id]) { + if let Some(previous_node) = previous.and_then(|id| arena.get(id)) { assert_eq!( previous_node.parent, parent, "`prev->parent` must equal to `parent`" @@ -41,7 +41,7 @@ pub(crate) fn assert_triangle_nodes( "`prev->next` must equal to `next`" ); } - if let Some(next_node) = next.map(|id| &arena[id]) { + if let Some(next_node) = next.and_then(|id| arena.get(id)) { assert_eq!( next_node.parent, parent, "`next->parent` must equal to `parent`" @@ -71,38 +71,38 @@ pub(crate) fn connect_neighbors( next: Option, ) { if cfg!(debug_assertions) { - if let Some(parent_node) = parent.map(|id| &arena[id]) { + if let Some(parent_node) = parent.and_then(|id| arena.get(id)) { debug_assert_eq!( parent_node.first_child.is_some(), parent_node.last_child.is_some() ); debug_assert!(!parent_node.is_removed()); } - debug_assert!(!previous.is_some_and(|id| arena[id].is_removed())); - debug_assert!(!next.is_some_and(|id| arena[id].is_removed())); } let (mut parent_first_child, mut parent_last_child) = parent - .map(|id| &arena[id]) + .map(|id| &arena.nodes[id.index0()]) .map_or((None, None), |node| (node.first_child, node.last_child)); + if let Some(previous) = previous { // `previous` ==> `next` - arena[previous].next_sibling = next; + arena.nodes[previous.index0()].next_sibling = next; parent_first_child = parent_first_child.or(Some(previous)); } else { // `next` is the first child of the parent. parent_first_child = next; } + if let Some(next) = next { // `previous` <== `next` - arena[next].previous_sibling = previous; + arena.nodes[next.index0()].previous_sibling = previous; parent_last_child = parent_last_child.or(Some(next)); } else { // `previous` is the last child of the parent. parent_last_child = previous; } - if let Some(parent_node) = parent.map(|id| &mut arena[id]) { + if let Some(parent_node) = parent.map(|id| &mut arena.nodes[id.index0()]) { debug_assert_eq!(parent_first_child.is_some(), parent_last_child.is_some()); parent_node.first_child = parent_first_child; parent_node.last_child = parent_last_child; @@ -174,7 +174,7 @@ pub(crate) fn insert_with_neighbors( /// ... -> prev -> (new) /// ``` pub(crate) fn insert_last_unchecked(arena: &mut Arena, new: NodeId, parent: NodeId) { - let previous_sibling = arena[parent].last_child; + let previous_sibling = arena.get(parent).and_then(Node::last_child); DetachedSiblingsRange::new(new, new) .transplant(arena, Some(parent), previous_sibling, None) .expect( diff --git a/indextree/src/siblings_range.rs b/indextree/src/siblings_range.rs index 394d2ff..f23dbf9 100644 --- a/indextree/src/siblings_range.rs +++ b/indextree/src/siblings_range.rs @@ -16,7 +16,7 @@ impl SiblingsRange { /// /// It is user's responsibility to guarantee that `first` to `last` is a /// correct range. - pub(crate) fn new(first: NodeId, last: NodeId) -> Self { + pub(crate) const fn new(first: NodeId, last: NodeId) -> Self { Self { first, last } } @@ -25,19 +25,20 @@ impl SiblingsRange { pub(crate) fn detach_from_siblings(self, arena: &mut Arena) -> DetachedSiblingsRange { // Update children's parents, siblings relations outside the range, and // old parent's first and last child nodes. - let parent = arena[self.first].parent; + let parent = arena.nodes[self.first.index0()].parent; // Update siblings relations outside the range and old parent's // children if necessary. - let prev_of_range = arena[self.first].previous_sibling.take(); - let next_of_range = arena[self.last].next_sibling.take(); + let prev_of_range = arena.nodes[self.first.index0()].previous_sibling.take(); + let next_of_range = arena.nodes[self.last.index0()].next_sibling.take(); + connect_neighbors(arena, parent, prev_of_range, next_of_range); if cfg!(debug_assertions) { - debug_assert_eq!(arena[self.first].previous_sibling, None); - debug_assert_eq!(arena[self.last].next_sibling, None); + debug_assert_eq!(arena.nodes[self.first.index0()].previous_sibling(), None); + debug_assert_eq!(arena.nodes[self.last.index0()].next_sibling(), None); debug_assert_triangle_nodes!(arena, parent, prev_of_range, next_of_range); - if let Some(parent_node) = parent.map(|id| &arena[id]) { + if let Some(parent_node) = parent.and_then(|id| arena.get(id)) { debug_assert_eq!( parent_node.first_child.is_some(), parent_node.last_child.is_some() @@ -73,7 +74,7 @@ impl DetachedSiblingsRange { /// /// It is user's responsibility to guarantee that `first` to `last` is a /// correct range. - pub(crate) fn new(first: NodeId, last: NodeId) -> Self { + pub(crate) const fn new(first: NodeId, last: NodeId) -> Self { Self { first, last } } @@ -94,7 +95,7 @@ impl DetachedSiblingsRange { // Attempt to set the node itself as its parent. return Err(ConsistencyError::ParentChildLoop); } - let child_node = &mut arena[child]; + let child_node = &mut arena.nodes[child.index0()]; child_node.parent = new_parent; child_opt = child_node.next_sibling; } @@ -121,13 +122,13 @@ impl DetachedSiblingsRange { // Check that the given arguments are consistent. if cfg!(debug_assertions) { if let Some(previous_sibling) = previous_sibling { - debug_assert_eq!(arena[previous_sibling].parent, parent); + debug_assert_eq!(arena.nodes[previous_sibling.index0()].parent(), parent); } if let Some(next_sibling) = next_sibling { - debug_assert_eq!(arena[next_sibling].parent, parent); + debug_assert_eq!((arena.nodes[next_sibling.index0()]).parent(), parent); } debug_assert_triangle_nodes!(arena, parent, previous_sibling, next_sibling); - if let Some(parent_node) = parent.map(|id| &arena[id]) { + if let Some(parent_node) = parent.and_then(|id| arena.get(id)) { debug_assert_eq!( parent_node.first_child.is_some(), parent_node.last_child.is_some() @@ -149,7 +150,7 @@ impl DetachedSiblingsRange { if cfg!(debug_assertions) { debug_assert_triangle_nodes!(arena, parent, previous_sibling, Some(self.first)); debug_assert_triangle_nodes!(arena, parent, Some(self.last), next_sibling); - if let Some(parent_node) = parent.map(|id| &arena[id]) { + if let Some(parent_node) = parent.and_then(|id| arena.get(id)) { debug_assert!( parent_node.first_child.is_some() && parent_node.last_child.is_some(), "parent should have children (at least `self.first`)" diff --git a/indextree/src/traverse.rs b/indextree/src/traverse.rs index 7c60bcd..9c41773 100644 --- a/indextree/src/traverse.rs +++ b/indextree/src/traverse.rs @@ -72,7 +72,13 @@ macro_rules! new_iterator { let next: fn(&Node) -> Option = $next; let node = self.0.node.take()?; - self.0.node = next(&self.0.arena[node]); + + if node.is_removed(self.0.arena) { + return None; + } + + self.0.node = next(&self.0.arena.nodes[node.index0()]); + Some(node) } @@ -97,15 +103,24 @@ macro_rules! new_iterator { fn next(&mut self) -> Option { match (self.0.head, self.0.tail) { (Some(head), Some(tail)) if head == tail => { - let result = head; self.0.head = None; self.0.tail = None; - Some(result) + + if head.is_removed(self.0.arena) { + return None; + } + + Some(head) } (Some(head), None) | (Some(head), Some(_)) => { let next: fn(&Node) -> Option = $next; - self.0.head = next(&self.0.arena[head]); + if head.is_removed(self.0.arena) { + return None; + } + + self.0.head = next(&self.0.arena.nodes[head.index0()]); + Some(head) } (None, Some(_)) | (None, None) => None, @@ -121,15 +136,24 @@ macro_rules! new_iterator { fn next_back(&mut self) -> Option { match (self.0.head, self.0.tail) { (Some(head), Some(tail)) if head == tail => { - let result = head; self.0.head = None; self.0.tail = None; - Some(result) + + if tail.is_removed(self.0.arena) { + return None; + } + + Some(tail) } (None, Some(tail)) | (Some(_), Some(tail)) => { let next_back: fn(&Node) -> Option = $next_back; - self.0.tail = next_back(&self.0.arena[tail]); + if tail.is_removed(self.0.arena) { + return None; + } + + self.0.tail = next_back(&self.0.arena.nodes[tail.index0()]); + Some(tail) } (Some(_), None) | (None, None) => None, @@ -161,7 +185,7 @@ macro_rules! new_iterator { new_iterator!( /// An iterator of the IDs of the ancestors of a given node. Ancestors, - next = |node| node.parent, + next = Node::parent, ); new_iterator!( @@ -176,40 +200,41 @@ new_iterator!( new = |arena, node| { let first = arena .get(node) - .unwrap() - .parent - .and_then(|parent_id| arena.get(parent_id)) - .and_then(|parent| parent.first_child); + .and_then(Node::parent) + .and_then(|parent_id| arena.get(parent_id)?.first_child()); DoubleEndedIter::new(arena, node, first) }, - next = |head| head.previous_sibling, - next_back = |tail| tail.next_sibling, + next = Node::previous_sibling, + next_back = Node::next_sibling, ); new_iterator!( /// An iterator of the IDs of the siblings after a given node. FollowingSiblings, new = |arena, node| { - let last = arena - .get(node) - .unwrap() - .parent - .and_then(|parent_id| arena.get(parent_id)) - .and_then(|parent| parent.last_child); + let last = arena.get(node) + .and_then(Node::parent) + .and_then(|parent_id| arena.get(parent_id)?.last_child()); DoubleEndedIter::new(arena, node, last) }, - next = |head| head.next_sibling, - next_back = |tail| tail.previous_sibling, + next = Node::next_sibling, + next_back = Node::previous_sibling, ); new_iterator!( /// An iterator of the IDs of the children of a given node, in insertion order. Children, - new = |arena, node| DoubleEndedIter::new(arena, arena[node].first_child, arena[node].last_child), - next = |node| node.next_sibling, - next_back = |tail| tail.previous_sibling, + new = |arena, node| { + DoubleEndedIter::new( + arena, + arena.get(node).and_then(Node::first_child), + arena.get(node).and_then(Node::last_child) + ) + }, + next = Node::next_sibling, + next_back = Node::previous_sibling, ); #[derive(Clone)] @@ -229,7 +254,13 @@ impl Iterator for Descendants<'_, T> { fn next(&mut self) -> Option { self.0.find_map(|edge| match edge { - NodeEdge::Start(node) => Some(node), + NodeEdge::Start(node) => { + if node.is_removed(self.0.arena) { + None + } else { + Some(node) + } + } NodeEdge::End(_) => None, }) } @@ -302,7 +333,7 @@ impl NodeEdge { /// being traversed. /// /// ``` - /// # use indextree::{Arena, NodeEdge}; + /// # use indextree::{Arena, Node, NodeEdge}; /// # let mut arena = Arena::new(); /// # let n1 = arena.new_node("1".to_owned()); /// # let n1_1 = arena.new_node("1_1".to_owned()); @@ -321,9 +352,9 @@ impl NodeEdge { /// // |-- 1_2 /// // `-- 1_3 /// - /// assert_eq!(*arena[n1].get(), "1"); - /// assert_eq!(*arena[n1_1_1].get(), "1_1_1"); - /// assert_eq!(*arena[n1_3].get(), "1_3"); + /// assert_eq!(arena.get(n1).map(|n| n.get().as_str()), Some("1")); + /// assert_eq!(arena.get(n1_1_1).map(|n| n.get().as_str()), Some("1_1_1")); + /// assert_eq!(arena.get(n1_3).map(|n| n.get().as_str()), Some("1_3")); /// /// let mut next = Some(NodeEdge::Start(n1)); /// let mut count = 0; @@ -334,29 +365,31 @@ impl NodeEdge { /// NodeEdge::End(_) => continue, /// }; /// - /// arena[current].get_mut().push_str(&format!(" (count={})", count)); + /// if let Some(string) = arena.get_mut(current).map(Node::get_mut) { + /// string.push_str(&format!(" (count={count})")); + /// } /// count += 1; /// } /// - /// assert_eq!(*arena[n1].get(), "1 (count=0)"); - /// assert_eq!(*arena[n1_1_1].get(), "1_1_1 (count=2)"); - /// assert_eq!(*arena[n1_3].get(), "1_3 (count=4)"); + /// assert_eq!(arena.get(n1).map(|n| n.get().as_str()), Some("1 (count=0)")); + /// assert_eq!(arena.get(n1_1_1).map(|n| n.get().as_str()), Some("1_1_1 (count=2)")); + /// assert_eq!(arena.get(n1_3).map(|n| n.get().as_str()), Some("1_3 (count=4)")); /// ``` #[must_use] pub fn next_traverse(self, arena: &Arena) -> Option { match self { - NodeEdge::Start(node) => match arena[node].first_child { + NodeEdge::Start(node) => match arena.get(node).and_then(Node::first_child) { Some(first_child) => Some(NodeEdge::Start(first_child)), None => Some(NodeEdge::End(node)), }, NodeEdge::End(node) => { - let node = &arena[node]; - match node.next_sibling { + let node = arena.get(node); + match node.and_then(Node::next_sibling) { Some(next_sibling) => Some(NodeEdge::Start(next_sibling)), // `node.parent()` here can only be `None` if the tree has // been modified during iteration, but silently stopping // iteration seems a more sensible behavior than panicking. - None => node.parent.map(NodeEdge::End), + None => node.and_then(Node::parent).map(NodeEdge::End), } } } @@ -404,7 +437,7 @@ impl NodeEdge { /// being traversed. /// /// ``` - /// use indextree::{Arena, NodeEdge}; + /// use indextree::{Arena, Node, NodeEdge}; /// /// # let mut arena = Arena::new(); /// # let n1 = arena.new_node("1".to_owned()); @@ -424,9 +457,9 @@ impl NodeEdge { /// // |-- 1_2 /// // `-- 1_3 /// - /// assert_eq!(*arena[n1_3].get(), "1_3"); - /// assert_eq!(*arena[n1_1_1].get(), "1_1_1"); - /// assert_eq!(*arena[n1].get(), "1"); + /// assert_eq!(arena.get(n1_3).map(|n| n.get().as_str()), Some("1_3")); + /// assert_eq!(arena.get(n1_1_1).map(|n| n.get().as_str()), Some("1_1_1")); + /// assert_eq!(arena.get(n1).map(|n| n.get().as_str()), Some("1")); /// /// let mut next = Some(NodeEdge::End(n1_3)); /// let mut count = 0; @@ -437,29 +470,31 @@ impl NodeEdge { /// NodeEdge::End(_) => continue, /// }; /// - /// arena[current].get_mut().push_str(&format!(" (count={})", count)); + /// if let Some(string) = arena.get_mut(current).map(Node::get_mut) { + /// string.push_str(&format!(" (count={count})")); + /// } /// count += 1; /// } /// - /// assert_eq!(*arena[n1_3].get(), "1_3 (count=0)"); - /// assert_eq!(*arena[n1_1_1].get(), "1_1_1 (count=2)"); - /// assert_eq!(*arena[n1].get(), "1 (count=4)"); + /// assert_eq!(arena.get(n1_3).map(|n| n.get().as_str()), Some("1_3 (count=0)")); + /// assert_eq!(arena.get(n1_1_1).map(|n| n.get().as_str()), Some("1_1_1 (count=2)")); + /// assert_eq!(arena.get(n1).map(|n| n.get().as_str()), Some("1 (count=4)")); /// ``` #[must_use] pub fn prev_traverse(self, arena: &Arena) -> Option { match self { - NodeEdge::End(node) => match arena[node].last_child { + NodeEdge::End(node) => match arena.get(node).and_then(Node::last_child) { Some(last_child) => Some(NodeEdge::End(last_child)), None => Some(NodeEdge::Start(node)), }, NodeEdge::Start(node) => { - let node = &arena[node]; - match node.previous_sibling { + let node = arena.get(node); + match node.and_then(Node::previous_sibling) { Some(previous_sibling) => Some(NodeEdge::End(previous_sibling)), // `node.parent()` here can only be `None` if the tree has // been modified during iteration, but silently stopping // iteration seems a more sensible behavior than panicking. - None => node.parent.map(NodeEdge::Start), + None => node.and_then(Node::parent).map(NodeEdge::Start), } } } @@ -570,3 +605,78 @@ impl Iterator for ReverseTraverse<'_, T> { } impl core::iter::FusedIterator for ReverseTraverse<'_, T> {} + +#[cfg(test)] +mod tests { + use crate::Arena; + + #[test] + fn preceding_siblings() { + let mut arena = Arena::new(); + + let n1 = arena.new_node(1); + + let n1_1 = n1.append_value(1, &mut arena); + let n1_2 = n1.append_value(2, &mut arena); + let n1_3 = n1.append_value(3, &mut arena); + + /* + 1 + |-- 1 + |-- 2 + `-- 3 + */ + + assert_eq!(n1_1.preceding_siblings(&arena).collect::>(), [n1_1]); + assert_eq!( + n1_1.preceding_siblings(&arena).rev().collect::>(), + [n1_1] + ); + + assert_eq!( + n1_2.preceding_siblings(&arena).collect::>(), + [n1_2, n1_1] + ); + assert_eq!( + n1_2.preceding_siblings(&arena).rev().collect::>(), + [n1_1, n1_2] + ); + + assert_eq!( + n1_3.preceding_siblings(&arena).collect::>(), + [n1_3, n1_2, n1_1] + ); + assert_eq!( + n1_3.preceding_siblings(&arena).rev().collect::>(), + [n1_1, n1_2, n1_3] + ); + + n1_2.remove(&mut arena); + + /* + 1 + |-- 1 + `-- 3 + */ + + assert_eq!(n1_1.preceding_siblings(&arena).collect::>(), [n1_1]); + assert_eq!( + n1_1.preceding_siblings(&arena).rev().collect::>(), + [n1_1] + ); + + // n1_2 is removed and so does not have any preceding siblings + assert!(n1_2.preceding_siblings(&arena).next().is_none()); + assert!(n1_2.preceding_siblings(&arena).next_back().is_none()); + + // n1_2 is omitted + assert_eq!( + n1_3.preceding_siblings(&arena).collect::>(), + [n1_3, n1_1] + ); + assert_eq!( + n1_3.preceding_siblings(&arena).rev().collect::>(), + [n1_1, n1_3] + ); + } +} diff --git a/indextree/tests/lib.rs b/indextree/tests/lib.rs index 24d6e41..41af032 100644 --- a/indextree/tests/lib.rs +++ b/indextree/tests/lib.rs @@ -1,4 +1,4 @@ -use indextree::{Arena, NodeError}; +use indextree::{Arena, Node, NodeError}; #[cfg(feature = "par_iter")] use rayon::prelude::*; @@ -26,11 +26,15 @@ fn success_create() { let c = new!(); // 10 assert!(b.checked_append(c, arena).is_ok()); - arena[c].previous_sibling().unwrap().detach(arena); + arena + .get(c) + .and_then(Node::previous_sibling) + .unwrap() + .detach(arena); assert_eq!( b.descendants(arena) - .map(|node| *arena[node].get()) + .flat_map(|node| arena.get(node).map(Node::get).copied()) .collect::>(), [5, 6, 7, 1, 4, 2, 3, 9, 10] ); @@ -82,7 +86,15 @@ fn iter() { assert!(a.checked_append(d, arena).is_ok()); let node_refs = arena.iter().collect::>(); - assert_eq!(node_refs, vec![&arena[a], &arena[b], &arena[c], &arena[d]]); + assert_eq!( + node_refs, + [ + arena.get(a).unwrap(), + arena.get(b).unwrap(), + arena.get(c).unwrap(), + arena.get(d).unwrap() + ] + ); } #[test] @@ -118,7 +130,15 @@ fn par_iter() { assert!(a.checked_append(d, arena).is_ok()); let node_refs = arena.par_iter().collect::>(); - assert_eq!(node_refs, vec![&arena[a], &arena[b], &arena[c], &arena[d]]); + assert_eq!( + node_refs, + [ + arena.get(a).unwrap(), + arena.get(b).unwrap(), + arena.get(c).unwrap(), + arena.get(d).unwrap() + ] + ); } #[test] @@ -140,39 +160,21 @@ fn remove() { assert!(n2.checked_append(n6, arena).is_ok()); n2.remove(arena); - let node_refs = arena - .iter() - .filter_map(|x| { - if !x.is_removed() { - Some(*x.get()) - } else { - None - } - }) - .collect::>(); - assert_eq!(node_refs, vec![0, 1, 3, 4, 5, 6]); + let node_refs = arena.iter().map(Node::get).copied().collect::>(); + assert_eq!(node_refs, [0, 1, 3, 4, 5, 6]); assert_eq!(n2.children(arena).count(), 0); - assert_eq!(n2.descendants(arena).count(), 1); - assert_eq!(n2.preceding_siblings(arena).count(), 1); - assert_eq!(n2.following_siblings(arena).count(), 1); + assert_eq!(n2.descendants(arena).count(), 0); + assert_eq!(n2.preceding_siblings(arena).count(), 0); + assert_eq!(n2.following_siblings(arena).count(), 0); n3.remove(arena); - let node_refs = arena - .iter() - .filter_map(|x| { - if !x.is_removed() { - Some(*x.get()) - } else { - None - } - }) - .collect::>(); - assert_eq!(node_refs, vec![0, 1, 4, 5, 6]); + let node_refs = arena.iter().map(Node::get).copied().collect::>(); + assert_eq!(node_refs, [0, 1, 4, 5, 6]); assert_eq!(n3.children(arena).count(), 0); - assert_eq!(n3.descendants(arena).count(), 1); - assert_eq!(n3.preceding_siblings(arena).count(), 1); - assert_eq!(n3.following_siblings(arena).count(), 1); + assert_eq!(n3.descendants(arena).count(), 0); + assert_eq!(n3.preceding_siblings(arena).count(), 0); + assert_eq!(n3.following_siblings(arena).count(), 0); } #[test] @@ -269,19 +271,6 @@ fn reserve() { assert!(arena.capacity() >= 5); } -#[test] -#[should_panic(expected = "index out of bounds")] -fn inaccessible_node() { - let mut arena = Arena::new(); - let n1_id = arena.new_node("1"); - let n2_id = arena.new_node("2"); - arena.clear(); - assert!(arena.get(n1_id).is_none()); - let n1_id = arena.new_node("1"); - assert_eq!(*arena[n1_id].get(), "1"); - n2_id.is_removed(&arena); -} - #[test] fn prepend_value() { let mut arena = Arena::new(); @@ -300,7 +289,11 @@ fn reverse_children() { root.append_value(1, &mut arena); root.append_value(2, &mut arena); root.append_value(3, &mut arena); - let mut iter = root.children(&arena).rev().map(|n| *arena[n].get()); + let mut iter = root + .children(&arena) + .rev() + .filter_map(|id| arena.get(id).map(Node::get)) + .copied(); assert_eq!(iter.next(), Some(3)); assert_eq!(iter.next(), Some(2)); assert_eq!(iter.next(), Some(1)); @@ -330,8 +323,8 @@ fn detach_children_single_child() { root.detach_children(&mut arena); assert_eq!(root.children(&arena).count(), 0); - assert!(arena[child].parent().is_none()); - assert!(!arena[child].is_removed()); + assert!(arena.get(child).and_then(Node::parent).is_none()); + assert!(!arena.get(child).is_some_and(Node::is_removed)); } #[test] @@ -351,12 +344,15 @@ fn detach_children_preserves_parent_position() { // Parent still in its original position assert_eq!(parent.parent(&arena), Some(grandparent)); - assert_eq!(arena[parent].next_sibling(), Some(sibling)); + assert_eq!( + arena.get(parent).and_then(Node::next_sibling), + Some(sibling) + ); // Children are detached and independent - assert!(arena[c1].parent().is_none()); - assert!(arena[c1].next_sibling().is_none()); - assert!(arena[c2].parent().is_none()); - assert!(arena[c2].previous_sibling().is_none()); + assert!(arena.get(c1).and_then(Node::parent).is_none()); + assert!(arena.get(c1).and_then(Node::next_sibling).is_none()); + assert!(arena.get(c2).and_then(Node::parent).is_none()); + assert!(arena.get(c2).and_then(Node::previous_sibling).is_none()); } #[test] @@ -391,7 +387,10 @@ fn remove_children_preserves_parent_position() { // Parent still in its original position assert_eq!(parent.parent(&arena), Some(grandparent)); - assert_eq!(arena[parent].next_sibling(), Some(sibling)); + assert_eq!( + arena.get(parent).and_then(Node::next_sibling), + Some(sibling) + ); assert_eq!(parent.children(&arena).count(), 0); // All children and grandchildren removed assert!(c1.is_removed(&arena)); @@ -458,11 +457,11 @@ fn node_display() { let n2 = arena.new_node("2"); n1.append(n2, &mut arena); - let display = format!("{}", arena[n1]); + let display = arena.get(n1).map(Node::to_string).unwrap(); assert!(display.contains("no parent")); assert!(display.contains("first child")); - let display = format!("{}", arena[n2]); + let display = arena.get(n2).map(Node::to_string).unwrap(); assert!(display.contains("parent:")); assert!(display.contains("no first child")); } @@ -483,14 +482,14 @@ fn node_error_display() { fn last_child_accessor() { let mut arena = Arena::new(); let n1 = arena.new_node("1"); - assert_eq!(arena[n1].last_child(), None); + assert_eq!(arena.get(n1).and_then(Node::last_child), None); let n1_1 = arena.new_node("1_1"); n1.append(n1_1, &mut arena); let n1_2 = arena.new_node("1_2"); n1.append(n1_2, &mut arena); - assert_eq!(arena[n1].last_child(), Some(n1_2)); + assert_eq!(arena.get(n1).and_then(Node::last_child), Some(n1_2)); } #[test] @@ -505,10 +504,10 @@ fn children_reverse_iterator() { root.append(c3, &mut arena); let forward: Vec<_> = root.children(&arena).collect(); - assert_eq!(forward, vec![c1, c2, c3]); + assert_eq!(forward, [c1, c2, c3]); let backward: Vec<_> = root.children(&arena).rev().collect(); - assert_eq!(backward, vec![c3, c2, c1]); + assert_eq!(backward, [c3, c2, c1]); } #[test] @@ -523,10 +522,10 @@ fn following_siblings_reverse() { root.append(c3, &mut arena); let forward: Vec<_> = c1.following_siblings(&arena).collect(); - assert_eq!(forward, vec![c1, c2, c3]); + assert_eq!(forward, [c1, c2, c3]); let backward: Vec<_> = c1.following_siblings(&arena).rev().collect(); - assert_eq!(backward, vec![c3, c2, c1]); + assert_eq!(backward, [c3, c2, c1]); } #[test] @@ -541,10 +540,10 @@ fn preceding_siblings_reverse() { root.append(c3, &mut arena); let forward: Vec<_> = c3.preceding_siblings(&arena).collect(); - assert_eq!(forward, vec![c3, c2, c1]); + assert_eq!(forward, [c3, c2, c1]); let backward: Vec<_> = c3.preceding_siblings(&arena).rev().collect(); - assert_eq!(backward, vec![c1, c2, c3]); + assert_eq!(backward, [c1, c2, c3]); } #[test] @@ -576,16 +575,16 @@ fn panicking_wrappers() { root.prepend(c1, &mut arena); root.prepend(c2, &mut arena); let children: Vec<_> = root.children(&arena).collect(); - assert_eq!(children, vec![c2, c1]); + assert_eq!(children, [c2, c1]); c2.insert_after(c3, &mut arena); let children: Vec<_> = root.children(&arena).collect(); - assert_eq!(children, vec![c2, c3, c1]); + assert_eq!(children, [c2, c3, c1]); let c4 = arena.new_node("c4"); c3.insert_before(c4, &mut arena); let children: Vec<_> = root.children(&arena).collect(); - assert_eq!(children, vec![c2, c4, c3, c1]); + assert_eq!(children, [c2, c4, c3, c1]); } #[test] @@ -614,22 +613,6 @@ fn get_node_id_at() { assert_eq!(arena.get_node_id_at(idx2), None); } -#[test] -fn as_slice() { - let mut arena = Arena::new(); - let n1 = arena.new_node(10); - let n2 = arena.new_node(20); - let n3 = arena.new_node(30); - n1.append(n2, &mut arena); - n1.append(n3, &mut arena); - - let slice = arena.as_slice(); - assert_eq!(slice.len(), 3); - assert_eq!(*slice[0].get(), 10); - assert_eq!(*slice[1].get(), 20); - assert_eq!(*slice[2].get(), 30); -} - #[test] fn child_count() { let mut arena = Arena::new(); @@ -703,17 +686,17 @@ fn node_display_with_siblings() { root.append(c3, &mut arena); // Middle child has both previous and next siblings - let display = format!("{}", arena[c2]); + let display = arena.get(c2).map(Node::to_string).unwrap(); assert!(display.contains("previous sibling:")); assert!(display.contains("next sibling:")); // First child has next but no previous - let display = format!("{}", arena[c1]); + let display = arena.get(c1).map(Node::to_string).unwrap(); assert!(display.contains("no previous sibling")); assert!(display.contains("next sibling:")); // Last child has previous but no next - let display = format!("{}", arena[c3]); + let display = arena.get(c3).map(Node::to_string).unwrap(); assert!(display.contains("previous sibling:")); assert!(display.contains("no next sibling")); } diff --git a/indextree/tests/remove.rs b/indextree/tests/remove.rs index ae1c4a0..d0d32f9 100644 --- a/indextree/tests/remove.rs +++ b/indextree/tests/remove.rs @@ -1,5 +1,5 @@ use indextree::{ - Arena, + Arena, Node, NodeEdge::{End, Start}, }; @@ -28,7 +28,7 @@ fn toplevel_with_single_child() { n1.remove(&mut arena); // arena // `-- 1_1 - assert!(arena[n1_1].parent().is_none()); + assert!(arena.get(n1_1).and_then(Node::parent).is_none()); } #[test] @@ -58,8 +58,8 @@ fn toplevel_with_multiple_children() { // arena // |-- 1_1 // `-- 1_2 - assert!(arena[n1_1].parent().is_none()); - assert!(arena[n1_2].parent().is_none()); + assert!(arena.get(n1_1).and_then(Node::parent).is_none()); + assert!(arena.get(n1_2).and_then(Node::parent).is_none()); assert_eq!( n1_1.following_siblings(&arena).collect::>(), &[n1_1, n1_2] @@ -82,7 +82,7 @@ fn single_child_with_no_children() { n1_1.remove(&mut arena); // arena // `-- 1 - assert!(arena[n1_1].parent().is_none()); + assert!(arena.get(n1_1).and_then(Node::parent).is_none()); assert_eq!( n1.traverse(&arena).collect::>(), &[Start(n1), End(n1),] @@ -116,8 +116,8 @@ fn single_child_with_single_child() { // arena // `-- 1 // `-- 1_1_1 - assert!(arena[n1_1].parent().is_none()); - assert!(arena[n1_1].first_child().is_none()); + assert!(arena.get(n1_1).and_then(Node::parent).is_none()); + assert!(arena.get(n1_1).and_then(Node::first_child).is_none()); assert_eq!(n1_1_1.ancestors(&arena).collect::>(), &[n1_1_1, n1]); assert_eq!( n1.traverse(&arena).collect::>(),