From 31e49f02723fe4631740fce4ef387aad31563bef Mon Sep 17 00:00:00 2001 From: Frank Steffahn Date: Thu, 12 Aug 2021 17:09:17 +0200 Subject: [PATCH 1/4] Test and fix size_hint for slice's [r]split* iterators Adds extensive test 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. --- library/alloc/tests/slice.rs | 74 ++++++++++++++++++++++++++++++++++ library/core/src/slice/iter.rs | 33 +++++++++++---- 2 files changed, 99 insertions(+), 8 deletions(-) diff --git a/library/alloc/tests/slice.rs b/library/alloc/tests/slice.rs index 1fb4a51acfd0a..fe3d7183ee2e8 100644 --- a/library/alloc/tests/slice.rs +++ b/library/alloc/tests/slice.rs @@ -993,6 +993,80 @@ fn test_rsplitnator() { assert!(xs.rsplitn(0, |x| *x % 2 == 0).next().is_none()); } +#[test] +fn test_split_iterators_size_hint() { + for len in 0..=2 { + let mut v: Vec = (0..len).collect(); + fn verify_descending(sequence: &[usize], context: &str) { + let len = sequence.len(); + let target: Vec = (0..len).rev().collect(); + assert_eq!(sequence, target, "while testing: {}", context); + } + + macro_rules! test_size_hint { + ($create_iterator:expr) => {{ + // with a predicate always returning false, the split*-iterators + // become maximally short, so the size_hint lower bounds are correct + + macro_rules! p { + () => { + |_| false + }; + } + let mut short_iterator = $create_iterator; + let mut lower_bounds = vec![short_iterator.size_hint().0]; + while let Some(_) = short_iterator.next() { + lower_bounds.push(short_iterator.size_hint().0); + } + verify_descending(&lower_bounds, stringify!($create_iterator)); + } + { + // with a predicate always returning true, the split*-iterators + // become maximally long, so the size_hint upper bounds are correct + + macro_rules! p { + () => { + |_| true + }; + } + let mut long_iterator = $create_iterator; + let mut upper_bounds = vec![ + long_iterator.size_hint().1.expect("split*-methods have known upper bound"), + ]; + while let Some(_) = long_iterator.next() { + upper_bounds.push( + long_iterator.size_hint().1.expect("split*-methods have known upper bound"), + ); + } + verify_descending(&upper_bounds, stringify!($create_iterator)); + }}; + } + + test_size_hint!(v.split(p!())); + test_size_hint!(v.split_mut(p!())); + test_size_hint!(v.splitn(0, p!())); + test_size_hint!(v.splitn(1, p!())); + test_size_hint!(v.splitn(2, p!())); + test_size_hint!(v.splitn(3, p!())); + test_size_hint!(v.splitn_mut(0, p!())); + test_size_hint!(v.splitn_mut(1, p!())); + test_size_hint!(v.splitn_mut(2, p!())); + test_size_hint!(v.splitn_mut(3, p!())); + test_size_hint!(v.split_inclusive(p!())); + test_size_hint!(v.split_inclusive_mut(p!())); + test_size_hint!(v.rsplit(p!())); + test_size_hint!(v.rsplit_mut(p!())); + test_size_hint!(v.rsplitn(0, p!())); + test_size_hint!(v.rsplitn(1, p!())); + test_size_hint!(v.rsplitn(2, p!())); + test_size_hint!(v.rsplitn(3, p!())); + test_size_hint!(v.rsplitn_mut(0, p!())); + test_size_hint!(v.rsplitn_mut(1, p!())); + test_size_hint!(v.rsplitn_mut(2, p!())); + test_size_hint!(v.rsplitn_mut(3, p!())); + } +} + #[test] fn test_windowsator() { let v = &[1, 2, 3, 4]; diff --git a/library/core/src/slice/iter.rs b/library/core/src/slice/iter.rs index d67af9cf6680c..c0dfba490eca7 100644 --- a/library/core/src/slice/iter.rs +++ b/library/core/src/slice/iter.rs @@ -400,7 +400,13 @@ where #[inline] fn size_hint(&self) -> (usize, Option) { - if self.finished { (0, Some(0)) } else { (1, Some(self.v.len() + 1)) } + if self.finished { + (0, Some(0)) + } else { + // If the predicate doesn't match anything, we yield one slice. + // If it matches every element, we yield `len() + 1` empty slices. + (1, Some(self.v.len() + 1)) + } } } @@ -525,7 +531,14 @@ where #[inline] fn size_hint(&self) -> (usize, Option) { - if self.finished { (0, Some(0)) } else { (1, Some(self.v.len() + 1)) } + if self.finished { + (0, Some(0)) + } else { + // If the predicate doesn't match anything, we yield one slice. + // If it matches every element, we yield `len()` one-element slices, + // or a single empty slice. + (1, Some(cmp::max(1, self.v.len()))) + } } } @@ -647,8 +660,8 @@ where if self.finished { (0, Some(0)) } else { - // if the predicate doesn't match anything, we yield one slice - // if it matches every element, we yield len+1 empty slices. + // If the predicate doesn't match anything, we yield one slice. + // If it matches every element, we yield `len() + 1` empty slices. (1, Some(self.v.len() + 1)) } } @@ -763,9 +776,10 @@ where if self.finished { (0, Some(0)) } else { - // if the predicate doesn't match anything, we yield one slice - // if it matches every element, we yield len+1 empty slices. - (1, Some(self.v.len() + 1)) + // If the predicate doesn't match anything, we yield one slice. + // If it matches every element, we yield `len()` one-element slices, + // or a single empty slice. + (1, Some(cmp::max(1, self.v.len()))) } } } @@ -1008,7 +1022,10 @@ impl> Iterator for GenericSplitN { #[inline] fn size_hint(&self) -> (usize, Option) { let (lower, upper_opt) = self.iter.size_hint(); - (lower, upper_opt.map(|upper| cmp::min(self.count, upper))) + ( + cmp::min(self.count, lower), + Some(upper_opt.map_or(self.count, |upper| cmp::min(self.count, upper))), + ) } } From 0bb11f43f65e971ee06e649fa496f762d3ad2b27 Mon Sep 17 00:00:00 2001 From: Frank Steffahn Date: Thu, 12 Aug 2021 22:44:40 +0200 Subject: [PATCH 2/4] Rewrite test from previous commit but without using macros. --- library/alloc/tests/slice.rs | 119 ++++++++++++++++------------------- 1 file changed, 55 insertions(+), 64 deletions(-) diff --git a/library/alloc/tests/slice.rs b/library/alloc/tests/slice.rs index fe3d7183ee2e8..4211cd42b02e2 100644 --- a/library/alloc/tests/slice.rs +++ b/library/alloc/tests/slice.rs @@ -1,6 +1,7 @@ use std::cell::Cell; use std::cmp::Ordering::{self, Equal, Greater, Less}; use std::convert::identity; +use std::fmt; use std::mem; use std::panic; use std::rc::Rc; @@ -995,75 +996,65 @@ fn test_rsplitnator() { #[test] fn test_split_iterators_size_hint() { - for len in 0..=2 { - let mut v: Vec = (0..len).collect(); - fn verify_descending(sequence: &[usize], context: &str) { - let len = sequence.len(); - let target: Vec = (0..len).rev().collect(); - assert_eq!(sequence, target, "while testing: {}", context); - } - - macro_rules! test_size_hint { - ($create_iterator:expr) => {{ - // with a predicate always returning false, the split*-iterators - // become maximally short, so the size_hint lower bounds are correct - - macro_rules! p { - () => { - |_| false - }; - } - let mut short_iterator = $create_iterator; - let mut lower_bounds = vec![short_iterator.size_hint().0]; - while let Some(_) = short_iterator.next() { - lower_bounds.push(short_iterator.size_hint().0); + #[derive(Copy, Clone)] + enum Bounds { + Lower, + Upper, + } + fn assert_precise_size_hints( + mut it: I, + which: Bounds, + context: impl fmt::Display, + ) { + match which { + Bounds::Lower => { + let mut lower_bounds = vec![it.size_hint().0]; + while let Some(_) = it.next() { + lower_bounds.push(it.size_hint().0); } - verify_descending(&lower_bounds, stringify!($create_iterator)); + let target: Vec<_> = (0..lower_bounds.len()).rev().collect(); + assert_eq!(lower_bounds, target, "incorrect lower bounds: {}", context); } - { - // with a predicate always returning true, the split*-iterators - // become maximally long, so the size_hint upper bounds are correct - - macro_rules! p { - () => { - |_| true - }; - } - let mut long_iterator = $create_iterator; - let mut upper_bounds = vec![ - long_iterator.size_hint().1.expect("split*-methods have known upper bound"), - ]; - while let Some(_) = long_iterator.next() { - upper_bounds.push( - long_iterator.size_hint().1.expect("split*-methods have known upper bound"), - ); + Bounds::Upper => { + let mut upper_bounds = vec![it.size_hint().1]; + while let Some(_) = it.next() { + upper_bounds.push(it.size_hint().1); } - verify_descending(&upper_bounds, stringify!($create_iterator)); - }}; + let target: Vec<_> = (0..upper_bounds.len()).map(Some).rev().collect(); + assert_eq!(upper_bounds, target, "incorrect upper bounds: {}", context); + } } + } - test_size_hint!(v.split(p!())); - test_size_hint!(v.split_mut(p!())); - test_size_hint!(v.splitn(0, p!())); - test_size_hint!(v.splitn(1, p!())); - test_size_hint!(v.splitn(2, p!())); - test_size_hint!(v.splitn(3, p!())); - test_size_hint!(v.splitn_mut(0, p!())); - test_size_hint!(v.splitn_mut(1, p!())); - test_size_hint!(v.splitn_mut(2, p!())); - test_size_hint!(v.splitn_mut(3, p!())); - test_size_hint!(v.split_inclusive(p!())); - test_size_hint!(v.split_inclusive_mut(p!())); - test_size_hint!(v.rsplit(p!())); - test_size_hint!(v.rsplit_mut(p!())); - test_size_hint!(v.rsplitn(0, p!())); - test_size_hint!(v.rsplitn(1, p!())); - test_size_hint!(v.rsplitn(2, p!())); - test_size_hint!(v.rsplitn(3, p!())); - test_size_hint!(v.rsplitn_mut(0, p!())); - test_size_hint!(v.rsplitn_mut(1, p!())); - test_size_hint!(v.rsplitn_mut(2, p!())); - test_size_hint!(v.rsplitn_mut(3, p!())); + for len in 0..=2 { + let mut v: Vec = (0..len).collect(); + + // p: predicate, b: bound selection + for (p, b) in [ + // with a predicate always returning false, the split*-iterators + // become maximally short, so the size_hint lower bounds are correct + ((|_| false) as fn(&_) -> _, Bounds::Lower), + // with a predicate always returning true, the split*-iterators + // become maximally long, so the size_hint upper bounds are correct + ((|_| true) as fn(&_) -> _, Bounds::Upper), + ] { + use assert_precise_size_hints as a; + use format_args as f; + + a(v.split(p), b, "split"); + a(v.split_mut(p), b, "split_mut"); + a(v.split_inclusive(p), b, "split_inclusive"); + a(v.split_inclusive_mut(p), b, "split_inclusive_mut"); + a(v.rsplit(p), b, "rsplit"); + a(v.rsplit_mut(p), b, "rsplit_mut"); + + for n in 0..=3 { + a(v.splitn(n, p), b, f!("splitn, n = {}", n)); + a(v.splitn_mut(n, p), b, f!("splitn_mut, n = {}", n)); + a(v.rsplitn(n, p), b, f!("rsplitn, n = {}", n)); + a(v.rsplitn_mut(n, p), b, f!("rsplitn_mut, n = {}", n)); + } + } } } From 43046860496df09d9e59a679ac3a20e1820619ee Mon Sep 17 00:00:00 2001 From: Frank Steffahn Date: Fri, 13 Aug 2021 12:02:03 +0200 Subject: [PATCH 3/4] Consistent use of `impl Trait` arguments in the test's helper function. --- library/alloc/tests/slice.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/library/alloc/tests/slice.rs b/library/alloc/tests/slice.rs index 4211cd42b02e2..198f467eea352 100644 --- a/library/alloc/tests/slice.rs +++ b/library/alloc/tests/slice.rs @@ -1001,11 +1001,7 @@ fn test_split_iterators_size_hint() { Lower, Upper, } - fn assert_precise_size_hints( - mut it: I, - which: Bounds, - context: impl fmt::Display, - ) { + fn assert_precise_size_hints(mut it: impl Iterator, which: Bounds, context: impl fmt::Display) { match which { Bounds::Lower => { let mut lower_bounds = vec![it.size_hint().0]; From 3f0d04e97b6e595fdff4895dfc4a35a2bd39f739 Mon Sep 17 00:00:00 2001 From: Frank Steffahn Date: Fri, 13 Aug 2021 14:41:50 +0200 Subject: [PATCH 4/4] Improve wording, correct -> tight. --- library/alloc/tests/slice.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/library/alloc/tests/slice.rs b/library/alloc/tests/slice.rs index 198f467eea352..13b8c059e37ae 100644 --- a/library/alloc/tests/slice.rs +++ b/library/alloc/tests/slice.rs @@ -1001,7 +1001,7 @@ fn test_split_iterators_size_hint() { Lower, Upper, } - fn assert_precise_size_hints(mut it: impl Iterator, which: Bounds, context: impl fmt::Display) { + fn assert_tight_size_hints(mut it: impl Iterator, which: Bounds, ctx: impl fmt::Display) { match which { Bounds::Lower => { let mut lower_bounds = vec![it.size_hint().0]; @@ -1009,7 +1009,7 @@ fn test_split_iterators_size_hint() { lower_bounds.push(it.size_hint().0); } let target: Vec<_> = (0..lower_bounds.len()).rev().collect(); - assert_eq!(lower_bounds, target, "incorrect lower bounds: {}", context); + assert_eq!(lower_bounds, target, "lower bounds incorrect or not tight: {}", ctx); } Bounds::Upper => { let mut upper_bounds = vec![it.size_hint().1]; @@ -1017,7 +1017,7 @@ fn test_split_iterators_size_hint() { upper_bounds.push(it.size_hint().1); } let target: Vec<_> = (0..upper_bounds.len()).map(Some).rev().collect(); - assert_eq!(upper_bounds, target, "incorrect upper bounds: {}", context); + assert_eq!(upper_bounds, target, "upper bounds incorrect or not tight: {}", ctx); } } } @@ -1028,13 +1028,13 @@ fn test_split_iterators_size_hint() { // p: predicate, b: bound selection for (p, b) in [ // with a predicate always returning false, the split*-iterators - // become maximally short, so the size_hint lower bounds are correct + // become maximally short, so the size_hint lower bounds are tight ((|_| false) as fn(&_) -> _, Bounds::Lower), // with a predicate always returning true, the split*-iterators - // become maximally long, so the size_hint upper bounds are correct + // become maximally long, so the size_hint upper bounds are tight ((|_| true) as fn(&_) -> _, Bounds::Upper), ] { - use assert_precise_size_hints as a; + use assert_tight_size_hints as a; use format_args as f; a(v.split(p), b, "split");