From a93d9f762d78dff472640ed54320d57766fcfa72 Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Wed, 12 Oct 2022 16:09:08 +0800 Subject: [PATCH] fix: cursor should not use Deref `crdt-list` expected derefed cursor to be a sliced operation, but it was not. so I updated crdt-list to use a GetOp trait instead of Deref, where users may slice the op if they want to. --- Cargo.lock | 5 +- crates/loro-core/Cargo.toml | 2 +- crates/loro-core/fuzz/Cargo.lock | 5 +- crates/loro-core/fuzz/Cargo.toml | 2 +- .../src/container/text/tracker/content_map.rs | 2 +- .../src/container/text/tracker/y_span.rs | 6 +- .../src/container/text/tracker/yata.rs | 79 +++++++++++-------- crates/loro-core/src/dag/iter.rs | 2 +- crates/loro-core/src/version.rs | 15 +++- crates/rle/Cargo.toml | 1 + crates/rle/src/rle_tree/cursor.rs | 25 +++--- .../rle/src/rle_tree/test/range_rle_test.rs | 2 +- 12 files changed, 87 insertions(+), 59 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2e15c9ca..9bdb525a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -161,9 +161,7 @@ dependencies = [ [[package]] name = "crdt-list" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "44c6c20076c40cfc4fc484455bda1ba3cac6f9585b2e5a464b98fc4a2600f95b" +version = "0.2.1" dependencies = [ "arbitrary", "arref", @@ -789,6 +787,7 @@ dependencies = [ "arref", "bumpalo", "color-backtrace", + "crdt-list", "ctor", "enum-as-inner", "num", diff --git a/crates/loro-core/Cargo.toml b/crates/loro-core/Cargo.toml index 454cc20f..d259dea7 100644 --- a/crates/loro-core/Cargo.toml +++ b/crates/loro-core/Cargo.toml @@ -19,7 +19,7 @@ thiserror = "1.0.31" im = "15.1.0" enum-as-inner = "0.5.1" num = "0.4.0" -crdt-list = "0.2.0" +crdt-list = { version = "0.2.1", path="../../../crdt-list" } rand = { version = "0.8.5", optional = true } [dev-dependencies] diff --git a/crates/loro-core/fuzz/Cargo.lock b/crates/loro-core/fuzz/Cargo.lock index 15bb0a03..64f2b155 100644 --- a/crates/loro-core/fuzz/Cargo.lock +++ b/crates/loro-core/fuzz/Cargo.lock @@ -79,9 +79,7 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" [[package]] name = "crdt-list" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "44c6c20076c40cfc4fc484455bda1ba3cac6f9585b2e5a464b98fc4a2600f95b" +version = "0.2.1" dependencies = [ "arbitrary", "arref", @@ -527,6 +525,7 @@ version = "0.1.0" dependencies = [ "arref", "bumpalo", + "crdt-list", "enum-as-inner", "num", "ouroboros", diff --git a/crates/loro-core/fuzz/Cargo.toml b/crates/loro-core/fuzz/Cargo.toml index 04fe9816..86152be3 100644 --- a/crates/loro-core/fuzz/Cargo.toml +++ b/crates/loro-core/fuzz/Cargo.toml @@ -9,7 +9,7 @@ edition = "2018" cargo-fuzz = true [dependencies] -crdt-list = "0.2.0" +crdt-list = { version = "0.2.1", path = "../../../../crdt-list" } libfuzzer-sys = "0.4" [dependencies.loro-core] diff --git a/crates/loro-core/src/container/text/tracker/content_map.rs b/crates/loro-core/src/container/text/tracker/content_map.rs index ea2c48a3..a953a4c0 100644 --- a/crates/loro-core/src/container/text/tracker/content_map.rs +++ b/crates/loro-core/src/container/text/tracker/content_map.rs @@ -144,7 +144,7 @@ impl ContentMap { pub fn get_id_spans(&self, pos: usize, len: usize) -> RleVec { let mut ans = RleVec::new(); for cursor in self.iter_range(pos, Some(pos + len)) { - let id = cursor.id; + let id = cursor.as_ref().id; let cursor = cursor.unwrap(); ans.push(IdSpan::new( id.client_id, diff --git a/crates/loro-core/src/container/text/tracker/y_span.rs b/crates/loro-core/src/container/text/tracker/y_span.rs index 780c5f7f..5eb29753 100644 --- a/crates/loro-core/src/container/text/tracker/y_span.rs +++ b/crates/loro-core/src/container/text/tracker/y_span.rs @@ -1,3 +1,5 @@ +use std::thread::panicking; + use crate::{id::Counter, span::IdSpan, ContentType, InsertContent, ID}; use rle::{rle_tree::tree_trait::CumulateTreeTrait, HasLength, Mergable, Sliceable}; @@ -129,7 +131,7 @@ impl Sliceable for YSpan { len: to - from, status: self.status.clone(), } - } else { + } else if to == self.content_len() { YSpan { origin_left: Some(self.id.inc(from as i32 - 1)), origin_right: self.origin_right, @@ -137,6 +139,8 @@ impl Sliceable for YSpan { len: to - from, status: self.status.clone(), } + } else { + unreachable!("`to` is greater than content_len") } } } diff --git a/crates/loro-core/src/container/text/tracker/yata.rs b/crates/loro-core/src/container/text/tracker/yata.rs index f970524e..c8a94856 100644 --- a/crates/loro-core/src/container/text/tracker/yata.rs +++ b/crates/loro-core/src/container/text/tracker/yata.rs @@ -1,10 +1,11 @@ use crdt_list::{ - crdt::{ListCrdt, OpSet}, + crdt::{GetOp, ListCrdt, OpSet}, yata::Yata, }; use rle::{ range_map::{RangeMap, WithStartEnd}, rle_tree::{iter::IterMut, SafeCursorMut}, + Sliceable, }; use crate::id::{Counter, ID}; @@ -223,8 +224,8 @@ pub mod fuzz { if aa != bb { dbg!(aa.vec()); dbg!(bb.vec()); - dbg!(a); - dbg!(b); + dbg!(&a.content); + dbg!(&b.content); } assert_eq!(aa, bb); @@ -246,14 +247,15 @@ pub mod fuzz { container: &mut Self::Container, pos: usize, ) -> Self::OpUnit { - container.content.get_yspan_at_pos( + let ans = container.content.get_yspan_at_pos( ID::new( container.client_id, *container.vv.get(&container.client_id).unwrap_or(&0), ), pos % container.content.len(), pos % 10 + 1, - ) + ); + ans } type DeleteOp = RleVec; @@ -319,11 +321,6 @@ pub mod fuzz { pos: 3, len: 3, }, - Delete { - client_id: 1, - pos: 1, - len: 4, - }, NewOp { client_id: 0, pos: 4, @@ -346,22 +343,23 @@ pub mod fuzz { }, Delete { client_id: 1, - pos: 4, - len: 4, + pos: 1, + len: 1, }, NewOp { - client_id: 1, - pos: 0, + client_id: 0, + pos: 1, }, + Sync { from: 1, to: 0 }, + Sync { from: 0, to: 1 }, Delete { client_id: 1, pos: 0, len: 2, }, - Delete { - client_id: 0, + NewOp { + client_id: 1, pos: 0, - len: 4, }, ], ) @@ -381,7 +379,7 @@ pub mod fuzz { len: 11429747308416114334, }, NewOp { - client_id: 4872331909652192926, + client_id: 4872506250964672158, pos: 11429747308416114334, }, NewOp { @@ -401,23 +399,42 @@ pub mod fuzz { pos: 18446744073709551615, }, Delete { - client_id: 7451037802331897855, + client_id: 12796479807323897855, + pos: 7450921, + len: 11429747308416114176, + }, + NewOp { + client_id: 18275218707659529886, + pos: 10811735328793034751, + }, + Sync { from: 29105, to: 0 }, + Sync { + from: 16565750046338121728, + to: 18446744069414584549, + }, + Delete { + client_id: 18446744073709551615, + pos: 18446744004990074879, + len: 18446744073709551615, + }, + Delete { + client_id: 9, + pos: 0, + len: 18446229502267752447, + }, + Delete { + client_id: 18446744073709551615, + pos: 9223367630218330111, + len: 18446742974332141567, + }, + Delete { + client_id: 7451205583484485631, pos: 7451037802321897319, len: 7451037802321897319, }, NewOp { - client_id: 7451037802321897319, - pos: 16529055059114682215, - }, - Delete { - client_id: 648103854079, - pos: 7133702213043879935, - len: 18446744069565579237, - }, - Delete { - client_id: 16638239752757634710, - pos: 18446744073288476390, - len: 7133701809771642879, + client_id: 18446743620969457511, + pos: 18446744073702670335, }, ]; diff --git a/crates/loro-core/src/dag/iter.rs b/crates/loro-core/src/dag/iter.rs index 63837c09..2562248d 100644 --- a/crates/loro-core/src/dag/iter.rs +++ b/crates/loro-core/src/dag/iter.rs @@ -147,7 +147,7 @@ impl<'a, T: DagNode> Iterator for DagIteratorVV<'a, T> { } if dep.id_start() != dep_id { - vv.as_mut().unwrap().set_end(dep_id); + vv.as_mut().unwrap().set_max(dep_id); } } diff --git a/crates/loro-core/src/version.rs b/crates/loro-core/src/version.rs index 6a3a08a3..37cb3e0f 100644 --- a/crates/loro-core/src/version.rs +++ b/crates/loro-core/src/version.rs @@ -91,9 +91,16 @@ impl VersionVector { Self(ImHashMap::new()) } + /// set the inclusive ending point. target id will be included by self + #[inline] + pub fn set_max(&mut self, id: ID) { + self.0.insert(id.client_id, id.counter + 1); + } + + /// set the exclusive ending point. target id will NOT be included by self #[inline] pub fn set_end(&mut self, id: ID) { - self.0.insert(id.client_id, id.counter + 1); + self.0.insert(id.client_id, id.counter); } /// update the end counter of the given client, if the end is greater @@ -170,7 +177,7 @@ impl From> for VersionVector { fn from(vec: Vec) -> Self { let mut vv = VersionVector::new(); for id in vec { - vv.set_end(id); + vv.set_max(id); } vv @@ -211,8 +218,8 @@ mod tests { #[test] fn im() { let mut a = VersionVector::new(); - a.set_end(ID::new(1, 1)); - a.set_end(ID::new(2, 1)); + a.set_max(ID::new(1, 1)); + a.set_max(ID::new(2, 1)); let mut b = a.clone(); b.merge(&vec![ID::new(1, 2), ID::new(2, 2)].into()); assert!(a != b); diff --git a/crates/rle/Cargo.toml b/crates/rle/Cargo.toml index 9dd76b52..8d31d567 100644 --- a/crates/rle/Cargo.toml +++ b/crates/rle/Cargo.toml @@ -11,6 +11,7 @@ num = "0.4.0" enum-as-inner = "0.5.1" ouroboros = "0.15.2" arref = "0.1.0" +crdt-list = { version = "0.2.1", path = "../../../crdt-list" } [dev-dependencies] color-backtrace = { version = "0.5" } diff --git a/crates/rle/src/rle_tree/cursor.rs b/crates/rle/src/rle_tree/cursor.rs index 819e9918..7337b4c8 100644 --- a/crates/rle/src/rle_tree/cursor.rs +++ b/crates/rle/src/rle_tree/cursor.rs @@ -1,6 +1,8 @@ use std::{hash::Hash, marker::PhantomData, ops::Deref, ptr::NonNull}; -use crate::{Rle, RleTreeTrait}; +use crdt_list::crdt::GetOp; + +use crate::{HasLength, Rle, RleTreeTrait}; use super::{node::LeafNode, tree_trait::Position}; @@ -373,6 +375,12 @@ impl<'tree, T: Rle, A: RleTreeTrait> SafeCursorMut<'tree, T, A> { } } +impl<'tree, T: Rle, A: RleTreeTrait> HasLength for SafeCursorMut<'tree, T, A> { + fn len(&self) -> usize { + self.0.len + } +} + impl<'tree, T: Rle, A: RleTreeTrait> SafeCursorMut<'tree, T, A> { /// # Safety /// @@ -446,7 +454,7 @@ impl<'tree, T: Rle, A: RleTreeTrait> SafeCursorMut<'tree, T, A> { } /// self should be moved here, because after mutating self should be invalidate - pub fn insert_before_notify(mut self, value: T, notify: &mut F) + pub fn insert_before_notify(self, value: T, notify: &mut F) where F: FnMut(&T, *mut LeafNode<'_, T, A>), { @@ -498,18 +506,11 @@ impl<'tree, T: Rle, A: RleTreeTrait> AsMut for SafeCursorMut<'tree, T, A> } } -impl<'tree, T: Rle, A: RleTreeTrait> Deref for SafeCursor<'tree, T, A> { +impl<'tree, T: Rle, A: RleTreeTrait> GetOp for SafeCursorMut<'tree, T, A> { type Target = T; - fn deref(&self) -> &Self::Target { - self.as_ref() - } -} - -impl<'tree, T: Rle, A: RleTreeTrait> Deref for SafeCursorMut<'tree, T, A> { - type Target = T; - - fn deref(&self) -> &Self::Target { + fn get_op(&self) -> Self::Target { self.as_ref() + .slice(self.offset(), self.offset() + self.len()) } } 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 a5707aa5..2fffa526 100644 --- a/crates/rle/src/rle_tree/test/range_rle_test.rs +++ b/crates/rle/src/rle_tree/test/range_rle_test.rs @@ -69,7 +69,7 @@ fn update_at_cursor() { } } - let arr: Vec<_> = tree.iter().map(|x| (*x).clone()).collect(); + let arr: Vec<_> = tree.iter().map(|x| (*x.as_ref()).clone()).collect(); assert_eq!( arr, vec![0..3, 4..5, 7..10, 11..12, 13..15, 16..20, 21..30, 31..40]