Skip to content

Commit

Permalink
Rollup merge of #99434 - timvermeulen:skip_next_non_fused, r=scottmcm
Browse files Browse the repository at this point in the history
Fix `Skip::next` for non-fused inner iterators

`iter.skip(n).next()` will currently call `nth` and `next` in succession on `iter`, without checking whether `nth` exhausts the iterator. Using `?` to propagate a `None` value returned by `nth` avoids this.
  • Loading branch information
Dylan-DPC authored Jul 19, 2022
2 parents 415f7e1 + e52837c commit e301cd3
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 1 deletion.
2 changes: 1 addition & 1 deletion library/core/src/iter/adapters/skip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ where
#[inline]
fn next(&mut self) -> Option<I::Item> {
if unlikely(self.n > 0) {
self.iter.nth(crate::mem::take(&mut self.n) - 1);
self.iter.nth(crate::mem::take(&mut self.n) - 1)?;
}
self.iter.next()
}
Expand Down
11 changes: 11 additions & 0 deletions library/core/tests/iter/adapters/skip.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use core::iter::*;

use super::Unfuse;

#[test]
fn test_iterator_skip() {
let xs = [0, 1, 2, 3, 5, 13, 15, 16, 17, 19, 20, 30];
Expand Down Expand Up @@ -190,3 +192,12 @@ fn test_skip_nth_back() {
it.by_ref().skip(2).nth_back(10);
assert_eq!(it.next_back(), Some(&1));
}

#[test]
fn test_skip_non_fused() {
let non_fused = Unfuse::new(0..10);

// `Skip` would previously exhaust the iterator in this `next` call and then erroneously try to
// advance it further. `Unfuse` tests that this doesn't happen by panicking in that scenario.
let _ = non_fused.skip(20).next();
}

0 comments on commit e301cd3

Please sign in to comment.