Don't fuse Chain in its second iterator

src/libcore/iter/adapters/chain.rs

@@ -18,6 +18,9 @@ pub struct Chain<A, B> {
  // 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
  // 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>,
  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")]
 impl<A, B> Iterator for Chain<A, B>
 where
@@ -54,7 +68,7 @@ where
  #[inline]
  fn next(&mut self) -> Option<A::Item> {
  match fuse!(self.a.next()) {
- None => fuse!(self.b.next()),
+ None => maybe!(self.b.next()),
  item => item,
  }
  }
@@ -85,7 +99,7 @@ where
  }
  if let Some(ref mut b) = self.b {
  acc = b.try_fold(acc, f)?;
- self.b = None;
+ // we don't fuse the second iterator
  }
  Try::from_ok(acc)
  }
@@ -114,7 +128,7 @@ where
  }
  self.a = None;
  }
- fuse!(self.b.nth(n))
+ maybe!(self.b.nth(n))
  }
 
  #[inline]
@@ -123,7 +137,7 @@ where
  P: FnMut(&Self::Item) -> bool,
  {
  match fuse!(self.a.find(&mut predicate)) {
- None => fuse!(self.b.find(predicate)),
+ None => maybe!(self.b.find(predicate)),
  item => item,
  }
  }
@@ -174,7 +188,7 @@ where
  #[inline]
  fn next_back(&mut self) -> Option<A::Item> {
  match fuse!(self.b.next_back()) {
- None => fuse!(self.a.next_back()),
+ None => maybe!(self.a.next_back()),
  item => item,
  }
  }
@@ -190,7 +204,7 @@ where
  }
  self.b = None;
  }
- fuse!(self.a.nth_back(n))
+ maybe!(self.a.nth_back(n))
  }
 
  #[inline]
@@ -199,7 +213,7 @@ where
  P: FnMut(&Self::Item) -> bool,
  {
  match fuse!(self.b.rfind(&mut predicate)) {
- None => fuse!(self.a.rfind(predicate)),
+ None => maybe!(self.a.rfind(predicate)),
  item => item,
  }
  }
@@ -216,7 +230,7 @@ where
  }
  if let Some(ref mut a) = self.a {
  acc = a.try_rfold(acc, f)?;
- self.a = None;
+ // we don't fuse the second iterator
  }
  Try::from_ok(acc)
  }
@@ -236,8 +250,6 @@ where
 }
 
 // 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")]
 impl<A, B> FusedIterator for Chain<A, B>
 where

src/libcore/tests/iter.rs

@@ -207,50 +207,64 @@ fn test_iterator_chain_find() {
  assert_eq!(iter.next(), None);
 }
 
-#[test]
-fn test_iterator_chain_size_hint() {
- struct Iter {
- is_empty: bool,
- }
+struct Toggle {
+ is_empty: bool,
+}
 
- impl Iterator for Iter {
-  type Item = ();
+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(())
- }
+ // 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)) }
- }
+ 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()
- }
+impl DoubleEndedIterator for Toggle {
+ fn next_back(&mut self) -> Option<Self::Item> {
+ self.next()
  }
+}
 
+#[test]
+fn test_iterator_chain_size_hint() {
  // this chains an iterator of length 0 with an iterator of length 1,
  // so after calling `.next()` once, the iterator is empty and the
  // state is `ChainState::Back`. `.size_hint()` should now disregard
  // 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.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.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]
 fn test_zip_nth() {
  let xs = [0, 1, 2, 4, 5];

src/libcore/tests/lib.rs

@@ -42,6 +42,7 @@
 #![feature(unwrap_infallible)]
 #![feature(leading_trailing_ones)]
 #![feature(const_forget)]
+#![feature(option_unwrap_none)]
 
 extern crate test;