Skip to content

Commit 02cb8f2

Browse files
committed
Auto merge of #53564 - MaloJaffre:vecdeque, r=gnzlbg
Reoptimize VecDeque::append ~Unfortunately, I don't know if these changes fix the unsoundness mentioned in #53529, so it is stil a WIP. This is also completely untested. The VecDeque code contains other unsound code: one example : [reading unitialized memory](https://play.rust-lang.org/?gist=6ff47551769af61fd8adc45c44010887&version=nightly&mode=release&edition=2015) (detected by MIRI), so I think this code will need a bigger refactor to make it clearer and safer.~ Note: this is based on #53571. r? @SimonSapin Cc: #53529 #52553 @yorickpeterse @jonas-schievink @Pazzaz @shepmaster.
2 parents e6b35b0 + 21d2a6c commit 02cb8f2

File tree

1 file changed

+47
-5
lines changed

1 file changed

+47
-5
lines changed

src/liballoc/collections/vec_deque.rs

+47-5
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
use core::cmp::Ordering;
2121
use core::fmt;
22+
use core::isize;
2223
use core::iter::{repeat, FromIterator, FusedIterator};
2324
use core::mem;
2425
use core::ops::Bound::{Excluded, Included, Unbounded};
@@ -202,6 +203,33 @@ impl<T> VecDeque<T> {
202203
len);
203204
}
204205

206+
/// Copies all values from `src` to the back of `self`, wrapping around if needed.
207+
///
208+
/// # Safety
209+
///
210+
/// The capacity must be sufficient to hold self.len() + src.len() elements.
211+
/// If so, this function never panics.
212+
#[inline]
213+
unsafe fn copy_slice(&mut self, src: &[T]) {
214+
/// This is guaranteed by `RawVec`.
215+
debug_assert!(self.capacity() <= isize::MAX as usize);
216+
217+
let expected_new_len = self.len() + src.len();
218+
debug_assert!(self.capacity() >= expected_new_len);
219+
220+
let dst_high_ptr = self.ptr().add(self.head);
221+
let dst_high_len = self.cap() - self.head;
222+
223+
let split = cmp::min(src.len(), dst_high_len);
224+
let (src_high, src_low) = src.split_at(split);
225+
226+
ptr::copy_nonoverlapping(src_high.as_ptr(), dst_high_ptr, src_high.len());
227+
ptr::copy_nonoverlapping(src_low.as_ptr(), self.ptr(), src_low.len());
228+
229+
self.head = self.wrap_add(self.head, src.len());
230+
debug_assert!(self.len() == expected_new_len);
231+
}
232+
205233
/// Copies a potentially wrapping block of memory len long from src to dest.
206234
/// (abs(dst - src) + len) must be no larger than cap() (There must be at
207235
/// most one continuous overlapping region between src and dest).
@@ -1024,7 +1052,7 @@ impl<T> VecDeque<T> {
10241052
iter: Iter {
10251053
tail: drain_tail,
10261054
head: drain_head,
1027-
ring: unsafe { self.buffer_as_mut_slice() },
1055+
ring: unsafe { self.buffer_as_slice() },
10281056
},
10291057
}
10301058
}
@@ -1834,8 +1862,22 @@ impl<T> VecDeque<T> {
18341862
#[inline]
18351863
#[stable(feature = "append", since = "1.4.0")]
18361864
pub fn append(&mut self, other: &mut Self) {
1837-
// naive impl
1838-
self.extend(other.drain(..));
1865+
unsafe {
1866+
// Guarantees there is space in `self` for `other`.
1867+
self.reserve(other.len());
1868+
1869+
{
1870+
let (src_high, src_low) = other.as_slices();
1871+
1872+
// This is only safe because copy_slice never panics when capacity is sufficient.
1873+
self.copy_slice(src_low);
1874+
self.copy_slice(src_high);
1875+
}
1876+
1877+
// Some values now exist in both `other` and `self` but are made inaccessible
1878+
// in`other`.
1879+
other.tail = other.head;
1880+
}
18391881
}
18401882

18411883
/// Retains only the elements specified by the predicate.
@@ -2593,8 +2635,8 @@ impl<T> From<VecDeque<T>> for Vec<T> {
25932635
let mut right_offset = 0;
25942636
for i in left_edge..right_edge {
25952637
right_offset = (i - left_edge) % (cap - right_edge);
2596-
let src: isize = (right_edge + right_offset) as isize;
2597-
ptr::swap(buf.add(i), buf.offset(src));
2638+
let src = right_edge + right_offset;
2639+
ptr::swap(buf.add(i), buf.add(src));
25982640
}
25992641
let n_ops = right_edge - left_edge;
26002642
left_edge += n_ops;

0 commit comments

Comments
 (0)