From 5fcafe3f41a1bc51f9d04fd726e24a1103a809e0 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Tue, 15 Nov 2022 00:04:55 -0800 Subject: [PATCH 1/2] `VecDeque::resize` should re-use the buffer in the passed-in element Today it always copies it for *every* appended element, but one of those clones is avoidable. --- alloc/src/collections/vec_deque/mod.rs | 9 +- alloc/src/lib.rs | 1 + alloc/tests/vec_deque.rs | 8 + core/src/iter/mod.rs | 2 + core/src/iter/sources.rs | 4 + core/src/iter/sources/repeat_n.rs | 201 +++++++++++++++++++++++++ core/tests/iter/sources.rs | 49 ++++++ core/tests/lib.rs | 1 + 8 files changed, 273 insertions(+), 2 deletions(-) create mode 100644 core/src/iter/sources/repeat_n.rs diff --git a/alloc/src/collections/vec_deque/mod.rs b/alloc/src/collections/vec_deque/mod.rs index 2a57dad89..537fe22a4 100644 --- a/alloc/src/collections/vec_deque/mod.rs +++ b/alloc/src/collections/vec_deque/mod.rs @@ -10,7 +10,7 @@ use core::cmp::{self, Ordering}; use core::fmt; use core::hash::{Hash, Hasher}; -use core::iter::{repeat_with, FromIterator}; +use core::iter::{repeat_n, repeat_with, FromIterator}; use core::marker::PhantomData; use core::mem::{ManuallyDrop, MaybeUninit, SizedTypeProperties}; use core::ops::{Index, IndexMut, Range, RangeBounds}; @@ -2833,7 +2833,12 @@ impl VecDeque { /// ``` #[stable(feature = "deque_extras", since = "1.16.0")] pub fn resize(&mut self, new_len: usize, value: T) { - self.resize_with(new_len, || value.clone()); + if new_len > self.len() { + let extra = new_len - self.len(); + self.extend(repeat_n(value, extra)) + } else { + self.truncate(new_len); + } } } diff --git a/alloc/src/lib.rs b/alloc/src/lib.rs index 008926666..5e13547ab 100644 --- a/alloc/src/lib.rs +++ b/alloc/src/lib.rs @@ -124,6 +124,7 @@ #![feature(inplace_iteration)] #![feature(iter_advance_by)] #![feature(iter_next_chunk)] +#![feature(iter_repeat_n)] #![feature(layout_for_ptr)] #![feature(maybe_uninit_slice)] #![feature(maybe_uninit_uninit_array)] diff --git a/alloc/tests/vec_deque.rs b/alloc/tests/vec_deque.rs index 019d73c0b..c1b9e7af9 100644 --- a/alloc/tests/vec_deque.rs +++ b/alloc/tests/vec_deque.rs @@ -1727,3 +1727,11 @@ fn test_from_zero_sized_vec() { let queue = VecDeque::from(v); assert_eq!(queue.len(), 100); } + +#[test] +fn test_resize_keeps_reserved_space_from_item() { + let v = Vec::::with_capacity(1234); + let mut d = VecDeque::new(); + d.resize(1, v); + assert_eq!(d[0].capacity(), 1234); +} diff --git a/core/src/iter/mod.rs b/core/src/iter/mod.rs index ef0f39782..bb35d50b4 100644 --- a/core/src/iter/mod.rs +++ b/core/src/iter/mod.rs @@ -401,6 +401,8 @@ pub use self::sources::{once, Once}; pub use self::sources::{once_with, OnceWith}; #[stable(feature = "rust1", since = "1.0.0")] pub use self::sources::{repeat, Repeat}; +#[unstable(feature = "iter_repeat_n", issue = "104434")] +pub use self::sources::{repeat_n, RepeatN}; #[stable(feature = "iterator_repeat_with", since = "1.28.0")] pub use self::sources::{repeat_with, RepeatWith}; #[stable(feature = "iter_successors", since = "1.34.0")] diff --git a/core/src/iter/sources.rs b/core/src/iter/sources.rs index d34772cd3..3ec426a3a 100644 --- a/core/src/iter/sources.rs +++ b/core/src/iter/sources.rs @@ -4,6 +4,7 @@ mod from_generator; mod once; mod once_with; mod repeat; +mod repeat_n; mod repeat_with; mod successors; @@ -16,6 +17,9 @@ pub use self::empty::{empty, Empty}; #[stable(feature = "iter_once", since = "1.2.0")] pub use self::once::{once, Once}; +#[unstable(feature = "iter_repeat_n", issue = "104434")] +pub use self::repeat_n::{repeat_n, RepeatN}; + #[stable(feature = "iterator_repeat_with", since = "1.28.0")] pub use self::repeat_with::{repeat_with, RepeatWith}; diff --git a/core/src/iter/sources/repeat_n.rs b/core/src/iter/sources/repeat_n.rs new file mode 100644 index 000000000..538f3bed5 --- /dev/null +++ b/core/src/iter/sources/repeat_n.rs @@ -0,0 +1,201 @@ +use crate::iter::{FusedIterator, TrustedLen}; +use crate::mem::ManuallyDrop; + +/// Creates a new iterator that repeats a single element a given number of times. +/// +/// The `repeat_n()` function repeats a single value exactly `n` times. +/// +/// This is very similar to using [`repeat()`] with [`Iterator::take()`], +/// but there are two differences: +/// - `repeat_n()` can return the original value, rather than always cloning. +/// - `repeat_n()` produces an [`ExactSizeIterator`]. +/// +/// [`repeat()`]: crate::iter::repeat +/// +/// # Examples +/// +/// Basic usage: +/// +/// ``` +/// #![feature(iter_repeat_n)] +/// use std::iter; +/// +/// // four of the the number four: +/// let mut four_fours = iter::repeat_n(4, 4); +/// +/// assert_eq!(Some(4), four_fours.next()); +/// assert_eq!(Some(4), four_fours.next()); +/// assert_eq!(Some(4), four_fours.next()); +/// assert_eq!(Some(4), four_fours.next()); +/// +/// // no more fours +/// assert_eq!(None, four_fours.next()); +/// ``` +/// +/// For non-`Copy` types, +/// +/// ``` +/// #![feature(iter_repeat_n)] +/// use std::iter; +/// +/// let v: Vec = Vec::with_capacity(123); +/// let mut it = iter::repeat_n(v, 5); +/// +/// for i in 0..4 { +/// // It starts by cloning things +/// let cloned = it.next().unwrap(); +/// assert_eq!(cloned.len(), 0); +/// assert_eq!(cloned.capacity(), 0); +/// } +/// +/// // ... but the last item is the original one +/// let last = it.next().unwrap(); +/// assert_eq!(last.len(), 0); +/// assert_eq!(last.capacity(), 123); +/// +/// // ... and now we're done +/// assert_eq!(None, it.next()); +/// ``` +#[inline] +#[unstable( + feature = "iter_repeat_n", + reason = "waiting on FCP to decide whether to expose publicly", + issue = "104434" +)] +pub fn repeat_n(element: T, count: usize) -> RepeatN { + let mut element = ManuallyDrop::new(element); + + if count == 0 { + // SAFETY: we definitely haven't dropped it yet, since we only just got + // passed it in, and because the count is zero the instance we're about + // to create won't drop it, so to avoid leaking we need to now. + unsafe { ManuallyDrop::drop(&mut element) }; + } + + RepeatN { element, count } +} + +/// An iterator that repeats an element an exact number of times. +/// +/// This `struct` is created by the [`repeat_n()`] function. +/// See its documentation for more. +#[derive(Clone, Debug)] +#[unstable( + feature = "iter_repeat_n", + reason = "waiting on FCP to decide whether to expose publicly", + issue = "104434" +)] +pub struct RepeatN { + count: usize, + // Invariant: has been dropped iff count == 0. + element: ManuallyDrop, +} + +impl RepeatN { + /// If we haven't already dropped the element, return it in an option. + /// + /// Clears the count so it won't be dropped again later. + #[inline] + fn take_element(&mut self) -> Option { + if self.count > 0 { + self.count = 0; + // SAFETY: We just set count to zero so it won't be dropped again, + // and it used to be non-zero so it hasn't already been dropped. + unsafe { Some(ManuallyDrop::take(&mut self.element)) } + } else { + None + } + } +} + +#[unstable(feature = "iter_repeat_n", issue = "104434")] +impl Drop for RepeatN { + fn drop(&mut self) { + self.take_element(); + } +} + +#[unstable(feature = "iter_repeat_n", issue = "104434")] +impl Iterator for RepeatN { + type Item = A; + + #[inline] + fn next(&mut self) -> Option { + if self.count == 0 { + return None; + } + + self.count -= 1; + Some(if self.count == 0 { + // SAFETY: the check above ensured that the count used to be non-zero, + // so element hasn't been dropped yet, and we just lowered the count to + // zero so it won't be dropped later, and thus it's okay to take it here. + unsafe { ManuallyDrop::take(&mut self.element) } + } else { + A::clone(&mut self.element) + }) + } + + #[inline] + fn size_hint(&self) -> (usize, Option) { + let len = self.len(); + (len, Some(len)) + } + + #[inline] + fn advance_by(&mut self, skip: usize) -> Result<(), usize> { + let len = self.count; + + if skip >= len { + self.take_element(); + } + + if skip > len { + Err(len) + } else { + self.count = len - skip; + Ok(()) + } + } + + #[inline] + fn last(mut self) -> Option { + self.take_element() + } + + #[inline] + fn count(self) -> usize { + self.len() + } +} + +#[unstable(feature = "iter_repeat_n", issue = "104434")] +impl ExactSizeIterator for RepeatN { + fn len(&self) -> usize { + self.count + } +} + +#[unstable(feature = "iter_repeat_n", issue = "104434")] +impl DoubleEndedIterator for RepeatN { + #[inline] + fn next_back(&mut self) -> Option { + self.next() + } + + #[inline] + fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { + self.advance_by(n) + } + + #[inline] + fn nth_back(&mut self, n: usize) -> Option { + self.nth(n) + } +} + +#[unstable(feature = "iter_repeat_n", issue = "104434")] +impl FusedIterator for RepeatN {} + +#[unstable(feature = "trusted_len", issue = "37572")] +unsafe impl TrustedLen for RepeatN {} diff --git a/core/tests/iter/sources.rs b/core/tests/iter/sources.rs index d0114ade6..a15f3a514 100644 --- a/core/tests/iter/sources.rs +++ b/core/tests/iter/sources.rs @@ -106,3 +106,52 @@ fn test_empty() { let mut it = empty::(); assert_eq!(it.next(), None); } + +#[test] +fn test_repeat_n_drop() { + #[derive(Clone, Debug)] + struct DropCounter<'a>(&'a Cell); + impl Drop for DropCounter<'_> { + fn drop(&mut self) { + self.0.set(self.0.get() + 1); + } + } + + // `repeat_n(x, 0)` drops `x` immediately + let count = Cell::new(0); + let item = DropCounter(&count); + let mut it = repeat_n(item, 0); + assert_eq!(count.get(), 1); + assert!(it.next().is_none()); + assert_eq!(count.get(), 1); + drop(it); + assert_eq!(count.get(), 1); + + // Dropping the iterator needs to drop the item if it's non-empty + let count = Cell::new(0); + let item = DropCounter(&count); + let it = repeat_n(item, 3); + assert_eq!(count.get(), 0); + drop(it); + assert_eq!(count.get(), 1); + + // Dropping the iterator doesn't drop the item if it was exhausted + let count = Cell::new(0); + let item = DropCounter(&count); + let mut it = repeat_n(item, 3); + assert_eq!(count.get(), 0); + let x0 = it.next().unwrap(); + assert_eq!(count.get(), 0); + let x1 = it.next().unwrap(); + assert_eq!(count.get(), 0); + let x2 = it.next().unwrap(); + assert_eq!(count.get(), 0); + assert!(it.next().is_none()); + assert_eq!(count.get(), 0); + assert!(it.next().is_none()); + assert_eq!(count.get(), 0); + drop(it); + assert_eq!(count.get(), 0); + drop((x0, x1, x2)); + assert_eq!(count.get(), 3); +} diff --git a/core/tests/lib.rs b/core/tests/lib.rs index 61d31b344..58e9045c9 100644 --- a/core/tests/lib.rs +++ b/core/tests/lib.rs @@ -73,6 +73,7 @@ #![feature(iter_is_partitioned)] #![feature(iter_next_chunk)] #![feature(iter_order_by)] +#![feature(iter_repeat_n)] #![feature(iterator_try_collect)] #![feature(iterator_try_reduce)] #![feature(const_mut_refs)] From bf1c98d5ddde2bd3378722a107753f0a4daa56d0 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Fri, 18 Nov 2022 19:46:18 -0800 Subject: [PATCH 2/2] Hide the items while waiting for the ACP --- core/src/iter/sources/repeat_n.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/core/src/iter/sources/repeat_n.rs b/core/src/iter/sources/repeat_n.rs index 538f3bed5..dc69bf4df 100644 --- a/core/src/iter/sources/repeat_n.rs +++ b/core/src/iter/sources/repeat_n.rs @@ -57,11 +57,8 @@ use crate::mem::ManuallyDrop; /// assert_eq!(None, it.next()); /// ``` #[inline] -#[unstable( - feature = "iter_repeat_n", - reason = "waiting on FCP to decide whether to expose publicly", - issue = "104434" -)] +#[unstable(feature = "iter_repeat_n", issue = "104434")] +#[doc(hidden)] // waiting on ACP#120 to decide whether to expose publicly pub fn repeat_n(element: T, count: usize) -> RepeatN { let mut element = ManuallyDrop::new(element); @@ -80,11 +77,8 @@ pub fn repeat_n(element: T, count: usize) -> RepeatN { /// This `struct` is created by the [`repeat_n()`] function. /// See its documentation for more. #[derive(Clone, Debug)] -#[unstable( - feature = "iter_repeat_n", - reason = "waiting on FCP to decide whether to expose publicly", - issue = "104434" -)] +#[unstable(feature = "iter_repeat_n", issue = "104434")] +#[doc(hidden)] // waiting on ACP#120 to decide whether to expose publicly pub struct RepeatN { count: usize, // Invariant: has been dropped iff count == 0.