fix: fix a few bugs

crates/loro-core/src/container/text/tracker/content_map.rs

@@ -105,7 +105,7 @@ impl ContentMap {
                 };
 
                 if prev.is_none() {
-                    let mut prev_cursor = cursor.prev();
+                    let mut prev_cursor = cursor.prev_elem_end();
                     while let Some(prev_inner) = prev_cursor {
                         if prev_inner.as_ref().status.is_activated() {
                             let cursor = prev_inner;
@@ -119,12 +119,12 @@ impl ContentMap {
                             });
                             break;
                         }
-                        prev_cursor = prev_inner.prev();
+                        prev_cursor = prev_inner.prev_elem_end();
                     }
                 }
 
                 if next.is_none() {
-                    let mut next_cursor = cursor.next();
+                    let mut next_cursor = cursor.next_elem_start();
                     while let Some(next_inner) = next_cursor {
                         if next_inner.as_ref().status.is_activated() {
                             let mut cursor = next_inner.unwrap();
@@ -136,7 +136,7 @@ impl ContentMap {
                             });
                             break;
                         }
-                        next_cursor = next_inner.next();
+                        next_cursor = next_inner.next_elem_start();
                     }
                 }
 

crates/loro-core/src/container/text/tracker/cursor_map.rs

@@ -33,7 +33,15 @@ impl Marker {
                 debug_assert!(!node.is_deleted());
                 let position = node.children().iter().position(|x| x.contain_id(id))?;
                 // SAFETY: we just checked it is valid
-                Some(unsafe { SafeCursor::new(*ptr, position, 0, rle::rle_tree::Position::Start) })
+                Some(unsafe {
+                    SafeCursor::new(
+                        *ptr,
+                        position,
+                        0,
+                        rle::rle_tree::Position::Start,
+                        self.len(),
+                    )
+                })
             }
             Marker::Delete(_) => None,
         }
@@ -50,7 +58,7 @@ impl Marker {
                 debug_assert!(!node.is_deleted());
                 let position = node.children().iter().position(|x| x.contain_id(id))?;
                 // SAFETY: we just checked it is valid
-                Some(unsafe { SafeCursorMut::new(*ptr, position, 0, Position::Start) })
+                Some(unsafe { SafeCursorMut::new(*ptr, position, 0, Position::Start, self.len()) })
             }
             Marker::Delete(_) => None,
         }

crates/rle/src/rle_tree.rs

@@ -96,7 +96,13 @@ impl<'bump, T: Rle, A: RleTreeTrait<T>> RleTreeRaw<'bump, T, A> {
 
                     // SAFETY: result is valid
                     return Some(unsafe {
-                        SafeCursor::new(leaf.into(), result.child_index, result.offset, result.pos)
+                        SafeCursor::new(
+                            leaf.into(),
+                            result.child_index,
+                            result.offset,
+                            result.pos,
+                            0,
+                        )
                     });
                 }
             }
@@ -126,7 +132,13 @@ impl<'bump, T: Rle, A: RleTreeTrait<T>> RleTreeRaw<'bump, T, A> {
 
                     // SAFETY: result is valid
                     return Some(unsafe {
-                        SafeCursor::new(leaf.into(), result.child_index, result.offset, result.pos)
+                        SafeCursor::new(
+                            leaf.into(),
+                            result.child_index,
+                            result.offset,
+                            result.pos,
+                            0,
+                        )
                     });
                 }
             }
@@ -169,6 +181,7 @@ impl<'bump, T: Rle, A: RleTreeTrait<T>> RleTreeRaw<'bump, T, A> {
                     0,
                     0,
                     Position::Start,
+                    0,
                 )
             });
 

crates/rle/src/rle_tree/cursor.rs

@@ -9,6 +9,7 @@ pub struct UnsafeCursor<'tree, 'bump, T: Rle, A: RleTreeTrait<T>> {
     pub index: usize,
     pub offset: usize,
     pub pos: Position,
+    pub len: usize,
     _phantom: PhantomData<&'tree usize>,
 }
 
@@ -20,6 +21,7 @@ impl<'tree, 'bump, T: Rle, A: RleTreeTrait<T>> Clone for UnsafeCursor<'tree, 'bu
             index: self.index,
             pos: self.pos,
             offset: self.offset,
+            len: self.len,
             _phantom: Default::default(),
         }
     }
