diff options
| author | bors <bors@rust-lang.org> | 2019-11-11 19:20:54 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2019-11-11 19:20:54 +0000 |
| commit | bc0e288ad02ef362b5a6c42aaf61f2901c9b46db (patch) | |
| tree | 517fef0f33192847e5c3d5826e691c4a0247648f /src/liballoc | |
| parent | 56237d75b4271a8a2e0f47d86ea76ebf6d966152 (diff) | |
| parent | 27e0ab578cc0fc4c72da54bbeb42c0c44d848207 (diff) | |
| download | rust-bc0e288ad02ef362b5a6c42aaf61f2901c9b46db.tar.gz rust-bc0e288ad02ef362b5a6c42aaf61f2901c9b46db.zip | |
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.
Diffstat (limited to 'src/liballoc')
| -rw-r--r-- | src/liballoc/collections/vec_deque.rs | 32 | ||||
| -rw-r--r-- | src/liballoc/collections/vec_deque/tests.rs | 35 |
2 files changed, 63 insertions, 4 deletions
diff --git a/src/liballoc/collections/vec_deque.rs b/src/liballoc/collections/vec_deque.rs index 8f3dfabd888..7795083e058 100644 --- a/src/liballoc/collections/vec_deque.rs +++ b/src/liballoc/collections/vec_deque.rs @@ -835,7 +835,8 @@ impl<T> VecDeque<T> { } } - /// Shortens the `VecDeque`, dropping excess elements from the back. + /// Shortens the `VecDeque`, keeping the first `len` elements and dropping + /// the rest. /// /// If `len` is greater than the `VecDeque`'s current length, this has no /// effect. @@ -855,8 +856,31 @@ impl<T> VecDeque<T> { /// ``` #[stable(feature = "deque_extras", since = "1.16.0")] pub fn truncate(&mut self, len: usize) { - for _ in len..self.len() { - self.pop_back(); + // Safe because: + // + // * Any slice passed to `drop_in_place` is valid; the second case has + // `len <= front.len()` and returning on `len > self.len()` ensures + // `begin <= back.len()` in the first case + // * The head of the VecDeque is moved before calling `drop_in_place`, + // so no value is dropped twice if `drop_in_place` panics + unsafe { + if len > self.len() { + return; + } + let num_dropped = self.len() - len; + let (front, back) = self.as_mut_slices(); + if len > front.len() { + let begin = len - front.len(); + let drop_back = back.get_unchecked_mut(begin..) as *mut _; + self.head = self.wrap_sub(self.head, num_dropped); + ptr::drop_in_place(drop_back); + } else { + let drop_back = back as *mut _; + let drop_front = front.get_unchecked_mut(len..) as *mut _; + self.head = self.wrap_sub(self.head, num_dropped); + ptr::drop_in_place(drop_front); + ptr::drop_in_place(drop_back); + } } } @@ -1117,7 +1141,7 @@ impl<T> VecDeque<T> { #[stable(feature = "rust1", since = "1.0.0")] #[inline] pub fn clear(&mut self) { - self.drain(..); + self.truncate(0); } /// Returns `true` if the `VecDeque` contains an element equal to the diff --git a/src/liballoc/collections/vec_deque/tests.rs b/src/liballoc/collections/vec_deque/tests.rs index d578ee0dac4..09009ff516a 100644 --- a/src/liballoc/collections/vec_deque/tests.rs +++ b/src/liballoc/collections/vec_deque/tests.rs @@ -385,6 +385,41 @@ fn test_clone_from() { } #[test] +fn test_vec_deque_truncate_drop() { + static mut DROPS: u32 = 0; + #[derive(Clone)] + struct Elem(i32); + impl Drop for Elem { + fn drop(&mut self) { + unsafe { + DROPS += 1; + } + } + } + + let v = vec![Elem(1), Elem(2), Elem(3), Elem(4), Elem(5)]; + for push_front in 0..=v.len() { + let v = v.clone(); + let mut tester = VecDeque::with_capacity(5); + for (index, elem) in v.into_iter().enumerate() { + if index < push_front { + tester.push_front(elem); + } else { + tester.push_back(elem); + } + } + assert_eq!(unsafe { DROPS }, 0); + tester.truncate(3); + assert_eq!(unsafe { DROPS }, 2); + tester.truncate(0); + assert_eq!(unsafe { DROPS }, 5); + unsafe { + DROPS = 0; + } + } +} + +#[test] fn issue_53529() { use crate::boxed::Box; |
