Fix bug in remove_index for OrdMap

src/nodes/btree.rs

@@ -134,11 +134,6 @@ impl<A> Node<A> {
         self.keys.len() < MEDIAN
     }
 
-    #[inline]
-    fn is_leaf(&self) -> bool {
-        self.children[0].is_none()
-    }
-
     #[inline]
     pub(crate) fn unit(value: A) -> Self {
         Node {
@@ -643,26 +638,35 @@ impl<A: BTreeValue> Node<A> {
                 match (&self.children[index], &self.children[index + 1]) {
                     // If we're a leaf, just delete the entry.
                     (&None, &None) => RemoveAction::DeleteAt(index),
-                    // If the left hand child has capacity, pull the predecessor up.
-                    (&Some(ref left), _) if !left.too_small() => {
-                        if left.is_leaf() {
-                            RemoveAction::PullUp(left.keys.len() - 1, index, index)
-                        } else {
+                    // Right is empty. Attempt to steal from left if enough capacity, 
+                    // otherwise pull the predecessor up.
+                    (&Some(ref left), &None) => {
+                        if !left.too_small() {
                             RemoveAction::StealFromLeft(index + 1)
+                        } else {
+                            RemoveAction::PullUp(left.keys.len() - 1, index, index)
                         }
                     }
-                    // If the right hand child has capacity, pull the successor up.
-                    (_, &Some(ref right)) if !right.too_small() => {
-                        if right.is_leaf() {
-                            RemoveAction::PullUp(0, index, index + 1)
+                    // Left is empty. Attempt to steal from right if enough capacity,
+                    // otherwise pull the successor up.
+                    (&None, &Some(ref right)) => {
+                        if !right.too_small() {
+                            RemoveAction::StealFromRight(index)
                         } else {
+                            RemoveAction::PullUp(0, index, index + 1)
+                        }
+                    }
+                    // Both left and right are non empty. Attempt to steal from left or 
+                    // right if enough capacity, otherwise just merge the children.
+                    (&Some(ref left), &Some(ref right)) => {
+                        if left.has_room() && !right.too_small() {
                             RemoveAction::StealFromRight(index)
+                        } else if right.has_room() && !left.too_small() {
+                            RemoveAction::StealFromLeft(index + 1)
+                        } else {
+                            RemoveAction::Merge(index)
                         }
                     }
-                    // If neither child has capacity, we'll have to merge them.
-                    (&Some(_), &Some(_)) => RemoveAction::Merge(index),
-                    // If one child exists and the other doesn't, we're in a bad state.
-                    _ => unreachable!(),
                 }
             }
             // Key is adjacent to some key in node

src/ord/map.rs

@@ -2377,6 +2377,19 @@ mod test {
         assert_eq!(vec![(3, 4), (2, 3), (1, 2)], range);
     }
 
+    #[test]
+    fn issue_124() {
+        let mut map = OrdMap::new();
+        let contents = include_str!("../../test-fixtures/issue_124.txt");
+        for line in contents.split('\n') {
+            if line.starts_with("insert ") {
+                map.insert(line[7..].parse::<u32>().unwrap(), 0);
+            } else if line.starts_with("remove ") {
+                map.remove(&line[7..].parse::<u32>().unwrap());
+            }
+        }
+    }
+
     proptest! {
         #[test]
         fn length(ref input in collection::btree_map(i16::ANY, i16::ANY, 0..1000)) {