7 files changed, 108 insertions(+), 71 deletions(-)

A src/debug.rs
M src/index.rs
M src/internal.rs
M src/iter.rs
M src/lib.rs
M tests/corner_cases.rs
M tests/serialization.rs

@@ 0,0 1,30 @@
+use std::fmt;
+
+use crate::{Author, Chronofold, LogIndex};
+
+impl<A: Author, T: fmt::Debug> Chronofold<A, T> {
+    pub fn formatted_log(&self) -> String {
+        let mut result = format!(
+            "{:<4} | {:<4} | {:<4} | {:<4} | change\n",
+            "idx", "ref", "next", "del"
+        );
+        for (idx, change) in self.log.iter().enumerate() {
+            let log_idx = LogIndex(idx);
+            let formatted_ref = format_option(self.references.get(&log_idx));
+            let next = format_option(self.next_indices.get(&log_idx));
+            let del = format_option(change.1);
+            result += &format!(
+                "{:<4} | {:<4} | {:<4} | {:<4} | {:?}\n",
+                idx, formatted_ref, next, del, change.0
+            );
+        }
+        result
+    }
+}
+
+fn format_option<T: fmt::Display>(option: Option<T>) -> String {
+    match option {
+        Some(t) => format!("{}", t),
+        None => "".to_owned(),
+    }
+}

@@ 15,7 15,7 @@ impl<A: Author, T> Index<LogIndex> for Chronofold<A, T> {
     type Output = Change<T>;
 
     fn index(&self, index: LogIndex) -> &Self::Output {
-        &self.log[index.0]
+        &self.log[index.0].0
     }
 }
 