@@ -52,12 +54,14 @@ impl<'tree, 'bump: 'tree, T: Rle, A: RleTreeTrait<T>> UnsafeCursor<'tree, 'bump,
         index: usize,
         offset: usize,
         pos: Position,
+        len: usize,
     ) -> Self {
         Self {
             leaf,
             index,
             pos,
             offset,
+            len,
             _phantom: PhantomData,
         }
     }
@@ -107,32 +111,93 @@ impl<'tree, 'bump: 'tree, T: Rle, A: RleTreeTrait<T>> UnsafeCursor<'tree, 'bump,
                 node = node.parent.unwrap().as_mut();
                 result = node.insert_at_pos(old_node_index + 1, new);
             }
+        } else {
+            A::update_cache_internal(node);
+        }
+
+        while node.parent.is_some() {
+            node = node.parent.unwrap().as_mut();
+            A::update_cache_internal(node);
         }
     }
 
     /// # Safety
     ///
     /// we need to make sure that the cursor is still valid
-    pub unsafe fn next(&self) -> Option<Self> {
+    pub unsafe fn next_elem_start(&self) -> Option<Self> {
         let leaf = self.leaf.as_ref();
         if leaf.children.len() > self.index + 1 {
-            return Some(Self::new(self.leaf, self.index + 1, 0, Position::Start));
+            return Some(Self::new(self.leaf, self.index + 1, 0, Position::Start, 0));
         }
 
-        leaf.next.map(|next| Self::new(next, 0, 0, Position::Start))
+        leaf.next
+            .map(|next| Self::new(next, 0, 0, Position::Start, 0))
     }
 
     /// # Safety
     ///
     /// we need to make sure that the cursor is still valid
-    pub unsafe fn prev(&self) -> Option<Self> {
+    pub unsafe fn prev_elem_end(&self) -> Option<Self> {
         let leaf = self.leaf.as_ref();
         if self.index > 0 {
-            return Some(Self::new(self.leaf, self.index - 1, 0, Position::Start));
+            return Some(Self::new(self.leaf, self.index - 1, 0, Position::Start, 0));
         }
 
-        leaf.prev
+        leaf.prev.map(|prev| {
-            .map(|prev| Self::new(prev, prev.as_ref().children.len() - 1, 0, Position::Start))
+            Self::new(
+                prev,
+                prev.as_ref().children.len() - 1,
+                0,
+                Position::Start,
+                0,
+            )
+        })
+    }
+
+    /// move cursor forward
+    ///
+    /// # Safety
+    ///
+    /// self should still be valid pointer
+    unsafe fn shift(mut self, mut shift: usize) -> Option<Self> {
+        if shift == 0 {
+            return Some(self);
+        }
+
+        let mut leaf = self.leaf.as_ref();
+        println!("s");
+        while shift > 0 {
+            let diff = leaf.children[self.index].len() - self.offset;
+            leaf.check();
+            match shift.cmp(&diff) {
+                std::cmp::Ordering::Less => {
+                    self.offset += shift;
+                    self.pos = Position::Middle;
+                    return Some(self);
+                }
+                std::cmp::Ordering::Equal => {
+                    self.offset = leaf.children[self.index].len();
+                    self.pos = Position::End;
+                    return Some(self);
+                }
+                std::cmp::Ordering::Greater => {
+                    shift -= diff;
+                    if self.index == leaf.children.len() - 1 {
+                        leaf = leaf.next()?;
+                        self.leaf = leaf.into();
+                        self.index = 0;
+                        self.offset = 0;
+                        self.pos = Position::Start;
+                    } else {
+                        self.index += 1;
+                        self.offset = 0;
+                        self.pos = Position::Start;
+                    }
+                }
+            }
+        }
+
+        None
     }
 }
 
@@ -152,15 +217,15 @@ impl<'tree, 'bump: 'tree, T: Rle, A: RleTreeTrait<T>> SafeCursor<'tree, 'bump, T
     }
 
     #[inline]
