Skip to content

Commit

Permalink
MergeJoinBy alias of new InternalMergeJoinBy
Browse files Browse the repository at this point in the history
And the trait `OrderingOrBool<L, R>` is now for wrapped functions rather than for `Ordering` and `bool`.

NOTES:
- The new struct `MergeFuncLR<F, T>` is useful to avoid conflicting implementations of `OrderingOrBool<L, R> for F` because the compiler thinks that `F` could implement both `FnMut(&L, &R) -> Ordering` and `FnMut(&L, &R) -> bool`.
- The new trait `FuncLR<L, R>` is useful to be able to infer the Output type `T` without having to add a fourth parameter to `MergeJoinBy`. From what I understand, this is needed because the `FnMut` trait is not fully stabilized yet.
- `OrderingOrBool<L, R>` has a new associated type `Out`, which is useful in `impl Iterator` because otherwise the compiler would complain about `T` be unconstrained.

This successfully pass the tests.
  • Loading branch information
Philippe-Cholet committed Aug 24, 2023
1 parent f920a9c commit 4f360bd
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 30 deletions.
1 change: 0 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1070,7 +1070,6 @@ pub trait Itertools : Iterator {
fn merge_join_by<J, F, T>(self, other: J, cmp_fn: F) -> MergeJoinBy<Self, J::IntoIter, F>
where J: IntoIterator,
F: FnMut(&Self::Item, &J::Item) -> T,
T: merge_join::OrderingOrBool<Self::Item, J::Item>,
Self: Sized
{
merge_join_by(self, other, cmp_fn)
Expand Down
73 changes: 44 additions & 29 deletions src/merge_join.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::cmp::Ordering;
use std::iter::Fuse;
use std::iter::{Peekable, FusedIterator};
use std::fmt;
use std::marker::PhantomData;

use either::Either;

Expand Down Expand Up @@ -156,46 +157,60 @@ pub fn merge_join_by<I, J, F, T>(left: I, right: J, cmp_fn: F)
where I: IntoIterator,
J: IntoIterator,
F: FnMut(&I::Item, &J::Item) -> T,
T: OrderingOrBool<I::Item, J::Item>,
{
MergeJoinBy {
InternalMergeJoinBy {
left: put_back(left.into_iter().fuse()),
right: put_back(right.into_iter().fuse()),
cmp_fn,
cmp_fn: MergeFuncLR(cmp_fn, PhantomData),
}
}

/// An iterator adaptor that merge-joins items from the two base iterators in ascending order.
///
/// See [`.merge_join_by()`](crate::Itertools::merge_join_by) for more information.
pub type MergeJoinBy<I, J, F> = InternalMergeJoinBy<I, J, MergeFuncLR<F, <F as FuncLR<<I as Iterator>::Item, <J as Iterator>::Item>>::T>>;

#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
pub struct MergeJoinBy<I: Iterator, J: Iterator, F> {
pub struct InternalMergeJoinBy<I: Iterator, J: Iterator, F> {
left: PutBack<Fuse<I>>,
right: PutBack<Fuse<J>>,
cmp_fn: F,
}

#[derive(Clone, Debug)]
pub struct MergeFuncLR<F, T>(F, PhantomData<T>);

pub trait FuncLR<L, R> {
type T;
}

impl<L, R, T, F: FnMut(&L, &R) -> T> FuncLR<L, R> for F {
type T = T;
}

pub trait OrderingOrBool<L, R> {
type Out;
type MergeResult;
fn left(left: L) -> Self::MergeResult;
fn right(right: R) -> Self::MergeResult;
// "merge" never returns (Some(...), Some(...), ...) so Option<Either<I::Item, J::Item>>
// is appealing but it is always followed by two put_backs, so we think the compiler is
// smart enough to optimize it. Or we could move put_backs into "merge".
fn merge(self, left: L, right: R) -> (Option<L>, Option<R>, Self::MergeResult);
fn merge(&mut self, left: L, right: R) -> (Option<L>, Option<R>, Self::MergeResult);
fn size_hint(left: SizeHint, right: SizeHint) -> SizeHint;
}

impl<L, R> OrderingOrBool<L, R> for Ordering {
impl<L, R, F: FnMut(&L, &R) -> Ordering> OrderingOrBool<L, R> for MergeFuncLR<F, Ordering> {
type Out = Ordering;
type MergeResult = EitherOrBoth<L, R>;
fn left(left: L) -> Self::MergeResult {
EitherOrBoth::Left(left)
}
fn right(right: R) -> Self::MergeResult {
EitherOrBoth::Right(right)
}
fn merge(self, left: L, right: R) -> (Option<L>, Option<R>, Self::MergeResult) {
match self {
fn merge(&mut self, left: L, right: R) -> (Option<L>, Option<R>, Self::MergeResult) {
match self.0(&left, &right) {
Ordering::Equal => (None, None, EitherOrBoth::Both(left, right)),
Ordering::Less => (None, Some(right), EitherOrBoth::Left(left)),
Ordering::Greater => (Some(left), None, EitherOrBoth::Right(right)),
Expand All @@ -213,16 +228,17 @@ impl<L, R> OrderingOrBool<L, R> for Ordering {
}
}

impl<L, R> OrderingOrBool<L, R> for bool {
impl<L, R, F: FnMut(&L, &R) -> bool> OrderingOrBool<L, R> for MergeFuncLR<F, bool> {
type Out = bool;
type MergeResult = Either<L, R>;
fn left(left: L) -> Self::MergeResult {
Either::Left(left)
}
fn right(right: R) -> Self::MergeResult {
Either::Right(right)
}
fn merge(self, left: L, right: R) -> (Option<L>, Option<R>, Self::MergeResult) {
if self {
fn merge(&mut self, left: L, right: R) -> (Option<L>, Option<R>, Self::MergeResult) {
if self.0(&left, &right) {
(None, Some(right), Either::Left(left))
} else {
(Some(left), None, Either::Right(right))
Expand All @@ -234,7 +250,7 @@ impl<L, R> OrderingOrBool<L, R> for bool {
}
}

impl<I, J, F> Clone for MergeJoinBy<I, J, F>
impl<I, J, F> Clone for InternalMergeJoinBy<I, J, F>
where I: Iterator,
J: Iterator,
PutBack<Fuse<I>>: Clone,
Expand All @@ -244,30 +260,29 @@ impl<I, J, F> Clone for MergeJoinBy<I, J, F>
clone_fields!(left, right, cmp_fn);
}

impl<I, J, F> fmt::Debug for MergeJoinBy<I, J, F>
impl<I, J, F> fmt::Debug for InternalMergeJoinBy<I, J, F>
where I: Iterator + fmt::Debug,
I::Item: fmt::Debug,
J: Iterator + fmt::Debug,
J::Item: fmt::Debug,
{
debug_fmt_fields!(MergeJoinBy, left, right);
debug_fmt_fields!(InternalMergeJoinBy, left, right);
}

impl<I, J, F, T> Iterator for MergeJoinBy<I, J, F>
impl<I, J, F, T> Iterator for InternalMergeJoinBy<I, J, F>
where I: Iterator,
J: Iterator,
F: FnMut(&I::Item, &J::Item) -> T,
T: OrderingOrBool<I::Item, J::Item>,
F: OrderingOrBool<I::Item, J::Item, Out = T>,
{
type Item = T::MergeResult;
type Item = F::MergeResult;

fn next(&mut self) -> Option<Self::Item> {
match (self.left.next(), self.right.next()) {
(None, None) => None,
(Some(left), None) => Some(T::left(left)),
(None, Some(right)) => Some(T::right(right)),
(Some(left), None) => Some(F::left(left)),
(None, Some(right)) => Some(F::right(right)),
(Some(left), Some(right)) => {
let (left, right, next) = (self.cmp_fn)(&left, &right).merge(left, right);
let (left, right, next) = self.cmp_fn.merge(left, right);
if let Some(left) = left {
self.left.put_back(left);
}
Expand All @@ -280,7 +295,7 @@ impl<I, J, F, T> Iterator for MergeJoinBy<I, J, F>
}

fn size_hint(&self) -> SizeHint {
T::size_hint(self.left.size_hint(), self.right.size_hint())
F::size_hint(self.left.size_hint(), self.right.size_hint())
}

fn count(mut self) -> usize {
Expand All @@ -292,7 +307,7 @@ impl<I, J, F, T> Iterator for MergeJoinBy<I, J, F>
(None, Some(_right)) => break count + 1 + self.right.into_parts().1.count(),
(Some(left), Some(right)) => {
count += 1;
let (left, right, _) = (self.cmp_fn)(&left, &right).merge(left, right);
let (left, right, _) = self.cmp_fn.merge(left, right);
if let Some(left) = left {
self.left.put_back(left);
}
Expand All @@ -310,17 +325,17 @@ impl<I, J, F, T> Iterator for MergeJoinBy<I, J, F>
match (self.left.next(), self.right.next()) {
(None, None) => break previous_element,
(Some(left), None) => {
break Some(T::left(
break Some(F::left(
self.left.into_parts().1.last().unwrap_or(left),
))
}
(None, Some(right)) => {
break Some(T::right(
break Some(F::right(
self.right.into_parts().1.last().unwrap_or(right),
))
}
(Some(left), Some(right)) => {
let (left, right, elem) = (self.cmp_fn)(&left, &right).merge(left, right);
let (left, right, elem) = self.cmp_fn.merge(left, right);
if let Some(left) = left {
self.left.put_back(left);
}
Expand All @@ -341,10 +356,10 @@ impl<I, J, F, T> Iterator for MergeJoinBy<I, J, F>
n -= 1;
match (self.left.next(), self.right.next()) {
(None, None) => break None,
(Some(_left), None) => break self.left.nth(n).map(T::left),
(None, Some(_right)) => break self.right.nth(n).map(T::right),
(Some(_left), None) => break self.left.nth(n).map(F::left),
(None, Some(_right)) => break self.right.nth(n).map(F::right),
(Some(left), Some(right)) => {
let (left, right, _) = (self.cmp_fn)(&left, &right).merge(left, right);
let (left, right, _) = self.cmp_fn.merge(left, right);
if let Some(left) = left {
self.left.put_back(left);
}
Expand Down

0 comments on commit 4f360bd

Please sign in to comment.