]> git.mikk.net Git - mtbl-rs/commitdiff
Generic sources working after big refactor.
authorChris Mikkelson <cmikk@fsi.io>
Tue, 23 Jul 2024 20:15:21 +0000 (15:15 -0500)
committerChris Mikkelson <cmikk@fsi.io>
Tue, 23 Jul 2024 20:21:20 +0000 (15:21 -0500)
Root cause was Box<dyn Trait> defaults to Box<dyn Trait + 'static>,
leading to the 'static lifetime "infecting" the generated iterators,
causing the borrow to last beyond the end of the function which dropped
the box or its containing structure.

The quick fix was to inject and add an explicit lifetime to the Box.

While tuning that fix, opted to inject lifetimes by implementing
Source methods on references. This tracks more with the C mtbl API,
where:

m = mtbl_merger_init(mopt);
/* populate m */
mtbl_merger_source(m);

is roughly mirrored by:

let m = mtbl::Merger::from(...);
let source = &m;

src/lib.rs
src/merger.rs
src/source.rs

index 9a79bf064648d2bdaca65454f6cf829db3637933..62ddf6bedcd01b2f079ee6f64b7e44892d7c2537 100644 (file)
@@ -1,9 +1,9 @@
-#![allow(dead_code)]
 pub mod entry;
 use entry::Entry;
 
 pub mod source;
-use source::{Iter, Source};
+use source::Iter;
+use source::Source;
 
 pub mod merger;
 //use merger::Merger;
index 66e920013fa84aaa5e4c1b0cb6d411e7421a1c53..630616d1dc16e065df0ebce219a509dd95eaee63 100644 (file)
@@ -1,22 +1,19 @@
 use crate::{source::IterSource, Entry, Iter, Source};
 use std::cmp::Ordering;
 use std::collections::BinaryHeap;
-use std::marker::PhantomData;
 
-pub struct Merger<'a, S: Source<'a>> {
+pub struct Merger<S: Source> {
     sources: Vec<S>,
-    phantom: PhantomData<&'a S>,
 }
 
-impl<'a, S, I> From<I> for Merger<'a, S>
+impl<S, I> From<I> for Merger<S>
 where
-    S: Source<'a>,
+    S: Source,
     I: IntoIterator<Item = S>,
 {
     fn from(i: I) -> Self {
         Merger {
             sources: Vec::from_iter(i),
-            phantom: PhantomData,
         }
     }
 }
@@ -138,27 +135,27 @@ impl<I: Iter> Iter for MergeIter<I> {
     }
 }
 
