diff --git a/library/alloc/src/collections/vec_deque/splice.rs b/library/alloc/src/collections/vec_deque/splice.rs index d7b9a96291c39..b82f9fba7ceb3 100644 --- a/library/alloc/src/collections/vec_deque/splice.rs +++ b/library/alloc/src/collections/vec_deque/splice.rs @@ -138,8 +138,48 @@ impl Drain<'_, T, A> { /// self.deque must be valid. unsafe fn move_tail(&mut self, additional: usize) { let deque = unsafe { self.deque.as_mut() }; - let tail_start = deque.len + self.drain_len; - deque.buf.reserve(tail_start + self.tail_len, additional); + + // `Drain::new` modifies the deque's len (so does `Drain::fill` here) + // directly with the start bound of the range passed into + // `VecDeque::splice`. This causes a few different issue: + // - Most notably, there will be a hole at the end of the + // buffer when our buffer resizes in the case that our + // data wraps around. + // - We cannot use `VecDeque::reserve` directly because + // how it reserves more space and updates the `VecDeque`'s + // `head` field accordingly depends on the `VecDeque`'s + // actual `len`. + // - We cannot just directly modify `VecDeque`'s `len` and + // and call `VecDeque::reserve` afterward because if + // `VecDeque::reserve` panics on capacity overflow, + // well now our `VecDeque`'s head does not get updated + // and we still have a potential hole at the end of the + // buffer. + // Therefore, we manually reserve additional space (if necessary) + // based on calculating the actual `len` of the `VecDeque` and adjust + // `VecDeque`'s len right *after* the panicking region of `VecDeque::reserve` + // (that is `RawVec` `reserve()` call) + + let drain_start = deque.len; + let tail_start = drain_start + self.drain_len; + + // Actual VecDeque's len = drain_start + tail_len + drain_len + let actual_len = drain_start + self.tail_len + self.drain_len; + let new_cap = actual_len.checked_add(additional).expect("capacity overflow"); + let old_cap = deque.capacity(); + + if new_cap > old_cap { + deque.buf.reserve(actual_len, additional); + // If new_cap doesn't panic, we can safely set the `VecDeque` len to its + // actual len; this needs to be done in order to set deque.head correctly + // on `VecDeque::handle_capacity_increase` + deque.len = actual_len; + // SAFETY: this cannot panic since our internal buffer's new_cap should + // be bigger than the passed in old_cap + unsafe { + deque.handle_capacity_increase(old_cap); + } + } let new_tail_start = tail_start + additional; unsafe { @@ -149,6 +189,9 @@ impl Drain<'_, T, A> { self.tail_len, ); } + + // revert the `VecDeque` len to what it was before + deque.len = drain_start; self.drain_len += additional; } } diff --git a/library/alloctests/tests/vec_deque.rs b/library/alloctests/tests/vec_deque.rs index 92853fe00fd63..91843dfd00585 100644 --- a/library/alloctests/tests/vec_deque.rs +++ b/library/alloctests/tests/vec_deque.rs @@ -2347,3 +2347,16 @@ fn test_splice_wrapping() { assert_eq!(Vec::from(vec), [7, 8, 9]); } + +#[test] +fn test_splice_wrapping_and_resize() { + let mut vec = VecDeque::new(); + + for i in [1; 6] { + vec.push_front(i); + } + + vec.splice(1..1, [2, 3, 4]); + + assert_eq!(Vec::from(vec), [1, 2, 3, 4, 1, 1, 1, 1, 1]) +}