Skip to content

Replace covariant PartialOrd with reborrow() #500

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/trace/cursor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub trait Cursor {
/// Key by which updates are indexed.
type Key<'a>: Copy + Clone + Ord;
/// Values associated with keys.
type Val<'a>: Copy + Clone + Ord + for<'b> PartialOrd<Self::Val<'b>>;
type Val<'a>: Copy + Clone + Ord;
/// Timestamps associated with updates
type Time: Timestamp + Lattice + Ord + Clone;
/// Associated update.
Expand Down
2 changes: 2 additions & 0 deletions src/trace/implementations/huffman_container.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ impl<B: Ord + Clone + 'static> PushInto<Vec<B>> for HuffmanContainer<B> {
impl<B: Ord + Clone + 'static> BatchContainer for HuffmanContainer<B> {
type ReadItem<'a> = Wrapped<'a, B>;

fn reborrow<'b, 'a: 'b>(item: Self::ReadItem<'a>) -> Self::ReadItem<'b> { item }

fn copy(&mut self, item: Self::ReadItem<'_>) {
match item.decode() {
Ok(decoded) => {
Expand Down
21 changes: 15 additions & 6 deletions src/trace/implementations/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,8 @@ impl BatchContainer for OffsetList {
}
}

fn reborrow<'b, 'a: 'b>(item: Self::ReadItem<'a>) -> Self::ReadItem<'b> { item }

fn with_capacity(size: usize) -> Self {
Self::with_capacity(size)
}
Expand Down Expand Up @@ -427,11 +429,11 @@ where
}