-    pub fn next(&self) -> Option<Self> {
+    pub fn next_elem_start(&self) -> Option<Self> {
         // SAFETY: SafeCursor is a shared reference to the tree
-        unsafe { self.0.next().map(|x| Self(x)) }
+        unsafe { self.0.next_elem_start().map(|x| Self(x)) }
     }
 
     #[inline]
-    pub fn prev(&self) -> Option<Self> {
+    pub fn prev_elem_end(&self) -> Option<Self> {
         // SAFETY: SafeCursor is a shared reference to the tree
-        unsafe { self.0.prev().map(|x| Self(x)) }
+        unsafe { self.0.prev_elem_end().map(|x| Self(x)) }
     }
 
     #[inline]
@@ -193,8 +258,9 @@ impl<'tree, 'bump: 'tree, T: Rle, A: RleTreeTrait<T>> SafeCursor<'tree, 'bump, T
         index: usize,
         offset: usize,
         pos: Position,
+        len: usize,
     ) -> Self {
-        Self(UnsafeCursor::new(leaf, index, offset, pos))
+        Self(UnsafeCursor::new(leaf, index, offset, pos, len))
     }
 
     #[inline]
@@ -248,8 +314,9 @@ impl<'tree, 'bump: 'tree, T: Rle, A: RleTreeTrait<T>> SafeCursorMut<'tree, 'bump
         index: usize,
         offset: usize,
         pos: Position,
+        len: usize,
     ) -> Self {
-        Self(UnsafeCursor::new(leaf, index, offset, pos))
+        Self(UnsafeCursor::new(leaf, index, offset, pos, len))
     }
 
     #[inline]
@@ -281,17 +348,17 @@ impl<'tree, 'bump: 'tree, T: Rle, A: RleTreeTrait<T>> SafeCursorMut<'tree, 'bump
     }
 
     #[inline]