-impl<'a, S: Source<'a>> IterSource<'a> for Merger<'a, S> {
+impl<S: Source> IterSource for Merger<S> {
     type It = MergeIter<S::It>;
-    fn iter(&'a self) -> Self::It {
+    fn iter(&self) -> Self::It {
         MergeIter::from(self.sources.iter().map(|s| s.iter()))
     }
 }
 
-impl<'a, S: Source<'a>> Source<'a> for Merger<'a, S> {
+impl<S: Source> Source for Merger<S> {
     type Get = MergeIter<S::Get>;
     type Prefix = MergeIter<S::Prefix>;
     type Range = MergeIter<S::Range>;
 
-    fn get(&'a self, key: &[u8]) -> Self::Get {
+    fn get(&self, key: &[u8]) -> Self::Get {
         MergeIter::from(self.sources.iter().map(|s| s.get(key)))
     }
 
-    fn get_prefix(&'a self, prefix: &[u8]) -> Self::Prefix {
+    fn get_prefix(&self, prefix: &[u8]) -> Self::Prefix {
         MergeIter::from(self.sources.iter().map(|s| s.get_prefix(prefix)))
     }
 
-    fn get_range(&'a self, start: &[u8], end: &[u8]) -> Self::Range {
+    fn get_range(&self, start: &[u8], end: &[u8]) -> Self::Range {
         MergeIter::from(self.sources.iter().map(|s| s.get_range(start, end)))
     }
 }
@@ -183,7 +180,8 @@ mod test {
     #[test]
     fn test_merge() {
         let range = 1..8;
-        let s = Merger::from(range.clone().into_iter().map(test_source));
+        let iters: Vec<_> = range.clone().into_iter().map(test_source).collect();
+        let s = Merger::from(iters.iter());
         let mut v = Vec::<u8>::new();
         for i in range {
             v.extend(tnum(i))
index d1e98a18e6bb20d50abb6259ca48c9b39aec1a04..6836b931441aa3f2d52b8b92f461e6eff4c2e7cb 100644 (file)
@@ -1,5 +1,6 @@
 use crate::Entry;
 use std::iter::Iterator;
+use std::marker::PhantomData;
 
 pub trait Iter: Iterator<Item = Entry> {
     fn seek(&mut self, key: &[u8]);
@@ -11,28 +12,28 @@ impl<'a> Iter for Box<dyn Iter + 'a> {
     }
 }
 
-pub trait IterSource<'a> {
+pub trait IterSource {
     type It: Iter;
-    fn iter(&'a self) -> Self::It;
+    fn iter(&self) -> Self::It;
 }
-pub trait Source<'a>: IterSource<'a> {
+pub trait Source: IterSource {
     type Get: Iter;
     type Prefix: Iter;
     type Range: Iter;
 
-    fn get(&'a self, key: &[u8]) -> Self::Get;
-    fn get_prefix(&'a self, prefix: &[u8]) -> Self::Prefix;
-    fn get_range(&'a self, start: &[u8], end: &[u8]) -> Self::Range;
+    fn get(&self, key: &[u8]) -> Self::Get;
+    fn get_prefix(&self, prefix: &[u8]) -> Self::Prefix;
+    fn get_range(&self, start: &[u8], end: &[u8]) -> Self::Range;
 }
 
-pub trait DefaultSource<'a>: IterSource<'a> {}
+pub trait DefaultSource: IterSource {}
 
-impl<'a, S: DefaultSource<'a>> Source<'a> for S {
+impl<S: DefaultSource> Source for S {
     type Get = RangeIter<S::It>;
     type Prefix = PrefixIter<S::It>;
     type Range = RangeIter<S::It>;
 
-    fn get(&'a self, key: &[u8]) -> Self::Get {
+    fn get(&self, key: &[u8]) -> Self::Get {
         let mut res = RangeIter {
             iter: self.iter(),
             start: Vec::from(key),
@@ -42,7 +43,7 @@ impl<'a, S: DefaultSource<'a>> Source<'a> for S {
         res
     }
 
-    fn get_prefix(&'a self, prefix: &[u8]) -> Self::Prefix {
+    fn get_prefix(&self, prefix: &[u8]) -> Self::Prefix {
         let mut res = PrefixIter {
             iter: self.iter(),
             prefix: Vec::from(prefix),
@@ -51,7 +52,7 @@ impl<'a, S: DefaultSource<'a>> Source<'a> for S {
         res
     }
 
-    fn get_range(&'a self, start: &[u8], end: &[u8]) -> Self::Range {
+    fn get_range(&self, start: &[u8], end: &[u8]) -> Self::Range {
         let mut res = RangeIter {
             iter: self.iter(),
             start: Vec::from(start),
@@ -116,11 +117,65 @@ impl<I: Iter> Iter for RangeIter<I> {
     }
 }
 
+pub type GenIter<'a> = Box<dyn Iter + 'a>;
+trait GenSource<'a>:
+    Source<It = GenIter<'a>, Get = GenIter<'a>, Prefix = GenIter<'a>, Range = GenIter<'a>> + 'a
+{
+}
+
+struct GenWrap<'a, S: Source + 'a>(S, PhantomData<&'a S>);
+impl<'a, S: Source + 'a> IterSource for GenWrap<'a, S> {
+    type It = GenIter<'a>;
+    fn iter(&self) -> Self::It {
+        Box::new(self.0.iter())
+    }
+}
+impl<'a, S: Source + 'a> Source for GenWrap<'a, S> {
+    type Get = GenIter<'a>;
+    fn get(&self, key: &[u8]) -> Self::Get {
+        Box::new(self.0.get(key))
+    }
+    type Prefix = GenIter<'a>;
+    fn get_prefix(&self, prefix: &[u8]) -> Self::Get {
+        Box::new(self.0.get_prefix(prefix))
+    }
+    type Range = GenIter<'a>;
+    fn get_range(&self, start: &[u8], end: &[u8]) -> Self::Get {
+        Box::new(self.0.get_range(start, end))
+    }
+}
+impl<'a, S: Source + 'a> GenSource<'a> for GenWrap<'a, S> {}
+
+pub struct GenericSource<'a>(Box<dyn GenSource<'a>>);
+impl<'a> GenericSource<'a> {
+    fn from<S: Source + 'a>(source: S) -> Self {
+        Self(Box::new(GenWrap(source, PhantomData)))
+    }
+}
+impl<'a> IterSource for GenericSource<'a> {
+    type It = GenIter<'a>;
+    fn iter(&self) -> Self::It {
+        self.0.as_ref().iter()
+    }
+}
+impl<'a> Source for GenericSource<'a> {
+    type Get = GenIter<'a>;
+    fn get(&self, key: &[u8]) -> Self::Get {
+        self.0.get(key)
+    }
+    type Prefix = GenIter<'a>;
+    fn get_prefix(&self, prefix: &[u8]) -> Self::Prefix {
+        self.0.get_prefix(prefix)
+    }
+    type Range = GenIter<'a>;
+    fn get_range(&self, start: &[u8], end: &[u8]) -> Self::Prefix {
+        self.0.get_range(start, end)
+    }
+}
+
 #[cfg(test)]
 pub mod test {
-    use std::marker::PhantomData;
-
-    use super::{DefaultSource, Entry, Iter, IterSource, Source};
+    use super::{DefaultSource, Entry, GenericSource, Iter, IterSource, Source};
 
     pub struct TestSource(pub Vec<Entry>);
 
@@ -152,17 +207,17 @@ pub mod test {
         }
     }
 
-    impl<'a> IterSource<'a> for TestSource {
+    impl<'a> IterSource for &'a TestSource {
         type It = TestIter<'a>;
 
-        fn iter(&'a self) -> TestIter<'a> {
+        fn iter(&self) -> TestIter<'a> {
             TestIter {
                 source: self,
                 off: 0,
             }
         }
     }
-    impl<'a> DefaultSource<'a> for TestSource {}
+    impl<'a> DefaultSource for &'a TestSource {}
 
     fn test_source() -> TestSource {
         TestSource(vec![
@@ -174,37 +229,10 @@ pub mod test {
         ])
     }
 
-    fn chk<'a, S: Source<'a>>(s: S) -> S {
-        s
-    }
-
     #[test]
     fn test_source_iter() {
-        // works
-        //let s = test_source();
+        let s = &test_source();
 
-        // works
-        //let w = Box::new(SourceWrapper(test_source(), PhantomData));
-        //let s: &dyn GenSource = w.as_ref();
-
-        // also works
-        // let w = SourceWrapper(test_source(), PhantomData);
-        // let s: &dyn GenSource = &w;
-
-        let ts = test_source();
-        let ws = SourceWrapper(ts, PhantomData);
-        let s: &dyn GenSource = &ws;
-
-        //let s = test_source();
-
-        // borrowed value 's' doesn't live long enough'
-        // box.iter() consumes! Ding
-        // let s: Box<dyn GenSource> = test_source().into();
-        //
-        // same as above
-        // let b: Box<dyn GenSource> = test_source().into();
-        // let s = b.as_ref();
-        //
         assert_eq!(
             Vec::from_iter(s.iter().map(|e| e.value[0])),
             vec![0, 1, 2, 3, 4]
@@ -226,66 +254,39 @@ pub mod test {
         );
     }
 
-    type GenIter<'a> = Box<dyn Iter + 'a>;
-    trait GenSource<'a>:
-        Source<'a, It = GenIter<'a>, Get = GenIter<'a>, Prefix = GenIter<'a>, Range = GenIter<'a>>
-    {
-    }
-    struct SourceWrapper<'a, S: Source<'a>>(S, PhantomData<&'a S>);
-    impl<'a, S: Source<'a>> IterSource<'a> for SourceWrapper<'a, S> {
-        type It = GenIter<'a>;
-        fn iter(&'a self) -> Self::It {
-            Box::new(self.0.iter())
-        }
-    }
-    impl<'a, S: Source<'a>> Source<'a> for SourceWrapper<'a, S> {
-        type Get = GenIter<'a>;
-        fn get(&'a self, key: &[u8]) -> Self::Get {
-            Box::new(self.0.get(key))
-        }
-        type Prefix = GenIter<'a>;
-        fn get_prefix(&'a self, prefix: &[u8]) -> Self::Prefix {
-            Box::new(self.0.get_prefix(prefix))
-        }
-        type Range = GenIter<'a>;
-        fn get_range(&'a self, start: &[u8], end: &[u8]) -> Self::Range {
-            Box::new(self.0.get_range(start, end))
-        }
-    }
-    impl<'a, S: Source<'a>> GenSource<'a> for SourceWrapper<'a, S> {}
+    #[test]
+    fn test_generic_source_iter() {
+        let ts = test_source();
+        let s = GenericSource::from(&ts);
 
-    struct GSource<'a>(&'a dyn GenSource<'a>);
-    impl<'a> IterSource<'a> for GSource<'a> {
-        type It = GenIter<'a>;
-        fn iter(&'a self) -> Self::It {
-            self.0.iter()
-        }
+        assert_eq!(
+            Vec::from_iter(s.iter().map(|e| e.value[0])),
+            vec![0, 1, 2, 3, 4]
+        );
+        assert_eq!(
+            Vec::from_iter(s.get(vec![0, 0, 1, 0].as_slice()).map(|e| e.value[0])),
+            vec![2]
+        );
+        assert_eq!(
+            Vec::from_iter(s.get_prefix(vec![0, 0].as_slice()).map(|e| e.value[0])),
+            vec![0, 1, 2]
+        );
+        assert_eq!(
+            Vec::from_iter(
+                s.get_range(vec![0, 0, 0, 1].as_slice(), vec![0, 1, 0, 0].as_slice())
+                    .map(|e| e.value[0])
+            ),
+            vec![1, 2, 3]
+        );
     }
-    impl<'a> DefaultSource<'a> for GSource<'a> {}
-
-    // Note: lesson learned here is:
-    //   1) Box lifetime is still testing my understanding
-    //   2) Mixed mergers / generic iteration is still doable
-    //      with a struct wrapping `&'a dyn GenSource<'a>`,
-    //      requiring the concrete Source to be separately
-    //      managed.
-    //   3) In practice, this would amount to
-    //        s1 = Vec::<Source1>::new();
-    //        s2 = Vec::<Source2>::new();
-    //        ws1 = s1.map(|s| GenWrap::from(s));
-    //        ws2 = s2.map(|s| GenWrap::from(s));
-    //        combined = Vec::new()
-    //        ws1.for_each(|s| combined.push(GenSource(s)));
-    //        ws2.for_each(|s| combined.push(GenSource(s)));
-    //        s = Merger::from(combined);
+
     #[test]
     fn test_dyn_source() {
         use crate::merger::Merger;
-        let mut v = Vec::<GSource>::new();
         let ts = test_source();
-        let ws = SourceWrapper(ts, PhantomData);
-        v.push(GSource(&ws));
-        //let gs: &dyn GenSource = &ws;
+        let mut v = Vec::<GenericSource>::new();
+        let s = GenericSource::from(&ts);
+        v.push(s);
         let gs = Merger::from(v);
         gs.iter();
     }