fn key_eq(this: &&K::Owned, other: <<K as PreferredContainer>::Container as BatchContainer>::ReadItem<'_>) -> bool {
other.eq(&<<<K as PreferredContainer>::Container as BatchContainer>::ReadItem<'_> as IntoOwned>::borrow_as(this))
<<K as PreferredContainer>::Container as BatchContainer>::reborrow(other).eq(&<<K as PreferredContainer>::Container as BatchContainer>::reborrow(<<<K as PreferredContainer>::Container as BatchContainer>::ReadItem<'_> as IntoOwned>::borrow_as(this)))
}

fn val_eq(this: &&V::Owned, other: <<V as PreferredContainer>::Container as BatchContainer>::ReadItem<'_>) -> bool {
other.eq(&<<<V as PreferredContainer>::Container as BatchContainer>::ReadItem<'_> as IntoOwned>::borrow_as(this))
<<V as PreferredContainer>::Container as BatchContainer>::reborrow(other).eq(&<<V as PreferredContainer>::Container as BatchContainer>::reborrow(<<<V as PreferredContainer>::Container as BatchContainer>::ReadItem<'_> as IntoOwned>::borrow_as(this)))
}
}

Expand All @@ -443,12 +445,10 @@ pub mod containers {
use timely::container::columnation::{Columnation, TimelyStack};
use timely::container::PushInto;

use std::borrow::ToOwned;

/// A general-purpose container resembling `Vec<T>`.
pub trait BatchContainer: 'static {
/// The type that can be read back out of the container.
type ReadItem<'a>: Copy + Ord + for<'b> PartialOrd<Self::ReadItem<'b>>;
type ReadItem<'a>: Copy + Ord;

/// Push an item into this container
fn push<D>(&mut self, item: D) where Self: PushInto<D> {
Expand All @@ -467,6 +467,9 @@ pub mod containers {
/// Creates a new container with sufficient capacity.
fn merge_capacity(cont1: &Self, cont2: &Self) -> Self;

/// Converts a read item into one with a narrower lifetime.
fn reborrow<'b, 'a: 'b>(item: Self::ReadItem<'a>) -> Self::ReadItem<'b>;

/// Reference to the element at this position.
fn index(&self, index: usize) -> Self::ReadItem<'_>;
/// Number of contained elements
Expand Down Expand Up @@ -533,6 +536,8 @@ pub mod containers {
impl<T: Ord + Clone + 'static> BatchContainer for Vec<T> {
type ReadItem<'a> = &'a T;

fn reborrow<'b, 'a: 'b>(item: Self::ReadItem<'a>) -> Self::ReadItem<'b> { item }

fn copy(&mut self, item: &T) {
self.push(item.clone());
}
Expand All @@ -555,9 +560,11 @@ pub mod containers {

// The `ToOwned` requirement exists to satisfy `self.reserve_items`, who must for now
// be presented with the actual contained type, rather than a type that borrows into it.
impl<T: Ord + Columnation + ToOwned<Owned = T> + 'static> BatchContainer for TimelyStack<T> {
impl<T: Ord + Columnation + 'static> BatchContainer for TimelyStack<T> {
type ReadItem<'a> = &'a T;

fn reborrow<'b, 'a: 'b>(item: Self::ReadItem<'a>) -> Self::ReadItem<'b> { item }

fn copy(&mut self, item: &T) {
self.copy(item);
}
Expand Down Expand Up @@ -622,6 +629,8 @@ pub mod containers {
{
type ReadItem<'a> = &'a [B];

fn reborrow<'b, 'a: 'b>(item: Self::ReadItem<'a>) -> Self::ReadItem<'b> { item }

fn copy(&mut self, item: Self::ReadItem<'_>) {
for x in item.iter() {
self.inner.copy(x);
Expand Down
6 changes: 3 additions & 3 deletions src/trace/implementations/ord_neu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ mod val_batch {
}
}
fn seek_key(&mut self, storage: &OrdValBatch<L>, key: Self::Key<'_>) {
self.key_cursor += storage.storage.keys.advance(self.key_cursor, storage.storage.keys.len(), |x| x.lt(&key));
self.key_cursor += storage.storage.keys.advance(self.key_cursor, storage.storage.keys.len(), |x| <L::KeyContainer as BatchContainer>::reborrow(x).lt(&<L::KeyContainer as BatchContainer>::reborrow(key)));
if self.key_valid(storage) {
self.rewind_vals(storage);
}
Expand All @@ -485,7 +485,7 @@ mod val_batch {
}
}
fn seek_val(&mut self, storage: &OrdValBatch<L>, val: Self::Val<'_>) {
self.val_cursor += storage.storage.vals.advance(self.val_cursor, storage.storage.values_for_key(self.key_cursor).1, |x| x.lt(&val));
self.val_cursor += storage.storage.vals.advance(self.val_cursor, storage.storage.values_for_key(self.key_cursor).1, |x| <L::ValContainer as BatchContainer>::reborrow(x).lt(&<L::ValContainer as BatchContainer>::reborrow(val)));
}
fn rewind_keys(&mut self, storage: &OrdValBatch<L>) {
self.key_cursor = 0;
Expand Down Expand Up @@ -921,7 +921,7 @@ mod key_batch {
}
}
fn seek_key(&mut self, storage: &Self::Storage, key: Self::Key<'_>) {
self.key_cursor += storage.storage.keys.advance(self.key_cursor, storage.storage.keys.len(), |x| x.lt(&key));
self.key_cursor += storage.storage.keys.advance(self.key_cursor, storage.storage.keys.len(), |x| <L::KeyContainer as BatchContainer>::reborrow(x).lt(&<L::KeyContainer as BatchContainer>::reborrow(key)));
if self.key_valid(storage) {
self.rewind_vals(storage);
}
Expand Down
4 changes: 2 additions & 2 deletions src/trace/implementations/rhh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ mod val_batch {
/// Returns true if one should advance one's index in the search for `key`.
fn advance_key(&self, index: usize, key: <L::KeyContainer as BatchContainer>::ReadItem<'_>) -> bool {
// Ideally this short-circuits, as `self.keys[index]` is bogus data.
!self.live_key(index) || self.keys.index(index).lt(&key)
!self.live_key(index) || self.keys.index(index).lt(&<L::KeyContainer as BatchContainer>::reborrow(key))
}

/// Indicates that a key is valid, rather than dead space, by looking for a valid offset range.
Expand Down Expand Up @@ -675,7 +675,7 @@ mod val_batch {
}
}
fn seek_val(&mut self, storage: &RhhValBatch<L>, val: Self::Val<'_>) {
self.val_cursor += storage.storage.vals.advance(self.val_cursor, storage.storage.values_for_key(self.key_cursor).1, |x| x.lt(&val));
self.val_cursor += storage.storage.vals.advance(self.val_cursor, storage.storage.values_for_key(self.key_cursor).1, |x| <L::ValContainer as BatchContainer>::reborrow(x).lt(&<L::ValContainer as BatchContainer>::reborrow(val)));
}
fn rewind_keys(&mut self, storage: &RhhValBatch<L>) {
self.key_cursor = 0;
Expand Down