bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Make the semantics of Vec::truncate(N) consistent with slices.

Browse source 
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 commit is contained in:
gnzlbg 2019-09-13 17:14:57 +02:00
parent 572d3d9634
commit 6da4df9fc9
1 changed files with 12 additions and 22 deletions
Download patch file Download diff file Expand all files Collapse all files

32
src/liballoc/vec.rs
View file

@ -685,25 +685,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;
// 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 {
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);
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);
}
}
@ -1590,11 +1585,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<'_> {