-    pub fn next(&self) -> Option<Self> {
+    pub fn next_elem_start(&self) -> Option<Self> {
         // SAFETY: SafeCursorMut is a exclusive reference to the tree so we are safe to
         // get a reference to the element
-        unsafe { self.0.next().map(|x| Self(x)) }
+        unsafe { self.0.next_elem_start().map(|x| Self(x)) }
     }
 
     #[inline]
-    pub fn prev(&self) -> Option<Self> {
+    pub fn prev_elem_end(&self) -> Option<Self> {
         // SAFETY: SafeCursorMut is a exclusive reference to the tree so we are safe to
         // get a reference to the element
-        unsafe { self.0.prev().map(|x| Self(x)) }
+        unsafe { self.0.prev_elem_end().map(|x| Self(x)) }
     }
 
     #[inline]
@@ -309,13 +376,28 @@ impl<'tree, 'bump: 'tree, T: Rle, A: RleTreeTrait<T>> SafeCursorMut<'tree, 'bump
         self.0.offset
     }
 
-    pub fn insert_notify<F>(&mut self, value: T, notify: &mut F)
+    /// self should be moved here, because after mutating self should be invalidate
+    pub fn insert_before_notify<F>(mut self, value: T, notify: &mut F)
     where
         F: FnMut(&T, *mut LeafNode<'_, T, A>),
     {
         // SAFETY: we know the cursor is a valid pointer
         unsafe { self.0.insert_notify(value, notify) }
     }
+
+    /// self should be moved here, because after mutating self should be invalidate
+    pub fn insert_after_notify<F>(self, value: T, notify: &mut F)
+    where
+        F: FnMut(&T, *mut LeafNode<'_, T, A>),
+    {
+        // SAFETY: we know the cursor is a valid pointer
+        unsafe {
+            self.0
+                .shift(self.0.len)
+                .unwrap()
+                .insert_notify(value, notify)
+        }
+    }
 }
 
 impl<'tree, 'bump: 'tree, T: Rle, A: RleTreeTrait<T>> AsMut<T>

crates/rle/src/rle_tree/iter.rs

@@ -37,7 +37,7 @@ impl<'tree, 'bump, T: Rle, A: RleTreeTrait<T>> IterMut<'tree, 'bump, T, A> {
         mut end: Option<SafeCursor<'tree, 'bump, T, A>>,
     ) -> Option<Self> {
         if start.0.pos == Position::After {
-            start = start.next()?
+            start = start.next_elem_start()?
         }
 
         if let Some(end_inner) = end {
@@ -45,7 +45,7 @@ impl<'tree, 'bump, T: Rle, A: RleTreeTrait<T>> IterMut<'tree, 'bump, T, A> {
                 || end_inner.0.pos == Position::End
                 || end_inner.0.pos == Position::After
             {
-                end = end_inner.next();
+                end = end_inner.next_elem_start();
             }
         }
 
@@ -75,7 +75,7 @@ impl<'tree, 'bump, T: Rle, A: RleTreeTrait<T>> Iter<'tree, 'bump, T, A> {
         mut end: Option<SafeCursor<'tree, 'bump, T, A>>,
     ) -> Option<Self> {
         if start.0.pos == Position::After {
-            start = start.next()?
+            start = start.next_elem_start()?
         }
 
         if let Some(end_inner) = end {
@@ -83,7 +83,7 @@ impl<'tree, 'bump, T: Rle, A: RleTreeTrait<T>> Iter<'tree, 'bump, T, A> {
                 || end_inner.0.pos == Position::End
                 || end_inner.0.pos == Position::After
             {
-                end = end_inner.next();
+                end = end_inner.next_elem_start();
             }
         }
 
@@ -114,7 +114,13 @@ impl<'rf, 'bump, T: Rle, A: RleTreeTrait<T>> Iterator for Iter<'rf, 'bump, T, A>
                     self.child_index += 1;
                     // SAFETY: we just checked that the child exists
                     return Some(unsafe {
-                        SafeCursor::new(node.into(), self.child_index - 1, 0, Position::Start)
+                        SafeCursor::new(
+                            node.into(),
+                            self.child_index - 1,
+                            0,
+                            Position::Start,
+                            node.children[self.child_index - 1].len(),
+                        )
                     });
                 }
                 None => match node.next() {
@@ -158,12 +164,13 @@ impl<'rf, 'bump, T: Rle, A: RleTreeTrait<T>> Iterator for IterMut<'rf, 'bump, T,
             let node_ptr = node as *const _;
             match node.children.get(self.child_index) {
                 Some(_) => {
+                    let len = node.children()[self.child_index - 1].len();
                     self.child_index += 1;
                     let leaf = node.into();
                     self.node = Some(node);
                     // SAFETY: we just checked that the child exists
                     return Some(unsafe {
-                        SafeCursorMut::new(leaf, self.child_index - 1, 0, Position::Start)
+                        SafeCursorMut::new(leaf, self.child_index - 1, 0, Position::Start, len)
                     });
                 }
                 None => match node.next_mut() {

crates/rle/src/rle_tree/node.rs

@@ -288,6 +288,19 @@ impl<'a, T: Rle, A: RleTreeTrait<T>> Node<'a, T, A> {
     }
 
     pub(crate) fn remove(&mut self) {
+        if let Some(leaf) = self.as_leaf_mut() {
+            let next = leaf.next;
+            let prev = leaf.prev;
+            if let Some(mut next) = next {
+                // SAFETY: it is safe here
+                unsafe { next.as_mut() }.prev = prev;
+            }
+            if let Some(mut prev) = prev {
+                // SAFETY: it is safe here
+                unsafe { prev.as_mut() }.next = next;
+            }
+        }
+
         let index = self.get_self_index().unwrap();
         let parent = self.parent_mut().unwrap();
         for _ in parent.children.drain(index..index + 1) {}

crates/rle/src/rle_tree/node/leaf_impl.rs

@@ -54,7 +54,15 @@ impl<'bump, T: Rle, A: RleTreeTrait<T>> LeafNode<'bump, T, A> {
         let result = A::find_pos_leaf(self, pos);
         assert!(result.found);
         // SAFETY: result.found is true
-        unsafe { SafeCursor::new(self.into(), result.child_index, result.offset, result.pos) }
+        unsafe {
+            SafeCursor::new(
+                self.into(),
+                result.child_index,
+                result.offset,
+                result.pos,
+                0,
+            )
+        }
     }
 
     #[inline]
@@ -62,7 +70,15 @@ impl<'bump, T: Rle, A: RleTreeTrait<T>> LeafNode<'bump, T, A> {
         let result = A::find_pos_leaf(self, pos);
         assert!(result.found);
         // SAFETY: result.found is true
-        unsafe { SafeCursorMut::new(self.into(), result.child_index, result.offset, result.pos) }
+        unsafe {
+            SafeCursorMut::new(
+                self.into(),
+                result.child_index,
+                result.offset,
+                result.pos,
+                0,
+            )
+        }
     }
 
     pub fn push_child<F>(
@@ -99,17 +115,22 @@ impl<'bump, T: Rle, A: RleTreeTrait<T>> LeafNode<'bump, T, A> {
         Ok(())
     }
 
-    pub(crate) fn check(&mut self) {
+    pub(crate) fn check(&self) {
         assert!(self.children.len() <= A::MAX_CHILDREN_NUM);
+        assert!(!self.is_deleted());
         A::check_cache_leaf(self);
         if let Some(next) = self.next {
             // SAFETY: this is only for testing, and next must be a valid pointer
             let self_ptr = unsafe { next.as_ref().prev.unwrap().as_ptr() };
+            // SAFETY: this is only for testing, and next must be a valid pointer
+            assert!(unsafe { !next.as_ref().is_deleted() });
             assert!(std::ptr::eq(self, self_ptr));
         }
         if let Some(prev) = self.prev {
             // SAFETY: this is only for testing, and prev must be a valid pointer
             let self_ptr = unsafe { prev.as_ref().next.unwrap().as_ptr() };
+            // SAFETY: this is only for testing, and next must be a valid pointer
+            assert!(unsafe { !prev.as_ref().is_deleted() });
             assert!(std::ptr::eq(self, self_ptr));
         }
     }
@@ -323,6 +344,12 @@ impl<'bump, T: Rle, A: RleTreeTrait<T>> LeafNode<'bump, T, A> {
         unsafe { self.prev.map(|p| p.as_ref()) }
     }
 
+    #[inline]
+    pub fn prev_mut(&mut self) -> Option<&mut Self> {
+        // SAFETY: internal variant ensure prev and next are valid reference
+        unsafe { self.prev.map(|mut p| p.as_mut()) }
+    }
+
     #[inline]
     pub fn children(&self) -> &[&'bump mut T] {
         &self.children

crates/rle/src/rle_tree/test/notify_prop_test.rs

@@ -64,8 +64,8 @@ impl Interaction {
                 };
                 _println!("Insert {{from: {}, len: {}}},", from, len);
                 if *use_cursor {
-                    if let Some(mut cursor) = tree.get_mut(from) {
+                    if let Some(cursor) = tree.get_mut(from) {
-                        cursor.insert_notify(value, notify)
+                        cursor.insert_before_notify(value, notify)
                     } else {
                         tree.insert_notify(from, value, notify);
                     }

crates/rle/src/rle_tree/test/string_prop_test.rs

@@ -157,7 +157,17 @@ impl Interaction {
             Interaction::Insert { insert_at, content } => {
                 tree.with_tree_mut(|tree| {
                     let insert_at = *insert_at % (tree.len() + 1);
-                    tree.insert(insert_at, content.clone().into());
+                    if insert_at % 3 == 0 && insert_at > 10 {
+                        let mut cursor = tree.get_mut(insert_at - 5).unwrap();
+                        cursor.0.len = 5;
+                        cursor.insert_after_notify(content.clone().into(), &mut |_a, _| {});
+                    } else {
+                        tree.node
+                            .as_internal_mut()
+                            .unwrap()
+                            .insert(insert_at, content.clone().into(), &mut |_a, _b| {})
+                            .unwrap();
+                    }
                 });
             }
             Interaction::Delete { from, len } => {
@@ -192,6 +202,48 @@ fn run_test(interactions: Vec<Interaction>) {
 
 use Interaction::*;
 
+#[test]
+fn issue_insert_after() {
+    run_test(vec![
+        Insert {
+            insert_at: 0,
+            content: "aaaaaaaa".to_string(),
+        },
+        Insert {
+            insert_at: 4040,
+            content: "aaaaaaaaaaa".to_string(),
+        },
+        Insert {
+            insert_at: 3209832,
+            content: "b".to_string(),
+        },
+    ])
+}
+
+#[test]
+fn issue_delete_0() {
+    run_test(vec![
+        Insert {
+            insert_at: 0,
+            content: "0000".into(),
+        },
+        Insert {
+            insert_at: 2,
+            content: "3333".into(),
+        },
+        // 00333300
+        Insert {
+            insert_at: 4,
+            content: "222".into(),
+        },
+        // insert: 0033[222]3300
+        // 00332223300
+        Delete { from: 1, len: 5 },
+        // delete: 0[03322]23300
+        // 023300
+    ]);
+}
+
 #[test]
 fn issue_delete() {
     run_test(vec![
@@ -252,7 +304,7 @@ fn issue_delete() {
 }
 
 #[cfg(not(no_proptest))]
-mod string_prop_test_0 {
+mod string_proptest {
     use super::*;
     use proptest::prelude::*;