From: Chris Mikkelson Date: Tue, 23 Jul 2024 20:15:21 +0000 (-0500) Subject: Generic sources working after big refactor. X-Git-Url: https://git.mikk.net/?a=commitdiff_plain;h=4954779c07519da03e79ef588d029452623e72d6;p=mtbl-rs Generic sources working after big refactor. Root cause was Box defaults to Box, 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; --- diff --git a/src/lib.rs b/src/lib.rs index 9a79bf0..62ddf6b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -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; diff --git a/src/merger.rs b/src/merger.rs index 66e9200..630616d 100644 --- a/src/merger.rs +++ b/src/merger.rs @@ -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 { sources: Vec, - phantom: PhantomData<&'a S>, } -impl<'a, S, I> From for Merger<'a, S> +impl From for Merger where - S: Source<'a>, + S: Source, I: IntoIterator, { fn from(i: I) -> Self { Merger { sources: Vec::from_iter(i), - phantom: PhantomData, } } } @@ -138,27 +135,27 @@ impl Iter for MergeIter { } } -impl<'a, S: Source<'a>> IterSource<'a> for Merger<'a, S> { +impl IterSource for Merger { type It = MergeIter; - 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 Source for Merger { type Get = MergeIter; type Prefix = MergeIter; type Range = MergeIter; - 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::::new(); for i in range { v.extend(tnum(i)) diff --git a/src/source.rs b/src/source.rs index d1e98a1..6836b93 100644 --- a/src/source.rs +++ b/src/source.rs @@ -1,5 +1,6 @@ use crate::Entry; use std::iter::Iterator; +use std::marker::PhantomData; pub trait Iter: Iterator { fn seek(&mut self, key: &[u8]); @@ -11,28 +12,28 @@ impl<'a> Iter for Box { } } -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 Source for S { type Get = RangeIter; type Prefix = PrefixIter; type Range = RangeIter; - 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 Iter for RangeIter { } } +pub type GenIter<'a> = Box; +trait GenSource<'a>: + Source, 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>); +impl<'a> GenericSource<'a> { + fn from(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); @@ -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 = test_source().into(); - // - // same as above - // let b: Box = 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; - 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::::new(); - // s2 = Vec::::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::::new(); let ts = test_source(); - let ws = SourceWrapper(ts, PhantomData); - v.push(GSource(&ws)); - //let gs: &dyn GenSource = &ws; + let mut v = Vec::::new(); + let s = GenericSource::from(&ts); + v.push(s); let gs = Merger::from(v); gs.iter(); }