Skip to content

Commit

Permalink
Auto merge of #85340 - the8472:no-inplaceiterable-on-peekable, r=yaahc
Browse files Browse the repository at this point in the history
remove InPlaceIterable marker from Peekable due to unsoundness

The unsoundness is not in Peekable per se, it rather is due to the
interaction between Peekable being able to hold an extra item
and vec::IntoIter's clone implementation shortening the allocation.

An alternative solution would be to change IntoIter's clone implementation
to keep enough spare capacity available.

fixes #85322
  • Loading branch information
bors committed May 19, 2021
2 parents f94942d + 7cb4e51 commit df70463
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 6 deletions.
1 change: 0 additions & 1 deletion library/alloc/benches/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,6 @@ fn bench_in_place_recycle(b: &mut Bencher) {
.enumerate()
.map(|(idx, e)| idx.wrapping_add(e))
.fuse()
.peekable()
.collect::<Vec<usize>>(),
);
});
Expand Down
13 changes: 12 additions & 1 deletion library/alloc/tests/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1002,7 +1002,6 @@ fn test_from_iter_specialization_with_iterator_adapters() {
.zip(std::iter::repeat(1usize))
.map(|(a, b)| a + b)
.map_while(Option::Some)
.peekable()
.skip(1)
.map(|e| if e != usize::MAX { Ok(std::num::NonZeroUsize::new(e)) } else { Err(()) });
assert_in_place_trait(&iter);
Expand Down Expand Up @@ -1095,6 +1094,18 @@ fn test_from_iter_specialization_panic_during_drop_leaks() {
}
}

// regression test for issue #85322. Peekable previously implemented InPlaceIterable,
// but due to an interaction with IntoIter's current Clone implementation it failed to uphold
// the contract.
#[test]
fn test_collect_after_iterator_clone() {
let v = vec![0; 5];
let mut i = v.into_iter().map(|i| i + 1).peekable();
i.peek();
let v = i.clone().collect::<Vec<_>>();
assert_eq!(v, [1, 1, 1, 1, 1]);
assert!(v.len() <= v.capacity());
}
#[test]
fn test_cow_from() {
let borrowed: &[_] = &["borrowed", "(slice)"];
Expand Down
5 changes: 1 addition & 4 deletions library/core/src/iter/adapters/peekable.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::iter::{adapters::SourceIter, FusedIterator, InPlaceIterable, TrustedLen};
use crate::iter::{adapters::SourceIter, FusedIterator, TrustedLen};
use crate::ops::{ControlFlow, Try};

/// An iterator with a `peek()` that returns an optional reference to the next
Expand Down Expand Up @@ -356,6 +356,3 @@ where
unsafe { SourceIter::as_inner(&mut self.iter) }
}
}

#[unstable(issue = "none", feature = "inplace_iteration")]
unsafe impl<I: InPlaceIterable> InPlaceIterable for Peekable<I> {}

0 comments on commit df70463

Please sign in to comment.