Skip to content

Commit 97e4596

Browse files
committed
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
2 parents 56237d7 + 6da4df9 commit 97e4596

File tree

1 file changed

+12
-22
lines changed

1 file changed

+12
-22
lines changed

src/liballoc/vec.rs

+12-22
Original file line numberDiff line numberDiff line change
@@ -727,25 +727,20 @@ impl<T> Vec<T> {
727727
/// [`drain`]: #method.drain
728728
#[stable(feature = "rust1", since = "1.0.0")]
729729
pub fn truncate(&mut self, len: usize) {
730-
if mem::needs_drop::<T>() {
731-
let current_len = self.len;
732-
unsafe {
733-
let mut ptr = self.as_mut_ptr().add(self.len);
734-
// Set the final length at the end, keeping in mind that
735-
// dropping an element might panic. Works around a missed
736-
// optimization, as seen in the following issue:
737-
// https://github.com/rust-lang/rust/issues/51802
738-
let mut local_len = SetLenOnDrop::new(&mut self.len);
739-
740-
// drop any extra elements
741-
for _ in len..current_len {
742-
local_len.decrement_len(1);
743-
ptr = ptr.offset(-1);
744-
ptr::drop_in_place(ptr);
745-
}
730+
// This is safe because:
731+
//
732+
// * the slice passed to `drop_in_place` is valid; the `len > self.len`
733+
// case avoids creating an invalid slice, and
734+
// * the `len` of the vector is shrunk before calling `drop_in_place`,
735+
// such that no value will be dropped twice in case `drop_in_place`
736+
// were to panic once (if it panics twice, the program aborts).
737+
unsafe {
738+
if len > self.len {
739+
return;
746740
}
747-
} else if len <= self.len {
741+
let s = self.get_unchecked_mut(len..) as *mut _;
748742
self.len = len;
743+
ptr::drop_in_place(s);
749744
}
750745
}
751746

@@ -1630,11 +1625,6 @@ impl<'a> SetLenOnDrop<'a> {
16301625
fn increment_len(&mut self, increment: usize) {
16311626
self.local_len += increment;
16321627
}
1633-
1634-
#[inline]
1635-
fn decrement_len(&mut self, decrement: usize) {
1636-
self.local_len -= decrement;
1637-
}
16381628
}
16391629

16401630
impl Drop for SetLenOnDrop<'_> {

0 commit comments

Comments
 (0)