From 9755782cf9b0173145c8fd2c9bef278f27f4c062 Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Fri, 12 Aug 2022 17:54:47 +0800 Subject: [PATCH] refactor: use ouroboros to self-ref previous solution has a fatal bug when dropping --- crates/rle/Cargo.toml | 2 +- crates/rle/src/rle_tree.rs | 42 ++--- .../rle/src/rle_tree/test/range_rle_test.rs | 169 +++++++++--------- crates/rle/src/rle_tree/test/string_fuzzy.rs | 19 +- 4 files changed, 117 insertions(+), 115 deletions(-) diff --git a/crates/rle/Cargo.toml b/crates/rle/Cargo.toml index 178ffcb8..0d7c25b1 100644 --- a/crates/rle/Cargo.toml +++ b/crates/rle/Cargo.toml @@ -9,7 +9,7 @@ edition = "2021" bumpalo = { version = "3.10.0", features = ["collections", "boxed"] } num = "0.4.0" enum-as-inner = "0.5.1" -owning_ref = "0.4.1" +ouroboros = "0.15.2" [dev-dependencies] color-backtrace = { version = "0.5" } diff --git a/crates/rle/src/rle_tree.rs b/crates/rle/src/rle_tree.rs index e08c8892..e4b9b8aa 100644 --- a/crates/rle/src/rle_tree.rs +++ b/crates/rle/src/rle_tree.rs @@ -2,8 +2,13 @@ use self::node::{InternalNode, Node}; use crate::{HasLength, Rle}; pub(self) use bumpalo::collections::vec::Vec as BumpVec; use bumpalo::Bump; -use owning_ref::OwningRefMut; -use std::marker::{PhantomData, PhantomPinned}; +use ouroboros::self_referencing; +use std::{ + borrow::{Borrow, BorrowMut}, + marker::{PhantomData, PhantomPinned}, + pin::Pin, + ptr::NonNull, +}; use tree_trait::RleTreeTrait; mod iter; mod node; @@ -18,34 +23,21 @@ pub struct RleTreeRaw<'a, T: Rle, A: RleTreeTrait> { _a: PhantomData<(A, T)>, } -type TreeRef = - OwningRefMut, RleTreeRaw<'static, T, A>)>, RleTreeRaw<'static, T, A>>; - +#[self_referencing] pub struct RleTree + 'static> { - tree: TreeRef, -} - -impl + 'static> RleTree { - pub fn new() -> Self { - let bump = Box::new(Bump::new()); - let tree = RleTreeRaw::new(unsafe { &*(&*bump as *const _) }); - let m = OwningRefMut::new(Box::new((bump, tree))); - let tree = m.map_mut(|(_, tree)| tree); - Self { tree } - } - - pub fn get_ref(&self) -> &RleTreeRaw<'static, T, A> { - self.tree.as_ref() - } - - pub fn get_mut(&mut self) -> &mut RleTreeRaw<'static, T, A> { - self.tree.as_mut() - } + bump: Bump, + #[borrows(bump)] + #[not_covariant] + tree: RleTreeRaw<'this, T, A>, } impl + 'static> Default for RleTree { fn default() -> Self { - Self::new() + RleTreeBuilder { + bump: Bump::new(), + tree_builder: |bump| RleTreeRaw::new(bump), + } + .build() } } diff --git a/crates/rle/src/rle_tree/test/range_rle_test.rs b/crates/rle/src/rle_tree/test/range_rle_test.rs index c72f719b..ef368c9d 100644 --- a/crates/rle/src/rle_tree/test/range_rle_test.rs +++ b/crates/rle/src/rle_tree/test/range_rle_test.rs @@ -103,118 +103,125 @@ fn get_pos(index: usize, child: &T) -> Position { #[test] fn insert() { - let mut t: RleTree, RangeTreeTrait> = RleTree::new(); - let tree = t.get_mut(); - tree.insert(0, 0..1); - tree.insert(1, 4..8); - tree.insert(5, 8..10); - tree.insert(3, 101..108); - tree.insert(2, 200..208); - assert_eq!(tree.len(), 22); + let mut t: RleTree, RangeTreeTrait> = RleTree::default(); + t.with_tree_mut(|tree| { + tree.insert(0, 0..1); + tree.insert(1, 4..8); + tree.insert(5, 8..10); + tree.insert(3, 101..108); + tree.insert(2, 200..208); + assert_eq!(tree.len(), 22); - let ans = vec![0..1, 4..5, 200..208, 5..6, 101..108, 6..10]; + let ans = vec![0..1, 4..5, 200..208, 5..6, 101..108, 6..10]; - for (actual, expected) in tree.iter().zip(ans.iter()) { - assert_eq!(actual, expected); - } + for (actual, expected) in tree.iter().zip(ans.iter()) { + assert_eq!(actual, expected); + } + }) } #[test] fn delete() { - let mut t: RleTree, RangeTreeTrait> = RleTree::new(); - let tree = t.get_mut(); - tree.insert(0, 0..10); - tree.delete_range(Some(4), Some(5)); - assert_eq!(tree.len(), 9); + let mut t: RleTree, RangeTreeTrait> = RleTree::default(); + t.with_tree_mut(|tree| { + tree.insert(0, 0..10); + tree.delete_range(Some(4), Some(5)); + assert_eq!(tree.len(), 9); - let ans = vec![0..4, 5..10]; - for (actual, expected) in tree.iter().zip(ans.iter()) { - assert_eq!(actual, expected); - } + let ans = vec![0..4, 5..10]; + for (actual, expected) in tree.iter().zip(ans.iter()) { + assert_eq!(actual, expected); + } + }) } #[test] fn insert_50times() { - let mut t: RleTree, RangeTreeTrait> = RleTree::new(); - let tree = t.get_mut(); - for i in (0..100).step_by(2) { - assert_eq!(tree.len(), i / 2); - tree.insert(tree.len(), i..i + 1); - tree.debug_check(); - } + let mut t: RleTree, RangeTreeTrait> = RleTree::default(); + t.with_tree_mut(|tree| { + for i in (0..100).step_by(2) { + assert_eq!(tree.len(), i / 2); + tree.insert(tree.len(), i..i + 1); + tree.debug_check(); + } + }); } #[test] fn deletion_that_need_merge_to_sibling() { - let mut t: RleTree, RangeTreeTrait> = RleTree::new(); - let tree = t.get_mut(); - for i in (0..18).step_by(2) { - tree.insert(tree.len(), i..i + 1); - } + let mut t: RleTree, RangeTreeTrait> = RleTree::default(); + t.with_tree_mut(|tree| { + for i in (0..18).step_by(2) { + tree.insert(tree.len(), i..i + 1); + } - tree.delete_range(Some(1), Some(tree.len() - 1)); - tree.debug_check(); + tree.delete_range(Some(1), Some(tree.len() - 1)); + tree.debug_check(); + }); } #[test] fn delete_that_need_borrow_from_sibling() { - let mut t: RleTree, RangeTreeTrait> = RleTree::new(); - let tree = t.get_mut(); - for i in (0..16).step_by(2) { - tree.insert(tree.len(), i..i + 1); - } - tree.delete_range(Some(2), Some(3)); - // Left [ 0..1, 2..3, 6..7 ] - // Right [8..9, 10..11, 12..13, 14..15] - - tree.delete_range(Some(1), Some(2)); - { - // Left [ 0..1, 6..7 ] + let mut t: RleTree, RangeTreeTrait> = RleTree::default(); + t.with_tree_mut(|tree| { + for i in (0..16).step_by(2) { + tree.insert(tree.len(), i..i + 1); + } + tree.delete_range(Some(2), Some(3)); + // Left [ 0..1, 2..3, 6..7 ] // Right [8..9, 10..11, 12..13, 14..15] - let left = &tree.node.as_internal().unwrap().children[0]; - assert_eq!(left.as_leaf().unwrap().cache, 2); - let right = &tree.node.as_internal().unwrap().children[1]; - assert_eq!(right.as_leaf().unwrap().cache, 4); - } - tree.delete_range(Some(1), Some(2)); - { - // Left [ 0..1, 8..9, 10..11 ] - // Right [12..13, 14..15] - let left = &tree.node.as_internal().unwrap().children[0]; - assert_eq!(left.as_leaf().unwrap().cache, 3); - let right = &tree.node.as_internal().unwrap().children[1]; - assert_eq!(right.as_leaf().unwrap().cache, 2); - } + tree.delete_range(Some(1), Some(2)); + { + // Left [ 0..1, 6..7 ] + // Right [8..9, 10..11, 12..13, 14..15] + let left = &tree.node.as_internal().unwrap().children[0]; + assert_eq!(left.as_leaf().unwrap().cache, 2); + let right = &tree.node.as_internal().unwrap().children[1]; + assert_eq!(right.as_leaf().unwrap().cache, 4); + } - let expected = [0..1, 8..9, 10..11, 12..13, 14..15]; - for (actual, expected) in tree.iter().zip(expected.iter()) { - assert_eq!(actual, expected); - } + tree.delete_range(Some(1), Some(2)); + { + // Left [ 0..1, 8..9, 10..11 ] + // Right [12..13, 14..15] + let left = &tree.node.as_internal().unwrap().children[0]; + assert_eq!(left.as_leaf().unwrap().cache, 3); + let right = &tree.node.as_internal().unwrap().children[1]; + assert_eq!(right.as_leaf().unwrap().cache, 2); + } - tree.debug_check(); + let expected = [0..1, 8..9, 10..11, 12..13, 14..15]; + for (actual, expected) in tree.iter().zip(expected.iter()) { + assert_eq!(actual, expected); + } + + tree.debug_check(); + }); } #[test] fn delete_that_causes_removing_layers() { - let mut t: RleTree, RangeTreeTrait> = RleTree::new(); - let tree = t.get_mut(); - for i in (0..128).step_by(2) { - tree.insert(tree.len(), i..i + 1); - } - tree.debug_check(); - tree.delete_range(Some(1), None); + let mut t: RleTree, RangeTreeTrait> = RleTree::default(); + t.with_tree_mut(|tree| { + for i in (0..128).step_by(2) { + tree.insert(tree.len(), i..i + 1); + } + tree.debug_check(); + tree.delete_range(Some(1), None); + }); } #[test] fn delete_that_causes_increase_levels() { - let mut t: RleTree, RangeTreeTrait> = RleTree::new(); - let tree = t.get_mut(); - tree.insert(0, 0..100); - for i in 0..50 { - tree.delete_range(Some(i), Some(i + 1)); - tree.debug_check(); - } + let mut t: RleTree, RangeTreeTrait> = RleTree::default(); + t.with_tree_mut(|tree| { + tree.insert(0, 0..100); + for i in 0..50 { + tree.delete_range(Some(i), Some(i + 1)); + tree.debug_check(); + } - dbg!(tree); + dbg!(tree); + }); } diff --git a/crates/rle/src/rle_tree/test/string_fuzzy.rs b/crates/rle/src/rle_tree/test/string_fuzzy.rs index f7e57f8c..c2361c79 100644 --- a/crates/rle/src/rle_tree/test/string_fuzzy.rs +++ b/crates/rle/src/rle_tree/test/string_fuzzy.rs @@ -146,11 +146,13 @@ fn get_pos(index: usize, child: &T) -> Position { impl Display for RleTree { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - for s in self.get_ref().iter() { - f.write_str(s.0.as_str())?; - } + self.with_tree(|tree| { + for s in tree.iter() { + f.write_str(s.0.as_str())?; + } - Ok(()) + Ok(()) + }) } } @@ -168,10 +170,11 @@ impl From<&str> for CustomString { #[test] fn basic_string_op() { - let mut tree: RleTree = RleTree::new(); - let handler = tree.get_mut(); - handler.insert(0, "test".into()); - handler.insert(0, "hello ".into()); + let mut tree: RleTree = RleTree::default(); + tree.with_tree_mut(|tree| { + tree.insert(0, "test".into()); + tree.insert(0, "hello ".into()); + }); let m = format!("{}", tree); assert_eq!(m, "hello test"); }