@@ 42,11 42,11 @@ impl<A: Author, T> Chronofold<A, T> {
     ///   1. `index` is the first index (causal order).
     ///   2. `index` is out of bounds.
     pub(crate) fn index_before(&self, index: LogIndex) -> Option<LogIndex> {
-        if matches!(self.log.get(index.0), Some(Change::Root)) {
+        if matches!(self.log.get(index.0).map(|e| &e.0), Some(Change::Root)) {
             Some(index)
         } else if let Some(reference) = self.references.get(&index) {
             self.iter_log_indices_causal_range(reference..index)
-                .map(|(_, idx)| idx)
+                .map(|(_, idx, _)| idx)
                 .last()
         } else {
             None

@@ 2,8 2,6 @@ use crate::index::{IndexShift, RelativeNextIndex};
 use crate::offsetmap::Offset;
 use crate::{Author, Change, Chronofold, LogIndex, Timestamp};
 
-use std::matches;
-
 impl<A: Author, T> Chronofold<A, T> {
     pub(crate) fn next_log_index(&self) -> LogIndex {
         LogIndex(self.log.len())


@@ 16,19 14,18 @@ impl<A: Author, T> Chronofold<A, T> {
         change: &Change<T>,
     ) -> Option<LogIndex> {
         match (reference, change) {
-            (_, Change::Delete) => reference, // deletes have priority
-            (None, Change::Root) => reference,
+            (None, Change::Root) => None,
             (_, Change::Root) => {
                 // Roots cannot reference other entries.
                 // XXX: Should we cover this by the type system?
                 unreachable!()
             }
             (Some(reference), _change) => {
-                if let Some((_, idx)) = self
+                if let Some((_, idx, _)) = self
                     .iter_log_indices_causal_range(reference..)
-                    .filter(|(_, i)| self.references.get(i) == Some(reference))
-                    .filter(|(c, i)| {
-                        matches!(c, Change::Delete) || self.timestamp(*i).unwrap() > id
+                    .filter(|(_, i, _)| {
+                        self.references.get(i) == Some(reference)
+                            && self.timestamp(*i).unwrap() > id
                     })
                     .last()
                 {


@@ 66,8 63,12 @@ impl<A: Author, T> Chronofold<A, T> {
             next_index = None;
         }
 
+        if let (Change::Delete, Some(deleted)) = (&change, reference) {
+            self.mark_as_deleted(deleted, new_index);
+        }
+
         // Append to the chronofold's log and secondary logs.
-        self.log.push(change);
+        self.log.push((change, None));
         self.next_indices.set(new_index, next_index);
         self.authors.set(new_index, id.1);
         self.index_shifts


@@ 98,10 99,7 @@ impl<A: Author, T> Chronofold<A, T> {
         let mut last_id = None;
         let mut last_next_index = None;
 
-        let mut predecessor = match self.find_last_delete(reference) {
-            Some(idx) => idx,
-            None => reference,
-        };
+        let mut predecessor = reference;
 
         let mut changes = changes.into_iter();
         if let Some(first_change) = changes.next() {


@@ 114,7 112,11 @@ impl<A: Author, T> Chronofold<A, T> {
             last_next_index = Some(self.next_indices.get(&predecessor));
             self.next_indices.set(predecessor, Some(new_index));
 
-            self.log.push(first_change);
+            if let Change::Delete = &first_change {
+                self.mark_as_deleted(predecessor, new_index);
+            }
+
+            self.log.push((first_change, None));
             self.authors.set(new_index, author);
             self.index_shifts.set(new_index, IndexShift(0));
             self.references.set(new_index, Some(predecessor));


@@ 127,8 129,12 @@ impl<A: Author, T> Chronofold<A, T> {
             let id = Timestamp(new_index, author);
             last_id = Some(id);
 
+            if let Change::Delete = &change {
+                self.mark_as_deleted(predecessor, new_index);
+            }
+
             // Append to the chronofold's log and secondary logs.
-            self.log.push(change);
+            self.log.push((change, None));
 
             predecessor = new_index;
         }


@@ 142,13 148,10 @@ impl<A: Author, T> Chronofold<A, T> {
         }
     }
 
-    pub(crate) fn find_last_delete(&self, reference: LogIndex) -> Option<LogIndex> {
-        self.iter_log_indices_causal_range(reference..)
-            .skip(1)
-            .filter(|(c, idx)| {
-                matches!(c, Change::Delete) && self.references.get(idx) == Some(reference)
-            })
-            .last()
-            .map(|(_, idx)| idx)
+    fn mark_as_deleted(&mut self, index: LogIndex, deletion: LogIndex) {
+        self.log[index.0].1 = Some(match self.log[index.0].1 {
+            None => deletion,
+            Some(other_deletion) => LogIndex(usize::min(deletion.0, other_deletion.0)),
+        })
     }
 }

@@ 3,7 3,9 @@ use std::marker::PhantomData;
 use std::matches;
 use std::ops::{Bound, Range, RangeBounds};
 
-use crate::{Author, Change, Chronofold, FromLocalValue, LogIndex, Op, OpPayload};
+use crate::{
+    Author, Change, Chronofold, EarliestDeletion, FromLocalValue, LogIndex, Op, OpPayload,
+};
 
 impl<A: Author, T> Chronofold<A, T> {
     /// Returns an iterator over the log indices in causal order.


@@ 40,7 42,7 @@ impl<A: Author, T> Chronofold<A, T> {
     pub(crate) fn iter_subtree(&self, root: LogIndex) -> impl Iterator<Item = LogIndex> + '_ {
         let mut subtree: HashSet<LogIndex> = HashSet::new();
         self.iter_log_indices_causal_range(root..)
-            .filter_map(move |(_, idx)| {
+            .filter_map(move |(_, idx, _)| {
                 if idx == root || subtree.contains(&self.references.get(&idx)?) {
                     subtree.insert(idx);
                     Some(idx)


@@ 60,12 62,8 @@ impl<A: Author, T> Chronofold<A, T> {
     where
         R: RangeBounds<LogIndex>,
     {
-        let mut causal_iter = self.iter_log_indices_causal_range(range);
-        let current = causal_iter.next();
-        Iter {
-            causal_iter,
-            current,
-        }
+        let causal_iter = self.iter_log_indices_causal_range(range);
+        Iter { causal_iter }
     }
 
     /// Returns an iterator over elements in causal order.


@@ 74,7 72,7 @@ impl<A: Author, T> Chronofold<A, T> {
     }
 
     /// Returns an iterator over changes in log order.
-    pub fn iter_changes(&self) -> impl Iterator<Item = &Change<T>> {
+    pub fn iter_changes(&self) -> impl Iterator<Item = &(Change<T>, EarliestDeletion)> {
         self.log.iter()
     }
 


@@ 112,13 110,14 @@ pub(crate) struct CausalIter<'a, A, T> {
 }
 
 impl<'a, A: Author, T> Iterator for CausalIter<'a, A, T> {
-    type Item = (&'a Change<T>, LogIndex);
+    type Item = (&'a Change<T>, LogIndex, EarliestDeletion);
 
     fn next(&mut self) -> Option<Self::Item> {
         match self.current.take() {
             Some(current) if Some(current) != self.first_excluded => {
                 self.current = self.cfold.index_after(current);
-                Some((&self.cfold.log[current.0], current))
+                let (change, deleted) = &self.cfold.log[current.0];
+                Some((change, current, *deleted))
             }
             _ => None,
         }


@@ 131,7 130,6 @@ impl<'a, A: Author, T> Iterator for CausalIter<'a, A, T> {
 /// `Chronofold`. See its documentation for more.
 pub struct Iter<'a, A, T> {
     causal_iter: CausalIter<'a, A, T>,
-    current: Option<(&'a Change<T>, LogIndex)>,
 }
 
 impl<'a, A: Author, T> Iterator for Iter<'a, A, T> {


@@ 139,23 137,17 @@ impl<'a, A: Author, T> Iterator for Iter<'a, A, T> {
 
     fn next(&mut self) -> Option<Self::Item> {
         loop {
-            let (skipped, next) =
-                skip_while(&mut self.causal_iter, |(c, _)| matches!(c, Change::Delete));
-            if skipped == 0 {
-                // the current item is not deleted
-                match self.current.take() {
-                    None => {
-                        return None;
-                    }
-                    Some((Change::Insert(v), idx)) => {
-                        self.current = next;
-                        return Some((v, idx));
-                    }
-                    _ => unreachable!(),
+            let next = skip_while(&mut self.causal_iter, |(c, _, deleted)| {
+                matches!(c, Change::Delete) || deleted.is_some()
+            });
+            match next {
+                None => {
+                    return None;
+                }
+                Some((Change::Insert(v), idx, _)) => {
+                    return Some((v, idx));
                 }
-            } else {
-                // the current item is deleted
-                self.current = next;
+                _ => unreachable!(),
             }
         }
     }


@@ 189,7 181,7 @@ where
                 .timestamp(r)
                 .expect("references of already applied ops have to exist")
         });
-        let payload = match &self.cfold.log[idx.0] {
+        let payload = match &self.cfold.log[idx.0].0 {
             Change::Root => OpPayload::Root,
             Change::Insert(v) => OpPayload::Insert(reference, V::from_local_value(v, self.cfold)),
             Change::Delete => OpPayload::Delete(reference.expect("deletes must have a reference")),


@@ 202,21 194,20 @@ where
 ///
 /// Note that while this works like `Iterator::skip_while`, it does not create
 /// a new iterator. Instead `iter` is modified.
-fn skip_while<I, P>(iter: &mut I, predicate: P) -> (usize, Option<I::Item>)
+fn skip_while<I, P>(iter: &mut I, predicate: P) -> Option<I::Item>
 where
     I: Iterator,
     P: Fn(&I::Item) -> bool,
 {
-    let mut skipped = 0;
     loop {
         match iter.next() {
             Some(item) if !predicate(&item) => {
-                return (skipped, Some(item));
+                return Some(item);
             }
             None => {
-                return (skipped, None);
+                return None;
             }
-            _ => skipped += 1,
+            _ => {}
         }
     }
 }


@@ 271,11 262,11 @@ mod tests {
     fn skip_while() {
         let mut iter = 2..10;
         let result = super::skip_while(&mut iter, |i| i < &7);
-        assert_eq!((5, Some(7)), result);
+        assert_eq!(Some(7), result);
         assert_eq!(vec![8, 9], iter.collect::<Vec<_>>());
 
         let mut iter2 = 2..10;
         let result = super::skip_while(&mut iter2, |i| i < &20);
-        assert_eq!((8, None), result);
+        assert_eq!(None, result);
     }
 }

@@ 72,6 72,7 @@
 // private. This keeps things simple for our users and gives us more
 // flexibility in restructuring the crate.
 mod change;
+mod debug;
 mod distributed;
 mod error;
 mod fmt;


@@ 132,7 133,7 @@ extern crate serde;
 #[derive(PartialEq, Eq, Clone, Debug)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub struct Chronofold<A, T> {
-    log: Vec<Change<T>>,
+    log: Vec<(Change<T>, EarliestDeletion)>,
     root: LogIndex,
     #[cfg_attr(
         feature = "serde",


@@ 149,6 150,8 @@ pub struct Chronofold<A, T> {
     index_shifts: RangeFromMap<LogIndex, IndexShift>,
 }
 
+pub type EarliestDeletion = Option<LogIndex>;
+
 impl<A: Author, T> Chronofold<A, T> {
     /// Constructs a new, empty chronofold.
     pub fn new(author: A) -> Self {


@@ 164,7 167,7 @@ impl<A: Author, T> Chronofold<A, T> {
         let mut references = OffsetMap::default();
         references.set(root_idx, None);
         Self {
-            log: vec![Change::Root],
+            log: vec![(Change::Root, None)],
             root: LogIndex(0),
             version,
             next_indices,


@@ 188,7 191,7 @@ impl<A: Author, T> Chronofold<A, T> {
     ///
     /// If `index` is out of bounds, `None` is returned.
     pub fn get(&self, index: LogIndex) -> Option<&Change<T>> {
-        self.log.get(index.0)
+        self.log.get(index.0).map(|e| &e.0)
     }
 
     /// Creates an editing session for a single author.

@@ 89,6 89,16 @@ fn concurrent_insertion_deletion() {
 }
 
 #[test]
+fn concurrent_inserts_referencing_deletions() {
+    let mutate = |s: &mut Session<u8, char>| {
+        s.remove(LogIndex(2));
+        let delete_idx = LogIndex(3);
+        s.insert_after(delete_idx, '3');
+    };
+    assert_concurrent_eq("133", "12", mutate, mutate);
+}
+
+#[test]
 fn insert_referencing_deleted_element() {
     let mut cfold = Chronofold::<u8, char>::default();
     let mut session = cfold.session(1);


@@ 128,13 138,13 @@ where
     assert_eq!(
         expected,
         format!("{}", cfold_left),
-        "Left ops:\n{:#?}",
-        cfold_left.iter_ops(..).collect::<Vec<Op<_, &char>>>(),
+        "\n{}",
+        cfold_left.formatted_log(),
     );
     assert_eq!(
         expected,
         format!("{}", cfold_right),
-        "Right ops:\n{:#?}",
-        cfold_right.iter_ops(..).collect::<Vec<Op<_, &char>>>()
+        "\n{}",
+        cfold_right.formatted_log(),
     );
 }

@@ 13,7 13,7 @@ fn roundtrip() {
 #[test]
 fn empty() {
     let cfold = Chronofold::<usize, char>::default();
-    assert_json_max_len(&cfold, 166);
+    assert_json_max_len(&cfold, 173);
 }
 
 #[test]


@@ 23,7 23,7 @@ fn local_edits_only() {
     cfold
         .session(1)
         .splice(LogIndex(6)..LogIndex(11), "cfold".chars());
-    assert_json_max_len(&cfold, 616);
+    assert_json_max_len(&cfold, 767);
 }
 
 fn assert_json_max_len(cfold: &Chronofold<usize, char>, max_len: usize) {