Merge pull request #17 from SimonSapin/nonzero

Use NonZeroUsize inside NodeId

Author: causal-agent

src/iter.rs

@@ -1,4 +1,5 @@
-use std::{slice, vec};
+use std::{mem, slice, vec};
+use std::num::NonZeroUsize;
 use std::ops::Range;
 
 use {Tree, NodeId, Node, NodeRef};
@@ -77,43 +78,56 @@ impl<'a, T: 'a> Clone for Nodes<'a, T> {
     }
 }
 impl<'a, T: 'a> ExactSizeIterator for Nodes<'a, T> { }
+impl<'a, T: 'a> Nodes<'a, T> {
+    unsafe fn from_index(&self, i: usize) -> NodeRef<'a, T> {
+        self.tree.get_unchecked(NodeId(NonZeroUsize::new_unchecked(i)))
+    }
+}
 impl<'a, T: 'a> Iterator for Nodes<'a, T> {
     type Item = NodeRef<'a, T>;
     fn next(&mut self) -> Option<Self::Item> {
-        self.iter.next().map(|i| unsafe { self.tree.get_unchecked(NodeId(i)) })
+        // Safety: `i` is in `1..self.vec.len()`, so it is non-zero and in bounds.
+        self.iter.next().map(|i| unsafe { self.from_index(i) })
     }
     fn size_hint(&self) -> (usize, Option<usize>) {
         self.iter.size_hint()
     }
 }
 impl<'a, T: 'a> DoubleEndedIterator for Nodes<'a, T> {
     fn next_back(&mut self) -> Option<Self::Item> {
-        self.iter.next_back().map(|i| unsafe { self.tree.get_unchecked(NodeId(i)) })
+        // Safety: `i` is in `1..self.vec.len()`, so it is non-zero and in bounds.
+        self.iter.next_back().map(|i| unsafe { self.from_index(i) })
     }
 }
 
 impl<T> IntoIterator for Tree<T> {
     type Item = T;
     type IntoIter = IntoIter<T>;
     fn into_iter(self) -> Self::IntoIter {
-        IntoIter(self.vec.into_iter())
+        let mut iter = self.vec.into_iter();
+        // Don’t yield the uninitialized node at index 0 or run its destructor.
+        mem::forget(iter.next());
+        IntoIter(iter)
     }
 }
 
 impl<T> Tree<T> {
     /// Returns an iterator over values in insert order.
     pub fn values(&self) -> Values<T> {
-        Values(self.vec.iter())
+        // Skip over the uninitialized node at index 0
+        Values(self.vec[1..].iter())
     }
 
     /// Returns a mutable iterator over values in insert order.
     pub fn values_mut(&mut self) -> ValuesMut<T> {
-        ValuesMut(self.vec.iter_mut())
+        // Skip over the uninitialized node at index 0
+        ValuesMut(self.vec[1..].iter_mut())
     }
 
     /// Returns an iterator over nodes in insert order.
     pub fn nodes(&self) -> Nodes<T> {
-        Nodes { tree: self, iter: 0..self.vec.len() }
+        // Skip over the uninitialized node at index 0
+        Nodes { tree: self, iter: 1..self.vec.len() }
     }
 }
 

src/lib.rs

@@ -36,20 +36,49 @@
 )]
 
 use std::fmt::{self, Debug, Formatter};
+use std::num::NonZeroUsize;
 
 /// Vec-backed ID-tree.
 ///
 /// Always contains at least a root node.
-#[derive(Clone, PartialEq, Eq, Hash)]
 pub struct Tree<T> {
+    // Safety note: node at index 0 is uninitialized!
     vec: Vec<Node<T>>,
 }
 
+impl<T> Clone for Tree<T> where T: Clone {
+    fn clone(&self) -> Self {
+        let mut vec = Vec::with_capacity(self.vec.len());
+        // See Tree::with_capacity
+        unsafe {
+            vec.set_len(1);
+        }
+        vec.extend(self.vec[1..].iter().cloned());
+        Tree { vec }
+    }
+}
+
+impl<T> std::hash::Hash for Tree<T> where T: std::hash::Hash {
+    fn hash<H>(&self, state: &mut H) where H: std::hash::Hasher {
+        self.vec[1..].hash(state)
+    }
+}
+
+impl<T> Eq for Tree<T> where T: Eq {}
+impl<T> PartialEq for Tree<T> where T: PartialEq {
+    fn eq(&self, other: &Self) -> bool {
+        self.vec[1..] == other.vec[1..]
+    }
+}
+
+
 /// Node ID.
 ///
 /// Index into a `Tree`-internal `Vec`.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub struct NodeId(usize);
