1
Fork 0

Auto merge of #64383 - pcpthm:btreeset-size-hint, r=dtolnay

Improve BTreeSet::Intersection::size_hint

A comment on `IntersectionInner` mentions `small_iter` should be smaller than `other_iter` but this condition is broken while iterating because those two iterators can be consumed at a different rate. I added a test to demonstrate this situation.
<del>I made `small_iter.len() < other_iter.len()` always true by swapping two iterators when that condition became false. This change affects the return value of `size_hint`. The previous result was also correct but this new version always returns smaller upper bound than the previous version.</del>
I changed `size_hint` to taking minimum of both lengths of iterators and renamed fields to `a` and `b` to match `Union` iterator.
This commit is contained in:
bors 2019-09-16 05:16:19 +00:00
commit b6269f27d9
2 changed files with 33 additions and 22 deletions

View file

@ -3,7 +3,7 @@
use core::borrow::Borrow; use core::borrow::Borrow;
use core::cmp::Ordering::{self, Less, Greater, Equal}; use core::cmp::Ordering::{self, Less, Greater, Equal};
use core::cmp::max; use core::cmp::{max, min};
use core::fmt::{self, Debug}; use core::fmt::{self, Debug};
use core::iter::{Peekable, FromIterator, FusedIterator}; use core::iter::{Peekable, FromIterator, FusedIterator};
use core::ops::{BitOr, BitAnd, BitXor, Sub, RangeBounds}; use core::ops::{BitOr, BitAnd, BitXor, Sub, RangeBounds};
@ -187,8 +187,8 @@ pub struct Intersection<'a, T: 'a> {
} }
enum IntersectionInner<'a, T: 'a> { enum IntersectionInner<'a, T: 'a> {
Stitch { Stitch {
small_iter: Iter<'a, T>, // for size_hint, should be the smaller of the sets a: Iter<'a, T>,
other_iter: Iter<'a, T>, b: Iter<'a, T>,
}, },
Search { Search {
small_iter: Iter<'a, T>, small_iter: Iter<'a, T>,
@ -201,12 +201,12 @@ impl<T: fmt::Debug> fmt::Debug for Intersection<'_, T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match &self.inner { match &self.inner {
IntersectionInner::Stitch { IntersectionInner::Stitch {
small_iter, a,
other_iter, b,
} => f } => f
.debug_tuple("Intersection") .debug_tuple("Intersection")
.field(&small_iter) .field(&a)
.field(&other_iter) .field(&b)
.finish(), .finish(),
IntersectionInner::Search { IntersectionInner::Search {
small_iter, small_iter,
@ -397,8 +397,8 @@ impl<T: Ord> BTreeSet<T> {
// Iterate both sets jointly, spotting matches along the way. // Iterate both sets jointly, spotting matches along the way.
Intersection { Intersection {
inner: IntersectionInner::Stitch { inner: IntersectionInner::Stitch {
small_iter: small.iter(), a: small.iter(),
other_iter: other.iter(), b: other.iter(),
}, },
} }
} else { } else {
@ -1221,11 +1221,11 @@ impl<T> Clone for Intersection<'_, T> {
Intersection { Intersection {
inner: match &self.inner { inner: match &self.inner {
IntersectionInner::Stitch { IntersectionInner::Stitch {
small_iter, a,
other_iter, b,
} => IntersectionInner::Stitch { } => IntersectionInner::Stitch {
small_iter: small_iter.clone(), a: a.clone(),
other_iter: other_iter.clone(), b: b.clone(),
}, },
IntersectionInner::Search { IntersectionInner::Search {
small_iter, small_iter,
@ -1245,16 +1245,16 @@ impl<'a, T: Ord> Iterator for Intersection<'a, T> {
fn next(&mut self) -> Option<&'a T> { fn next(&mut self) -> Option<&'a T> {
match &mut self.inner { match &mut self.inner {
IntersectionInner::Stitch { IntersectionInner::Stitch {
small_iter, a,
other_iter, b,
} => { } => {
let mut small_next = small_iter.next()?; let mut a_next = a.next()?;
let mut other_next = other_iter.next()?; let mut b_next = b.next()?;
loop { loop {
match Ord::cmp(small_next, other_next) { match Ord::cmp(a_next, b_next) {
Less => small_next = small_iter.next()?, Less => a_next = a.next()?,
Greater => other_next = other_iter.next()?, Greater => b_next = b.next()?,
Equal => return Some(small_next), Equal => return Some(a_next),
} }
} }
} }
@ -1272,7 +1272,7 @@ impl<'a, T: Ord> Iterator for Intersection<'a, T> {
fn size_hint(&self) -> (usize, Option<usize>) { fn size_hint(&self) -> (usize, Option<usize>) {
let min_len = match &self.inner { let min_len = match &self.inner {
IntersectionInner::Stitch { small_iter, .. } => small_iter.len(), IntersectionInner::Stitch { a, b } => min(a.len(), b.len()),
IntersectionInner::Search { small_iter, .. } => small_iter.len(), IntersectionInner::Search { small_iter, .. } => small_iter.len(),
}; };
(0, Some(min_len)) (0, Some(min_len))

View file

@ -90,6 +90,17 @@ fn test_intersection() {
&[1, 3, 11, 77, 103]); &[1, 3, 11, 77, 103]);
} }
#[test]
fn test_intersection_size_hint() {
let x: BTreeSet<i32> = [3, 4].iter().copied().collect();
let y: BTreeSet<i32> = [1, 2, 3].iter().copied().collect();
let mut iter = x.intersection(&y);
assert_eq!(iter.size_hint(), (0, Some(2)));
assert_eq!(iter.next(), Some(&3));
assert_eq!(iter.size_hint(), (0, Some(0)));
assert_eq!(iter.next(), None);
}
#[test] #[test]
fn test_difference() { fn test_difference() {
fn check_difference(a: &[i32], b: &[i32], expected: &[i32]) { fn check_difference(a: &[i32], b: &[i32], expected: &[i32]) {