Skip to content

Commit bc0e288

Browse files
committed
Auto merge of #65933 - crgl:vec-deque-truncate, r=alexcrichton
Use ptr::drop_in_place for VecDeque::truncate and VecDeque::clear This commit allows `VecDeque::truncate` to take advantage of its (largely) contiguous memory layout and is consistent with the change in #64432 for `Vec`. As with the change to `Vec::truncate`, this changes both: - the drop order, from back-to-front to front-to-back - the behavior when dropping an element panics For consistency, it also changes the behavior when dropping an element panics for `VecDeque::clear`. These changes in behavior can be observed. This example ([playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=d0b1f2edc123437a2f704cbe8d93d828)) ```rust use std::collections::VecDeque; fn main() { struct Bomb(usize); impl Drop for Bomb { fn drop(&mut self) { panic!(format!("{}", self.0)); } } let mut v = VecDeque::from(vec![Bomb(0), Bomb(1)]); std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { v.truncate(0); })); std::mem::forget(v); } ``` panics printing `1` today and succeeds. `v.clear()` panics printing `0` today and succeeds. With the change, `v.clear()`, `v.truncate(0)`, and dropping the `VecDeque` all panic printing `0` first and then abort with a double-panic printing `1`. The motivation for this was making `VecDeque::truncate` more efficient since it was used in the implementation of `VecDeque::clone_from` (#65069), but it also makes behavior more consistent within the `VecDeque` and with `Vec` if that change is accepted (this probably doesn't make sense to merge if not). This might need a crater run and an FCP as well.
2 parents 56237d7 + 27e0ab5 commit bc0e288

File tree

2 files changed

+63
-4
lines changed

2 files changed

+63
-4
lines changed

src/liballoc/collections/vec_deque.rs

+28-4
Original file line numberDiff line numberDiff line change
@@ -835,7 +835,8 @@ impl<T> VecDeque<T> {
835835
}
836836
}
837837

838-
/// Shortens the `VecDeque`, dropping excess elements from the back.
838+
/// Shortens the `VecDeque`, keeping the first `len` elements and dropping
839+
/// the rest.
839840
///
840841
/// If `len` is greater than the `VecDeque`'s current length, this has no
841842
/// effect.
@@ -855,8 +856,31 @@ impl<T> VecDeque<T> {
855856
/// ```
856857
#[stable(feature = "deque_extras", since = "1.16.0")]
857858
pub fn truncate(&mut self, len: usize) {
858-
for _ in len..self.len() {
859-
self.pop_back();
859+
// Safe because:
860+
//
861+
// * Any slice passed to `drop_in_place` is valid; the second case has
862+
// `len <= front.len()` and returning on `len > self.len()` ensures
863+
// `begin <= back.len()` in the first case
864+
// * The head of the VecDeque is moved before calling `drop_in_place`,
865+
// so no value is dropped twice if `drop_in_place` panics
866+
unsafe {
867+
if len > self.len() {
868+
return;
869+
}
870+
let num_dropped = self.len() - len;
871+
let (front, back) = self.as_mut_slices();
872+
if len > front.len() {
873+
let begin = len - front.len();
874+
let drop_back = back.get_unchecked_mut(begin..) as *mut _;
875+
self.head = self.wrap_sub(self.head, num_dropped);
876+
ptr::drop_in_place(drop_back);
877+
} else {
878+
let drop_back = back as *mut _;
879+
let drop_front = front.get_unchecked_mut(len..) as *mut _;
880+
self.head = self.wrap_sub(self.head, num_dropped);
881+
ptr::drop_in_place(drop_front);
882+
ptr::drop_in_place(drop_back);
883+
}
860884
}
861885
}
862886

@@ -1117,7 +1141,7 @@ impl<T> VecDeque<T> {
11171141
#[stable(feature = "rust1", since = "1.0.0")]
11181142
#[inline]
11191143
pub fn clear(&mut self) {
1120-
self.drain(..);
1144+
self.truncate(0);
11211145
}
11221146

11231147
/// Returns `true` if the `VecDeque` contains an element equal to the

src/liballoc/collections/vec_deque/tests.rs

+35
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,41 @@ fn test_clone_from() {
384384
}
385385
}
386386

387+
#[test]
388+
fn test_vec_deque_truncate_drop() {
389+
static mut DROPS: u32 = 0;
390+
#[derive(Clone)]
391+
struct Elem(i32);
392+
impl Drop for Elem {
393+
fn drop(&mut self) {
394+
unsafe {
395+
DROPS += 1;
396+
}
397+
}
398+
}
399+
400+
let v = vec![Elem(1), Elem(2), Elem(3), Elem(4), Elem(5)];
401+
for push_front in 0..=v.len() {
402+
let v = v.clone();
403+
let mut tester = VecDeque::with_capacity(5);
404+
for (index, elem) in v.into_iter().enumerate() {
405+
if index < push_front {
406+
tester.push_front(elem);
407+
} else {
408+
tester.push_back(elem);
409+
}
410+
}
411+
assert_eq!(unsafe { DROPS }, 0);
412+
tester.truncate(3);
413+
assert_eq!(unsafe { DROPS }, 2);
414+
tester.truncate(0);
415+
assert_eq!(unsafe { DROPS }, 5);
416+
unsafe {
417+
DROPS = 0;
418+
}
419+
}
420+
}
421+
387422
#[test]
388423
fn issue_53529() {
389424
use crate::boxed::Box;

0 commit comments

Comments
 (0)