Skip to content

Commit

Permalink
Auto merge of #64432 - gnzlbg:simplify_truncate, r=alexcrichton
Browse files Browse the repository at this point in the history
Make the semantics of Vec::truncate(N) consistent with slices.

This commit simplifies the implementation of `Vec::truncate(N)` and
makes its semantics identical to dropping the `[vec.len() - N..]`
sub-slice tail of the vector, which is the same behavior as dropping a
vector containing the same sub-slice.

This changes two unspecified aspects of `Vec::truncate` behavior:

* the drop order, from back-to-front to front-to-back,
* the behavior of `Vec::truncate` on panics: if dropping one element of
  the tail panics, currently, `Vec::truncate` panics, but with this PR all other
  elements are still dropped, and if dropping a second element of the tail
  panics, with this PR, the program aborts.

Programs can trivially observe both changes. For example
([playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=7bef575b83b06e82b3e3529e4edbcac7)):

```rust
fn main() {
    struct Bomb(usize);
    impl Drop for Bomb {
        fn drop(&mut self) {
            panic!(format!("{}", self.0));
        }
    }
    let mut v = vec![Bomb(0), Bomb(1)];
    std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
        v.truncate(0);
    }));
    assert_eq!(v.len(), 1);
    std::mem::forget(v);
}
```

panics printing `1` today and succeeds. With this change, it panics
printing `0` first (due to the drop order change), and then aborts
with a double-panic printing `1`, just like dropping the
`[Bomb(0), Bomb(1)]` slice does, or dropping
`vec![Bomb(0), Bomb(1)]` does.

This needs to go through a crater run.

r? @SimonSapin
  • Loading branch information
bors committed Nov 15, 2019
2 parents cefcc20 + 6da4df9 commit 9e8c4e6
Showing 1 changed file with 12 additions and 22 deletions.
34 changes: 12 additions & 22 deletions src/liballoc/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -727,25 +727,20 @@ impl<T> Vec<T> {
/// [`drain`]: #method.drain
#[stable(feature = "rust1", since = "1.0.0")]
pub fn truncate(&mut self, len: usize) {
if mem::needs_drop::<T>() {
let current_len = self.len;
unsafe {
let mut ptr = self.as_mut_ptr().add(self.len);
// Set the final length at the end, keeping in mind that
// dropping an element might panic. Works around a missed
// optimization, as seen in the following issue:
// https://github.com/rust-lang/rust/issues/51802
let mut local_len = SetLenOnDrop::new(&mut self.len);

// drop any extra elements
for _ in len..current_len {
local_len.decrement_len(1);
ptr = ptr.offset(-1);
ptr::drop_in_place(ptr);
}
// This is safe because:
//
// * the slice passed to `drop_in_place` is valid; the `len > self.len`
// case avoids creating an invalid slice, and
// * the `len` of the vector is shrunk before calling `drop_in_place`,
// such that no value will be dropped twice in case `drop_in_place`
// were to panic once (if it panics twice, the program aborts).
unsafe {
if len > self.len {
return;
}
} else if len <= self.len {
let s = self.get_unchecked_mut(len..) as *mut _;
self.len = len;
ptr::drop_in_place(s);
}
}

Expand Down Expand Up @@ -1630,11 +1625,6 @@ impl<'a> SetLenOnDrop<'a> {
fn increment_len(&mut self, increment: usize) {
self.local_len += increment;
}

#[inline]
fn decrement_len(&mut self, decrement: usize) {
self.local_len -= decrement;
}
}

impl Drop for SetLenOnDrop<'_> {
Expand Down

0 comments on commit 9e8c4e6

Please sign in to comment.