From 2f53aeafda0a0d3670b980ff2ce5bcb991e5306d Mon Sep 17 00:00:00 2001 From: kinto-b Date: Wed, 24 Apr 2024 10:15:56 +1000 Subject: [PATCH] Simplify `OrderingOrBool::merge` Return `(Option>, MergeResult)` instead of `(Option, Option, MergeResult)` --- src/merge_join.rs | 66 ++++++++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 30 deletions(-) diff --git a/src/merge_join.rs b/src/merge_join.rs index 7ae21e3b..c0de35f9 100644 --- a/src/merge_join.rs +++ b/src/merge_join.rs @@ -115,7 +115,7 @@ pub trait OrderingOrBool { // "merge" never returns (Some(...), Some(...), ...) so Option> // 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(&mut self, left: L, right: R) -> (Option, Option, Self::MergeResult); + fn merge(&mut self, left: L, right: R) -> (Option>, Self::MergeResult); fn size_hint(left: SizeHint, right: SizeHint) -> SizeHint; } @@ -127,11 +127,11 @@ impl Ordering> OrderingOrBool for MergeFuncLR Self::MergeResult { EitherOrBoth::Right(right) } - fn merge(&mut self, left: L, right: R) -> (Option, Option, Self::MergeResult) { + fn merge(&mut self, left: L, right: R) -> (Option>, 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)), + Ordering::Equal => (None, EitherOrBoth::Both(left, right)), + Ordering::Less => (Some(Either::Right(right)), EitherOrBoth::Left(left)), + Ordering::Greater => (Some(Either::Left(left)), EitherOrBoth::Right(right)), } } fn size_hint(left: SizeHint, right: SizeHint) -> SizeHint { @@ -154,11 +154,11 @@ impl bool> OrderingOrBool for MergeFuncLR Self::MergeResult { Either::Right(right) } - fn merge(&mut self, left: L, right: R) -> (Option, Option, Self::MergeResult) { + fn merge(&mut self, left: L, right: R) -> (Option>, Self::MergeResult) { if self.0(&left, &right) { - (None, Some(right), Either::Left(left)) + (Some(Either::Right(right)), Either::Left(left)) } else { - (Some(left), None, Either::Right(right)) + (Some(Either::Left(left)), Either::Right(right)) } } fn size_hint(left: SizeHint, right: SizeHint) -> SizeHint { @@ -175,11 +175,11 @@ impl bool> OrderingOrBool for F { fn right(right: T) -> Self::MergeResult { right } - fn merge(&mut self, left: T, right: T) -> (Option, Option, Self::MergeResult) { + fn merge(&mut self, left: T, right: T) -> (Option>, Self::MergeResult) { if self(&left, &right) { - (None, Some(right), left) + (Some(Either::Right(right)), left) } else { - (Some(left), None, right) + (Some(Either::Left(left)), right) } } fn size_hint(left: SizeHint, right: SizeHint) -> SizeHint { @@ -196,11 +196,11 @@ impl OrderingOrBool for MergeLte { fn right(right: T) -> Self::MergeResult { right } - fn merge(&mut self, left: T, right: T) -> (Option, Option, Self::MergeResult) { + fn merge(&mut self, left: T, right: T) -> (Option>, Self::MergeResult) { if left <= right { - (None, Some(right), left) + (Some(Either::Right(right)), left) } else { - (Some(left), None, right) + (Some(Either::Left(left)), right) } } fn size_hint(left: SizeHint, right: SizeHint) -> SizeHint { @@ -244,13 +244,17 @@ where (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.merge(left, right); - if let Some(left) = left { - self.left.put_back(left); - } - if let Some(right) = right { - self.right.put_back(right); + let (not_next, next) = self.cmp_fn.merge(left, right); + match not_next { + Some(Either::Left(l)) => { + self.left.put_back(l); + } + Some(Either::Right(r)) => { + self.right.put_back(r); + } + None => (), } + Some(next) } } @@ -268,22 +272,21 @@ where loop { match (left, right) { (Some(l), Some(r)) => match self.cmp_fn.merge(l, r) { - (None, Some(r), x) => { + (Some(Either::Right(r)), x) => { acc = f(acc, x); left = self.left.next(); right = Some(r); } - (Some(l), None, x) => { + (Some(Either::Left(l)), x) => { acc = f(acc, x); left = Some(l); right = self.right.next(); } - (None, None, x) => { + (None, x) => { acc = f(acc, x); left = self.left.next(); right = self.right.next(); } - (Some(_), Some(_), _) => unreachable!(), }, (Some(l), None) => { self.left.put_back(l); @@ -319,12 +322,15 @@ where (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.merge(left, right); - if let Some(left) = left { - self.left.put_back(left); - } - if let Some(right) = right { - self.right.put_back(right); + let (not_next, _) = self.cmp_fn.merge(left, right); + match not_next { + Some(Either::Left(l)) => { + self.left.put_back(l); + } + Some(Either::Right(r)) => { + self.right.put_back(r); + } + None => (), } } }