+pub struct NodeId(NonZeroUsize);
+
+const ROOT: NodeId = NodeId(unsafe { NonZeroUsize::new_unchecked(1) });
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 struct Node<T> {
@@ -60,6 +89,13 @@ struct Node<T> {
     value: T,
 }
 
+fn _static_assert_size_of_node() {
+    // "Instanciating" the generic `transmute` function without calling it
+    // still triggers the magic compile-time check
+    // that input and output types have the same `size_of()`.
+    let _ = std::mem::transmute::<Node<()>, [usize; 5]>;
+}
+
 impl<T> Node<T> {
     fn new(value: T) -> Self {
         Node {
@@ -113,33 +149,42 @@ impl<'a, T: 'a> PartialEq for NodeRef<'a, T> {
 impl<T> Tree<T> {
     /// Creates a tree with a root node.
     pub fn new(root: T) -> Self {
-        Tree { vec: vec![Node::new(root)] }
+        Self::with_capacity(root, 1)
     }
 
     /// Creates a tree with a root node and the specified capacity.
     pub fn with_capacity(root: T, capacity: usize) -> Self {
-        let mut vec = Vec::with_capacity(capacity);
+        let mut vec = Vec::with_capacity(capacity.saturating_add(1));
+        // The node at index 0 is unused and uninitialized.
+        // This allows using NonZeroUsize directly as an index.
+        //
+        // Safety: we requested at least 1 of capacity, so this is in bounds.
+        // It is up to the rest of the crate to not access this uninitialized node.
+        unsafe {
+            vec.set_len(1);
+        }
+        // The root node is at index 1
         vec.push(Node::new(root));
         Tree { vec }
     }
 
     /// Returns a reference to the specified node.
     pub fn get(&self, id: NodeId) -> Option<NodeRef<T>> {
-        self.vec.get(id.0).map(|node| NodeRef { id, node, tree: self })
+        self.vec.get(id.0.get()).map(|node| NodeRef { id, node, tree: self })
     }
 
     /// Returns a mutator of the specified node.
     pub fn get_mut(&mut self, id: NodeId) -> Option<NodeMut<T>> {
-        let exists = self.vec.get(id.0).map(|_| ());
+        let exists = self.vec.get(id.0.get()).map(|_| ());
         exists.map(move |_| NodeMut { id, tree: self })
     }
 
     unsafe fn node(&self, id: NodeId) -> &Node<T> {
-        self.vec.get_unchecked(id.0)
+        self.vec.get_unchecked(id.0.get())
     }
 
     unsafe fn node_mut(&mut self, id: NodeId) -> &mut Node<T> {
-        self.vec.get_unchecked_mut(id.0)
+        self.vec.get_unchecked_mut(id.0.get())
     }
 
     /// Returns a reference to the specified node.
@@ -154,17 +199,19 @@ impl<T> Tree<T> {
 
     /// Returns a reference to the root node.
     pub fn root(&self) -> NodeRef<T> {
-        unsafe { self.get_unchecked(NodeId(0)) }
+        unsafe { self.get_unchecked(ROOT) }
     }
 
     /// Returns a mutator of the root node.
     pub fn root_mut(&mut self) -> NodeMut<T> {
-        unsafe { self.get_unchecked_mut(NodeId(0)) }
+        unsafe { self.get_unchecked_mut(ROOT) }
     }
 
     /// Creates an orphan node.
     pub fn orphan(&mut self, value: T) -> NodeMut<T> {
-        let id = NodeId(self.vec.len());
+        // Safety: vec.len() starts at 2 in Self::with_capacity and never shrinks,
+        // so it is non-zero.
+        let id = NodeId(unsafe { NonZeroUsize::new_unchecked(self.vec.len()) });
         self.vec.push(Node::new(value));
         unsafe { self.get_unchecked_mut(id) }
     }
@@ -628,8 +675,8 @@ macro_rules! tree {
 impl<T: Debug> Debug for Tree<T> {
     fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> {
         use iter::Edge;
+        write!(f, "Tree {{")?;
         if f.alternate() {
-            write!(f, "Tree {{")?;
             for edge in self.root().traverse() {
                 match edge {
                     Edge::Open(node) if node.has_children() => {
@@ -653,7 +700,12 @@ impl<T: Debug> Debug for Tree<T> {
             }
             write!(f, " }}")
         } else {
-            f.debug_struct("Tree").field("vec", &self.vec).finish()
+            write!(f, "Tree {{ [<uninitialized>")?;
+            for node in &self.vec[1..] {
+                write!(f, ", ")?;
+                node.fmt(f)?;
+            }
+            write!(f, "] }}")
         }
     }
 }