about summary refs log tree commit diff
path: root/src/liballoc
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2019-11-15 01:23:51 +0000
committerbors <bors@rust-lang.org>2019-11-15 01:23:51 +0000
commit9e8c4e6fb1c952048fb823e59f4c9c6487bf9a58 (patch)
tree4526b81aafdaef185d9e83d8ebfe75d42313232b /src/liballoc
parentcefcc20898959a0c4fc11637cf8e53d3b6291125 (diff)
parent6da4df9fc95c1da7373f49c07742a42aaf840198 (diff)
downloadrust-9e8c4e6fb1c952048fb823e59f4c9c6487bf9a58.tar.gz
rust-9e8c4e6fb1c952048fb823e59f4c9c6487bf9a58.zip
Auto merge of #64432 - gnzlbg:simplify_truncate, r=alexcrichton
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
Diffstat (limited to 'src/liballoc')
-rw-r--r--src/liballoc/vec.rs34
1 files changed, 12 insertions, 22 deletions
diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs
index b1741f00507..4363314dc45 100644
--- a/src/liballoc/vec.rs
+++ b/src/liballoc/vec.rs
@@ -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);
         }
     }
 
@@ -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<'_> {