Don't fuse Chain in its second iterator
Only the "first" iterator is actually set `None` when exhausted, depending on whether you iterate forward or backward. This restores behavior similar to the former `ChainState`, where it would transition from `Both` to `Front`/`Back` and only continue from that side. However, if you mix directions, then this may still set both sides to `None`, totally fusing the iterator.
This commit is contained in:
parent
45d050cde2
commit
eeb687f20e
@ -18,6 +18,9 @@ pub struct Chain<A, B> {
|
|||||||
// adapter because its specialization for `FusedIterator` unconditionally descends into the
|
// adapter because its specialization for `FusedIterator` unconditionally descends into the
|
||||||
// iterator, and that could be expensive to keep revisiting stuff like nested chains. It also
|
// iterator, and that could be expensive to keep revisiting stuff like nested chains. It also
|
||||||
// hurts compiler performance to add more iterator layers to `Chain`.
|
// hurts compiler performance to add more iterator layers to `Chain`.
|
||||||
|
//
|
||||||
|
// Only the "first" iterator is actually set `None` when exhausted, depending on whether you
|
||||||
|
// iterate forward or backward. If you mix directions, then both sides may be `None`.
|
||||||
a: Option<A>,
|
a: Option<A>,
|
||||||
b: Option<B>,
|
b: Option<B>,
|
||||||
}
|
}
|
||||||
@ -43,6 +46,17 @@ macro_rules! fuse {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Try an iterator method without fusing,
|
||||||
|
/// like an inline `.as_mut().and_then(...)`
|
||||||
|
macro_rules! maybe {
|
||||||
|
($self:ident . $iter:ident . $($call:tt)+) => {
|
||||||
|
match $self.$iter {
|
||||||
|
Some(ref mut iter) => iter.$($call)+,
|
||||||
|
None => None,
|
||||||
|
}
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
#[stable(feature = "rust1", since = "1.0.0")]
|
#[stable(feature = "rust1", since = "1.0.0")]
|
||||||
impl<A, B> Iterator for Chain<A, B>
|
impl<A, B> Iterator for Chain<A, B>
|
||||||
where
|
where
|
||||||
@ -54,7 +68,7 @@ where
|
|||||||
#[inline]
|
#[inline]
|
||||||
fn next(&mut self) -> Option<A::Item> {
|
fn next(&mut self) -> Option<A::Item> {
|
||||||
match fuse!(self.a.next()) {
|
match fuse!(self.a.next()) {
|
||||||
None => fuse!(self.b.next()),
|
None => maybe!(self.b.next()),
|
||||||
item => item,
|
item => item,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -85,7 +99,7 @@ where
|
|||||||
}
|
}
|
||||||
if let Some(ref mut b) = self.b {
|
if let Some(ref mut b) = self.b {
|
||||||
acc = b.try_fold(acc, f)?;
|
acc = b.try_fold(acc, f)?;
|
||||||
self.b = None;
|
// we don't fuse the second iterator
|
||||||
}
|
}
|
||||||
Try::from_ok(acc)
|
Try::from_ok(acc)
|
||||||
}
|
}
|
||||||
@ -114,7 +128,7 @@ where
|
|||||||
}
|
}
|
||||||
self.a = None;
|
self.a = None;
|
||||||
}
|
}
|
||||||
fuse!(self.b.nth(n))
|
maybe!(self.b.nth(n))
|
||||||
}
|
}
|
||||||
|
|
||||||
#[inline]
|
#[inline]
|
||||||
@ -123,7 +137,7 @@ where
|
|||||||
P: FnMut(&Self::Item) -> bool,
|
P: FnMut(&Self::Item) -> bool,
|
||||||
{
|
{
|
||||||
match fuse!(self.a.find(&mut predicate)) {
|
match fuse!(self.a.find(&mut predicate)) {
|
||||||
None => fuse!(self.b.find(predicate)),
|
None => maybe!(self.b.find(predicate)),
|
||||||
item => item,
|
item => item,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -174,7 +188,7 @@ where
|
|||||||
#[inline]
|
#[inline]
|
||||||
fn next_back(&mut self) -> Option<A::Item> {
|
fn next_back(&mut self) -> Option<A::Item> {
|
||||||
match fuse!(self.b.next_back()) {
|
match fuse!(self.b.next_back()) {
|
||||||
None => fuse!(self.a.next_back()),
|
None => maybe!(self.a.next_back()),
|
||||||
item => item,
|
item => item,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -190,7 +204,7 @@ where
|
|||||||
}
|
}
|
||||||
self.b = None;
|
self.b = None;
|
||||||
}
|
}
|
||||||
fuse!(self.a.nth_back(n))
|
maybe!(self.a.nth_back(n))
|
||||||
}
|
}
|
||||||
|
|
||||||
#[inline]
|
#[inline]
|
||||||
@ -199,7 +213,7 @@ where
|
|||||||
P: FnMut(&Self::Item) -> bool,
|
P: FnMut(&Self::Item) -> bool,
|
||||||
{
|
{
|
||||||
match fuse!(self.b.rfind(&mut predicate)) {
|
match fuse!(self.b.rfind(&mut predicate)) {
|
||||||
None => fuse!(self.a.rfind(predicate)),
|
None => maybe!(self.a.rfind(predicate)),
|
||||||
item => item,
|
item => item,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -216,7 +230,7 @@ where
|
|||||||
}
|
}
|
||||||
if let Some(ref mut a) = self.a {
|
if let Some(ref mut a) = self.a {
|
||||||
acc = a.try_rfold(acc, f)?;
|
acc = a.try_rfold(acc, f)?;
|
||||||
self.a = None;
|
// we don't fuse the second iterator
|
||||||
}
|
}
|
||||||
Try::from_ok(acc)
|
Try::from_ok(acc)
|
||||||
}
|
}
|
||||||
@ -236,8 +250,6 @@ where
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Note: *both* must be fused to handle double-ended iterators.
|
// Note: *both* must be fused to handle double-ended iterators.
|
||||||
// Now that we "fuse" both sides, we *could* implement this unconditionally,
|
|
||||||
// but we should be cautious about committing to that in the public API.
|
|
||||||
#[stable(feature = "fused", since = "1.26.0")]
|
#[stable(feature = "fused", since = "1.26.0")]
|
||||||
impl<A, B> FusedIterator for Chain<A, B>
|
impl<A, B> FusedIterator for Chain<A, B>
|
||||||
where
|
where
|
||||||
|
@ -207,50 +207,64 @@ fn test_iterator_chain_find() {
|
|||||||
assert_eq!(iter.next(), None);
|
assert_eq!(iter.next(), None);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
struct Toggle {
|
||||||
|
is_empty: bool,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl Iterator for Toggle {
|
||||||
|
type Item = ();
|
||||||
|
|
||||||
|
// alternates between `None` and `Some(())`
|
||||||
|
fn next(&mut self) -> Option<Self::Item> {
|
||||||
|
if self.is_empty {
|
||||||
|
self.is_empty = false;
|
||||||
|
None
|
||||||
|
} else {
|
||||||
|
self.is_empty = true;
|
||||||
|
Some(())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn size_hint(&self) -> (usize, Option<usize>) {
|
||||||
|
if self.is_empty { (0, Some(0)) } else { (1, Some(1)) }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
impl DoubleEndedIterator for Toggle {
|
||||||
|
fn next_back(&mut self) -> Option<Self::Item> {
|
||||||
|
self.next()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_iterator_chain_size_hint() {
|
fn test_iterator_chain_size_hint() {
|
||||||
struct Iter {
|
|
||||||
is_empty: bool,
|
|
||||||
}
|
|
||||||
|
|
||||||
impl Iterator for Iter {
|
|
||||||
type Item = ();
|
|
||||||
|
|
||||||
// alternates between `None` and `Some(())`
|
|
||||||
fn next(&mut self) -> Option<Self::Item> {
|
|
||||||
if self.is_empty {
|
|
||||||
self.is_empty = false;
|
|
||||||
None
|
|
||||||
} else {
|
|
||||||
self.is_empty = true;
|
|
||||||
Some(())
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
fn size_hint(&self) -> (usize, Option<usize>) {
|
|
||||||
if self.is_empty { (0, Some(0)) } else { (1, Some(1)) }
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
impl DoubleEndedIterator for Iter {
|
|
||||||
fn next_back(&mut self) -> Option<Self::Item> {
|
|
||||||
self.next()
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// this chains an iterator of length 0 with an iterator of length 1,
|
// this chains an iterator of length 0 with an iterator of length 1,
|
||||||
// so after calling `.next()` once, the iterator is empty and the
|
// so after calling `.next()` once, the iterator is empty and the
|
||||||
// state is `ChainState::Back`. `.size_hint()` should now disregard
|
// state is `ChainState::Back`. `.size_hint()` should now disregard
|
||||||
// the size hint of the left iterator
|
// the size hint of the left iterator
|
||||||
let mut iter = Iter { is_empty: true }.chain(once(()));
|
let mut iter = Toggle { is_empty: true }.chain(once(()));
|
||||||
assert_eq!(iter.next(), Some(()));
|
assert_eq!(iter.next(), Some(()));
|
||||||
assert_eq!(iter.size_hint(), (0, Some(0)));
|
assert_eq!(iter.size_hint(), (0, Some(0)));
|
||||||
|
|
||||||
let mut iter = once(()).chain(Iter { is_empty: true });
|
let mut iter = once(()).chain(Toggle { is_empty: true });
|
||||||
assert_eq!(iter.next_back(), Some(()));
|
assert_eq!(iter.next_back(), Some(()));
|
||||||
assert_eq!(iter.size_hint(), (0, Some(0)));
|
assert_eq!(iter.size_hint(), (0, Some(0)));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_iterator_chain_unfused() {
|
||||||
|
// Chain shouldn't be fused in its second iterator, depending on direction
|
||||||
|
let mut iter = NonFused::new(empty()).chain(Toggle { is_empty: true });
|
||||||
|
iter.next().unwrap_none();
|
||||||
|
iter.next().unwrap();
|
||||||
|
iter.next().unwrap_none();
|
||||||
|
|
||||||
|
let mut iter = Toggle { is_empty: true }.chain(NonFused::new(empty()));
|
||||||
|
iter.next_back().unwrap_none();
|
||||||
|
iter.next_back().unwrap();
|
||||||
|
iter.next_back().unwrap_none();
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_zip_nth() {
|
fn test_zip_nth() {
|
||||||
let xs = [0, 1, 2, 4, 5];
|
let xs = [0, 1, 2, 4, 5];
|
||||||
|
@ -42,6 +42,7 @@
|
|||||||
#![feature(unwrap_infallible)]
|
#![feature(unwrap_infallible)]
|
||||||
#![feature(leading_trailing_ones)]
|
#![feature(leading_trailing_ones)]
|
||||||
#![feature(const_forget)]
|
#![feature(const_forget)]
|
||||||
|
#![feature(option_unwrap_none)]
|
||||||
|
|
||||||
extern crate test;
|
extern crate test;
|
||||||
|
|
||||||
|
Loading…
x
Reference in New Issue
Block a user