diff --git a/library/core/src/iter/adapters/intersperse.rs b/library/core/src/iter/adapters/intersperse.rs index bb94ed0a0a170..f64dea2df862c 100644 --- a/library/core/src/iter/adapters/intersperse.rs +++ b/library/core/src/iter/adapters/intersperse.rs @@ -1,5 +1,4 @@ use crate::fmt; -use crate::iter::{Fuse, FusedIterator}; /// An iterator adapter that places a separator between all elements. /// @@ -14,15 +13,7 @@ where started: bool, separator: I::Item, next_item: Option, - iter: Fuse, -} - -#[unstable(feature = "iter_intersperse", issue = "79524")] -impl FusedIterator for Intersperse -where - I: FusedIterator, - I::Item: Clone, -{ + iter: I, } impl Intersperse @@ -30,7 +21,7 @@ where I::Item: Clone, { pub(in crate::iter) fn new(iter: I, separator: I::Item) -> Self { - Self { started: false, separator, next_item: None, iter: iter.fuse() } + Self { started: false, separator, next_item: None, iter } } } @@ -57,8 +48,9 @@ where } } } else { - self.started = true; - self.iter.next() + let item = self.iter.next(); + self.started = item.is_some(); + item } } @@ -95,15 +87,7 @@ where started: bool, separator: G, next_item: Option, - iter: Fuse, -} - -#[unstable(feature = "iter_intersperse", issue = "79524")] -impl FusedIterator for IntersperseWith -where - I: FusedIterator, - G: FnMut() -> I::Item, -{ + iter: I, } #[unstable(feature = "iter_intersperse", issue = "79524")] @@ -146,7 +130,7 @@ where G: FnMut() -> I::Item, { pub(in crate::iter) fn new(iter: I, separator: G) -> Self { - Self { started: false, separator, next_item: None, iter: iter.fuse() } + Self { started: false, separator, next_item: None, iter } } } @@ -173,8 +157,9 @@ where } } } else { - self.started = true; - self.iter.next() + let item = self.iter.next(); + self.started = item.is_some(); + item } } diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index 2e82f45771823..11b1c04ce0e20 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -635,11 +635,24 @@ pub const trait Iterator { /// of the original iterator. /// /// Specifically on fused iterators, it is guaranteed that the new iterator - /// places a copy of `separator` between adjacent `Some(_)` items. However, - /// for non-fused iterators, [`intersperse`] will create a new iterator that - /// is a fused version of the original iterator and place a copy of `separator` - /// between adjacent `Some(_)` items. This behavior for non-fused iterators - /// is subject to change. + /// places a copy of `separator` between *adjacent* `Some(_)` items. For non-fused iterators, + /// it is guaranteed that [`intersperse`] will create a new iterator that places a copy + /// of `separator` between `Some(_)` items, particularly just right before the subsequent + /// `Some(_)` item. + /// + /// For example, consider the following non-fused iterator: + /// + /// ```text + /// Some(1) -> Some(2) -> None -> Some(3) -> Some(4) -> ... + /// ``` + /// + /// If this non-fused iterator were to be interspersed with `0`, + /// then the interspersed iterator will produce: + /// + /// ```text + /// Some(1) -> Some(0) -> Some(2) -> None -> Some(0) -> Some(3) -> Some(0) -> + /// Some(4) -> ... + /// ``` /// /// In case `separator` does not implement [`Clone`] or needs to be /// computed every time, use [`intersperse_with`]. @@ -688,10 +701,23 @@ pub const trait Iterator { /// /// Specifically on fused iterators, it is guaranteed that the new iterator /// places an item generated by `separator` between adjacent `Some(_)` items. - /// However, for non-fused iterators, [`intersperse_with`] will create a new - /// iterator that is a fused version of the original iterator and place an item - /// generated by `separator` between adjacent `Some(_)` items. This - /// behavior for non-fused iterators is subject to change. + /// For non-fused iterators, it is guaranteed that [`intersperse_with`] will + /// create a new iterator that places an item generated by `separator` between `Some(_)` + /// items, particularly just right before the subsequent `Some(_)` item. + /// + /// For example, consider the following non-fused iterator: + /// + /// ```text + /// Some(1) -> Some(2) -> None -> Some(3) -> Some(4) -> ... + /// ``` + /// + /// If this non-fused iterator were to be interspersed with a `separator` closure + /// that returns `0` repeatedly, the interspersed iterator will produce: + /// + /// ```text + /// Some(1) -> Some(0) -> Some(2) -> None -> Some(0) -> Some(3) -> Some(0) -> + /// Some(4) -> ... + /// ``` /// /// The `separator` closure will be called exactly once each time an item /// is placed between two adjacent items from the underlying iterator; diff --git a/library/coretests/tests/iter/adapters/intersperse.rs b/library/coretests/tests/iter/adapters/intersperse.rs index df6be79308be7..bcb7709077cd7 100644 --- a/library/coretests/tests/iter/adapters/intersperse.rs +++ b/library/coretests/tests/iter/adapters/intersperse.rs @@ -153,10 +153,6 @@ fn test_try_fold_specialization_intersperse_err() { assert_eq!(iter.next(), None); } -// FIXME(iter_intersperse): `intersperse` current behavior may change for -// non-fused iterators, so this test will likely have to -// be adjusted; see PR #152855 and issue #79524 -// if `intersperse` doesn't change, remove this FIXME. #[test] fn test_non_fused_iterator_intersperse() { #[derive(Debug)] @@ -183,24 +179,26 @@ fn test_non_fused_iterator_intersperse() { } let counter = 0; - // places a 2 between `Some(_)` items + // places a 1 between `Some(_)` items let non_fused_iter = TestCounter { counter }; - let mut intersperse_iter = non_fused_iter.intersperse(2); - // Since `intersperse` currently transforms the original - // iterator into a fused iterator, this intersperse_iter - // should always have `None` - for _ in 0..counter + 6 { - assert_eq!(intersperse_iter.next(), None); - } + let mut intersperse_iter = non_fused_iter.intersperse(1); + // Interspersed iter produces: + // `None` -> `Some(2)` -> `None` -> `Some(1)` -> Some(4)` -> `None` -> `Some(1)` -> + // `Some(6)` -> and then `None` endlessly + assert_eq!(intersperse_iter.next(), None); + assert_eq!(intersperse_iter.next(), Some(2)); + assert_eq!(intersperse_iter.next(), None); + assert_eq!(intersperse_iter.next(), Some(1)); + assert_eq!(intersperse_iter.next(), Some(4)); + assert_eq!(intersperse_iter.next(), None); + assert_eq!(intersperse_iter.next(), Some(1)); + assert_eq!(intersperse_iter.next(), Some(6)); + assert_eq!(intersperse_iter.next(), None); - // Extra check to make sure it is `None` after processing 6 items + // Extra check to make sure it is `None` after processing all items assert_eq!(intersperse_iter.next(), None); } -// FIXME(iter_intersperse): `intersperse` current behavior may change for -// non-fused iterators, so this test will likely have to -// be adjusted; see PR #152855 and issue #79524 -// if `intersperse` doesn't change, remove this FIXME. #[test] fn test_non_fused_iterator_intersperse_2() { #[derive(Debug)] @@ -228,35 +226,26 @@ fn test_non_fused_iterator_intersperse_2() { } let counter = 0; - // places a 2 between `Some(_)` items + // places a 100 between `Some(_)` items let non_fused_iter = TestCounter { counter }; - let mut intersperse_iter = non_fused_iter.intersperse(2); - // Since `intersperse` currently transforms the original - // iterator into a fused iterator, this interspersed iter - // will be `Some(1)` -> `Some(2)` -> `Some(2)` -> and then - // `None` endlessly - let mut items_processed = 0; - for num in 0..counter + 6 { - if num < 3 { - if num % 2 != 0 { - assert_eq!(intersperse_iter.next(), Some(2)); - } else { - items_processed += 1; - assert_eq!(intersperse_iter.next(), Some(items_processed)); - } - } else { - assert_eq!(intersperse_iter.next(), None); - } - } + let mut intersperse_iter = non_fused_iter.intersperse(100); + // Interspersed iter produces: + // `Some(1)` -> `Some(100)` -> `Some(2)` -> `None` -> `Some(100)` + // -> `Some(4)` -> `Some(100)` -> `Some(5)` -> `None` endlessly + assert_eq!(intersperse_iter.next(), Some(1)); + assert_eq!(intersperse_iter.next(), Some(100)); + assert_eq!(intersperse_iter.next(), Some(2)); + assert_eq!(intersperse_iter.next(), None); + assert_eq!(intersperse_iter.next(), Some(100)); + assert_eq!(intersperse_iter.next(), Some(4)); + assert_eq!(intersperse_iter.next(), Some(100)); + assert_eq!(intersperse_iter.next(), Some(5)); + assert_eq!(intersperse_iter.next(), None); // Extra check to make sure it is `None` after processing 6 items assert_eq!(intersperse_iter.next(), None); } -// FIXME(iter_intersperse): `intersperse_with` current behavior may change for -// non-fused iterators, so this test will likely have to -// be adjusted; see PR #152855 and issue #79524 -// if `intersperse_with` doesn't change, remove this FIXME. #[test] fn test_non_fused_iterator_intersperse_with() { #[derive(Debug)] @@ -285,22 +274,24 @@ fn test_non_fused_iterator_intersperse_with() { let counter = 0; let non_fused_iter = TestCounter { counter }; // places a 2 between `Some(_)` items - let mut intersperse_iter = non_fused_iter.intersperse_with(|| 2); - // Since `intersperse` currently transforms the original - // iterator into a fused iterator, this intersperse_iter - // should always have `None` - for _ in 0..counter + 6 { - assert_eq!(intersperse_iter.next(), None); - } + let mut intersperse_iter = non_fused_iter.intersperse_with(|| 1); + // Interspersed iter produces: + // `None` -> `Some(2)` -> `None` -> `Some(1)` -> Some(4)` -> `None` -> `Some(1)` -> + // `Some(6)` -> and then `None` endlessly + assert_eq!(intersperse_iter.next(), None); + assert_eq!(intersperse_iter.next(), Some(2)); + assert_eq!(intersperse_iter.next(), None); + assert_eq!(intersperse_iter.next(), Some(1)); + assert_eq!(intersperse_iter.next(), Some(4)); + assert_eq!(intersperse_iter.next(), None); + assert_eq!(intersperse_iter.next(), Some(1)); + assert_eq!(intersperse_iter.next(), Some(6)); + assert_eq!(intersperse_iter.next(), None); // Extra check to make sure it is `None` after processing 6 items assert_eq!(intersperse_iter.next(), None); } -// FIXME(iter_intersperse): `intersperse_with` current behavior may change for -// non-fused iterators, so this test will likely have to -// be adjusted; see PR #152855 and issue #79524 -// if `intersperse_with` doesn't change, remove this FIXME. #[test] fn test_non_fused_iterator_intersperse_with_2() { #[derive(Debug)] @@ -328,26 +319,21 @@ fn test_non_fused_iterator_intersperse_with_2() { } let counter = 0; - // places a 2 between `Some(_)` items + // places a 100 between `Some(_)` items let non_fused_iter = TestCounter { counter }; - let mut intersperse_iter = non_fused_iter.intersperse(2); - // Since `intersperse` currently transforms the original - // iterator into a fused iterator, this interspersed iter - // will be `Some(1)` -> `Some(2)` -> `Some(2)` -> and then - // `None` endlessly - let mut items_processed = 0; - for num in 0..counter + 6 { - if num < 3 { - if num % 2 != 0 { - assert_eq!(intersperse_iter.next(), Some(2)); - } else { - items_processed += 1; - assert_eq!(intersperse_iter.next(), Some(items_processed)); - } - } else { - assert_eq!(intersperse_iter.next(), None); - } - } + let mut intersperse_iter = non_fused_iter.intersperse_with(|| 100); + // Interspersed iter produces: + // `Some(1)` -> `Some(100)` -> `Some(2)` -> `None` -> `Some(100)` + // -> `Some(4)` -> `Some(100)` -> `Some(5)` -> `None` endlessly + assert_eq!(intersperse_iter.next(), Some(1)); + assert_eq!(intersperse_iter.next(), Some(100)); + assert_eq!(intersperse_iter.next(), Some(2)); + assert_eq!(intersperse_iter.next(), None); + assert_eq!(intersperse_iter.next(), Some(100)); + assert_eq!(intersperse_iter.next(), Some(4)); + assert_eq!(intersperse_iter.next(), Some(100)); + assert_eq!(intersperse_iter.next(), Some(5)); + assert_eq!(intersperse_iter.next(), None); // Extra check to make sure it is `None` after processing 6 items assert_eq!(intersperse_iter.next(), None);