Skip to content

Commit 40db258

Browse files
committed
Auto merge of #87974 - steffahn:slice_split_size_hints, r=dtolnay
Test and fix `size_hint` for slice’s [r]split* iterators Adds extensive test (of `size_hint`) for all the _[r]split*_ iterators. Fixes `size_hint` upper bound for _split_inclusive*_ iterators which was one higher than necessary for non-empty slices. Fixes `size_hint` lower bound for _[r]splitn*_ iterators when _n == 0_, which was one too high. **Lower bound being one too high was a logic error, violating the correctness condition of `size_hint`.** _Edit:_ I’ve opened an issue for that bug, so this PR fixes #87978
2 parents 85109e2 + 3f0d04e commit 40db258

File tree

2 files changed

+86
-8
lines changed

2 files changed

+86
-8
lines changed

library/alloc/tests/slice.rs

+61
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use std::cell::Cell;
22
use std::cmp::Ordering::{self, Equal, Greater, Less};
33
use std::convert::identity;
4+
use std::fmt;
45
use std::mem;
56
use std::panic;
67
use std::rc::Rc;
@@ -993,6 +994,66 @@ fn test_rsplitnator() {
993994
assert!(xs.rsplitn(0, |x| *x % 2 == 0).next().is_none());
994995
}
995996

997+
#[test]
998+
fn test_split_iterators_size_hint() {
999+
#[derive(Copy, Clone)]
1000+
enum Bounds {
1001+
Lower,
1002+
Upper,
1003+
}
1004+
fn assert_tight_size_hints(mut it: impl Iterator, which: Bounds, ctx: impl fmt::Display) {
1005+
match which {
1006+
Bounds::Lower => {
1007+
let mut lower_bounds = vec![it.size_hint().0];
1008+
while let Some(_) = it.next() {
1009+
lower_bounds.push(it.size_hint().0);
1010+
}
1011+
let target: Vec<_> = (0..lower_bounds.len()).rev().collect();
1012+
assert_eq!(lower_bounds, target, "lower bounds incorrect or not tight: {}", ctx);
1013+
}
1014+
Bounds::Upper => {
1015+
let mut upper_bounds = vec![it.size_hint().1];
1016+
while let Some(_) = it.next() {
1017+
upper_bounds.push(it.size_hint().1);
1018+
}
1019+
let target: Vec<_> = (0..upper_bounds.len()).map(Some).rev().collect();
1020+
assert_eq!(upper_bounds, target, "upper bounds incorrect or not tight: {}", ctx);
1021+
}
1022+
}
1023+
}
1024+
1025+
for len in 0..=2 {
1026+
let mut v: Vec<u8> = (0..len).collect();
1027+
1028+
// p: predicate, b: bound selection
1029+
for (p, b) in [
1030+
// with a predicate always returning false, the split*-iterators
1031+
// become maximally short, so the size_hint lower bounds are tight
1032+
((|_| false) as fn(&_) -> _, Bounds::Lower),
1033+
// with a predicate always returning true, the split*-iterators
1034+
// become maximally long, so the size_hint upper bounds are tight
1035+
((|_| true) as fn(&_) -> _, Bounds::Upper),
1036+
] {
1037+
use assert_tight_size_hints as a;
1038+
use format_args as f;
1039+
1040+
a(v.split(p), b, "split");
1041+
a(v.split_mut(p), b, "split_mut");
1042+
a(v.split_inclusive(p), b, "split_inclusive");
1043+
a(v.split_inclusive_mut(p), b, "split_inclusive_mut");
1044+
a(v.rsplit(p), b, "rsplit");
1045+
a(v.rsplit_mut(p), b, "rsplit_mut");
1046+
1047+
for n in 0..=3 {
1048+
a(v.splitn(n, p), b, f!("splitn, n = {}", n));
1049+
a(v.splitn_mut(n, p), b, f!("splitn_mut, n = {}", n));
1050+
a(v.rsplitn(n, p), b, f!("rsplitn, n = {}", n));
1051+
a(v.rsplitn_mut(n, p), b, f!("rsplitn_mut, n = {}", n));
1052+
}
1053+
}
1054+
}
1055+
}
1056+
9961057
#[test]
9971058
fn test_windowsator() {
9981059
let v = &[1, 2, 3, 4];

library/core/src/slice/iter.rs

+25-8
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,13 @@ where
400400

401401
#[inline]
402402
fn size_hint(&self) -> (usize, Option<usize>) {
403-
if self.finished { (0, Some(0)) } else { (1, Some(self.v.len() + 1)) }
403+
if self.finished {
404+
(0, Some(0))
405+
} else {
406+
// If the predicate doesn't match anything, we yield one slice.
407+
// If it matches every element, we yield `len() + 1` empty slices.
408+
(1, Some(self.v.len() + 1))
409+
}
404410
}
405411
}
406412

@@ -525,7 +531,14 @@ where
525531

526532
#[inline]
527533
fn size_hint(&self) -> (usize, Option<usize>) {
528-
if self.finished { (0, Some(0)) } else { (1, Some(self.v.len() + 1)) }
534+
if self.finished {
535+
(0, Some(0))
536+
} else {
537+
// If the predicate doesn't match anything, we yield one slice.
538+
// If it matches every element, we yield `len()` one-element slices,
539+
// or a single empty slice.
540+
(1, Some(cmp::max(1, self.v.len())))
541+
}
529542
}
530543
}
531544

@@ -647,8 +660,8 @@ where
647660
if self.finished {
648661
(0, Some(0))
649662
} else {
650-
// if the predicate doesn't match anything, we yield one slice
651-
// if it matches every element, we yield len+1 empty slices.
663+
// If the predicate doesn't match anything, we yield one slice.
664+
// If it matches every element, we yield `len() + 1` empty slices.
652665
(1, Some(self.v.len() + 1))
653666
}
654667
}
@@ -763,9 +776,10 @@ where
763776
if self.finished {
764777
(0, Some(0))
765778
} else {
766-
// if the predicate doesn't match anything, we yield one slice
767-
// if it matches every element, we yield len+1 empty slices.
768-
(1, Some(self.v.len() + 1))
779+
// If the predicate doesn't match anything, we yield one slice.
780+
// If it matches every element, we yield `len()` one-element slices,
781+
// or a single empty slice.
782+
(1, Some(cmp::max(1, self.v.len())))
769783
}
770784
}
771785
}
@@ -1008,7 +1022,10 @@ impl<T, I: SplitIter<Item = T>> Iterator for GenericSplitN<I> {
10081022
#[inline]
10091023
fn size_hint(&self) -> (usize, Option<usize>) {
10101024
let (lower, upper_opt) = self.iter.size_hint();
1011-
(lower, upper_opt.map(|upper| cmp::min(self.count, upper)))
1025+
(
1026+
cmp::min(self.count, lower),
1027+
Some(upper_opt.map_or(self.count, |upper| cmp::min(self.count, upper))),
1028+
)
10121029
}
10131030
}
10141031

0 commit comments

Comments
 (0)