Skip to content

Commit 335ffbf

Browse files
committed
Auto merge of #94472 - JmPotato:use_maybeuninit_for_vecdeque, r=m-ou-se
Use MaybeUninit in VecDeque to remove the undefined behavior of slice Signed-off-by: JmPotato <ghzpotato@gmail.com> Ref #74189. Adjust the code to follow the [doc.rust-lang.org/reference/behavior-considered-undefined.html](https://doc.rust-lang.org/reference/behavior-considered-undefined.html). * Change the return type of `buffer_as_slice` from `&[T]` to `&[MaybeUninit<T>]`. * Add some corresponding safety comments. Benchmark results: master 8d6f527 ```rust test collections::vec_deque::tests::bench_pop_back_100 ... bench: 47 ns/iter (+/- 1) test collections::vec_deque::tests::bench_pop_front_100 ... bench: 50 ns/iter (+/- 4) test collections::vec_deque::tests::bench_push_back_100 ... bench: 69 ns/iter (+/- 10) test collections::vec_deque::tests::bench_push_front_100 ... bench: 72 ns/iter (+/- 6) test collections::vec_deque::tests::bench_retain_half_10000 ... bench: 145,891 ns/iter (+/- 7,975) test collections::vec_deque::tests::bench_retain_odd_10000 ... bench: 141,647 ns/iter (+/- 3,711) test collections::vec_deque::tests::bench_retain_whole_10000 ... bench: 120,132 ns/iter (+/- 4,078) ``` This PR ```rust test collections::vec_deque::tests::bench_pop_back_100 ... bench: 48 ns/iter (+/- 2) test collections::vec_deque::tests::bench_pop_front_100 ... bench: 51 ns/iter (+/- 3) test collections::vec_deque::tests::bench_push_back_100 ... bench: 73 ns/iter (+/- 2) test collections::vec_deque::tests::bench_push_front_100 ... bench: 73 ns/iter (+/- 2) test collections::vec_deque::tests::bench_retain_half_10000 ... bench: 131,796 ns/iter (+/- 5,440) test collections::vec_deque::tests::bench_retain_odd_10000 ... bench: 137,563 ns/iter (+/- 3,349) test collections::vec_deque::tests::bench_retain_whole_10000 ... bench: 128,815 ns/iter (+/- 3,289) ```
2 parents c9b45e6 + 2f18fa8 commit 335ffbf

File tree

2 files changed

+95
-30
lines changed

2 files changed

+95
-30
lines changed

library/alloc/src/collections/vec_deque/iter.rs

+50-19
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use core::fmt;
22
use core::iter::{FusedIterator, TrustedLen, TrustedRandomAccess, TrustedRandomAccessNoCoerce};
3+
use core::mem::MaybeUninit;
34
use core::ops::Try;
45

56
use super::{count, wrap_index, RingSlices};
@@ -12,7 +13,7 @@ use super::{count, wrap_index, RingSlices};
1213
/// [`iter`]: super::VecDeque::iter
1314
#[stable(feature = "rust1", since = "1.0.0")]
1415
pub struct Iter<'a, T: 'a> {
15-
pub(crate) ring: &'a [T],
16+
pub(crate) ring: &'a [MaybeUninit<T>],
1617
pub(crate) tail: usize,
1718
pub(crate) head: usize,
1819
}
@@ -21,7 +22,15 @@ pub struct Iter<'a, T: 'a> {
2122
impl<T: fmt::Debug> fmt::Debug for Iter<'_, T> {
2223
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
2324
let (front, back) = RingSlices::ring_slices(self.ring, self.head, self.tail);
24-
f.debug_tuple("Iter").field(&front).field(&back).finish()
25+
// Safety:
26+
// - `self.head` and `self.tail` in a ring buffer are always valid indices.
27+
// - `RingSlices::ring_slices` guarantees that the slices split according to `self.head` and `self.tail` are initialized.
28+
unsafe {
29+
f.debug_tuple("Iter")
30+
.field(&MaybeUninit::slice_assume_init_ref(front))
31+
.field(&MaybeUninit::slice_assume_init_ref(back))
32+
.finish()
33+
}
2534
}
2635
}
2736

@@ -44,7 +53,10 @@ impl<'a, T> Iterator for Iter<'a, T> {
4453
}
4554
let tail = self.tail;
4655
self.tail = wrap_index(self.tail.wrapping_add(1), self.ring.len());
47-
unsafe { Some(self.ring.get_unchecked(tail)) }
56+
// Safety:
57+
// - `self.tail` in a ring buffer is always a valid index.
58+
// - `self.head` and `self.tail` equality is checked above.
59+
unsafe { Some(self.ring.get_unchecked(tail).assume_init_ref()) }
4860
}
4961

5062
#[inline]
@@ -58,8 +70,13 @@ impl<'a, T> Iterator for Iter<'a, T> {
5870
F: FnMut(Acc, Self::Item) -> Acc,
5971
{
6072
let (front, back) = RingSlices::ring_slices(self.ring, self.head, self.tail);
61-
accum = front.iter().fold(accum, &mut f);
62-
back.iter().fold(accum, &mut f)
73+
// Safety:
74+
// - `self.head` and `self.tail` in a ring buffer are always valid indices.
75+
// - `RingSlices::ring_slices` guarantees that the slices split according to `self.head` and `self.tail` are initialized.
76+
unsafe {
77+
accum = MaybeUninit::slice_assume_init_ref(front).iter().fold(accum, &mut f);
78+
MaybeUninit::slice_assume_init_ref(back).iter().fold(accum, &mut f)
79+
}
6380
}
6481

6582
fn try_fold<B, F, R>(&mut self, init: B, mut f: F) -> R
@@ -70,17 +87,19 @@ impl<'a, T> Iterator for Iter<'a, T> {
7087
{
7188
let (mut iter, final_res);
7289
if self.tail <= self.head {
73-
// single slice self.ring[self.tail..self.head]
74-
iter = self.ring[self.tail..self.head].iter();
90+
// Safety: single slice self.ring[self.tail..self.head] is initialized.
91+
iter = unsafe { MaybeUninit::slice_assume_init_ref(&self.ring[self.tail..self.head]) }
92+
.iter();
7593
final_res = iter.try_fold(init, &mut f);
7694
} else {
77-
// two slices: self.ring[self.tail..], self.ring[..self.head]
95+
// Safety: two slices: self.ring[self.tail..], self.ring[..self.head] both are initialized.
7896
let (front, back) = self.ring.split_at(self.tail);
79-
let mut back_iter = back.iter();
97+
98+
let mut back_iter = unsafe { MaybeUninit::slice_assume_init_ref(back).iter() };
8099
let res = back_iter.try_fold(init, &mut f);
81100
let len = self.ring.len();
82101
self.tail = (self.ring.len() - back_iter.len()) & (len - 1);
83-
iter = front[..self.head].iter();
102+
iter = unsafe { MaybeUninit::slice_assume_init_ref(&front[..self.head]).iter() };
84103
final_res = iter.try_fold(res?, &mut f);
85104
}
86105
self.tail = self.head - iter.len();
@@ -109,7 +128,7 @@ impl<'a, T> Iterator for Iter<'a, T> {
109128
// that is in bounds.
110129
unsafe {
111130
let idx = wrap_index(self.tail.wrapping_add(idx), self.ring.len());
112-
self.ring.get_unchecked(idx)
131+
self.ring.get_unchecked(idx).assume_init_ref()
113132
}
114133
}
115134
}
@@ -122,16 +141,24 @@ impl<'a, T> DoubleEndedIterator for Iter<'a, T> {
122141
return None;
123142
}
124143
self.head = wrap_index(self.head.wrapping_sub(1), self.ring.len());
125-
unsafe { Some(self.ring.get_unchecked(self.head)) }
144+
// Safety:
145+
// - `self.head` in a ring buffer is always a valid index.
146+
// - `self.head` and `self.tail` equality is checked above.
147+
unsafe { Some(self.ring.get_unchecked(self.head).assume_init_ref()) }
126148
}
127149

128150
fn rfold<Acc, F>(self, mut accum: Acc, mut f: F) -> Acc
129151
where
130152
F: FnMut(Acc, Self::Item) -> Acc,
131153
{
132154
let (front, back) = RingSlices::ring_slices(self.ring, self.head, self.tail);
133-
accum = back.iter().rfold(accum, &mut f);
134-
front.iter().rfold(accum, &mut f)
155+
// Safety:
156+
// - `self.head` and `self.tail` in a ring buffer are always valid indices.
157+
// - `RingSlices::ring_slices` guarantees that the slices split according to `self.head` and `self.tail` are initialized.
158+
unsafe {
159+
accum = MaybeUninit::slice_assume_init_ref(back).iter().rfold(accum, &mut f);
160+
MaybeUninit::slice_assume_init_ref(front).iter().rfold(accum, &mut f)
161+
}
135162
}
136163

137164
fn try_rfold<B, F, R>(&mut self, init: B, mut f: F) -> R
@@ -142,16 +169,20 @@ impl<'a, T> DoubleEndedIterator for Iter<'a, T> {
142169
{
143170
let (mut iter, final_res);
144171
if self.tail <= self.head {
145-
// single slice self.ring[self.tail..self.head]
146-
iter = self.ring[self.tail..self.head].iter();
172+
// Safety: single slice self.ring[self.tail..self.head] is initialized.
173+
iter = unsafe {
174+
MaybeUninit::slice_assume_init_ref(&self.ring[self.tail..self.head]).iter()
175+
};
147176
final_res = iter.try_rfold(init, &mut f);
148177
} else {
149-
// two slices: self.ring[self.tail..], self.ring[..self.head]
178+
// Safety: two slices: self.ring[self.tail..], self.ring[..self.head] both are initialized.
150179
let (front, back) = self.ring.split_at(self.tail);
151-
let mut front_iter = front[..self.head].iter();
180+
181+
let mut front_iter =
182+
unsafe { MaybeUninit::slice_assume_init_ref(&front[..self.head]).iter() };
152183
let res = front_iter.try_rfold(init, &mut f);
153184
self.head = front_iter.len();
154-
iter = back.iter();
185+
iter = unsafe { MaybeUninit::slice_assume_init_ref(back).iter() };
155186
final_res = iter.try_rfold(res?, &mut f);
156187
}
157188
self.head = self.tail + iter.len();

library/alloc/src/collections/vec_deque/mod.rs

+45-11
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use core::fmt;
1212
use core::hash::{Hash, Hasher};
1313
use core::iter::{repeat_with, FromIterator};
1414
use core::marker::PhantomData;
15-
use core::mem::{self, ManuallyDrop};
15+
use core::mem::{self, ManuallyDrop, MaybeUninit};
1616
use core::ops::{Index, IndexMut, Range, RangeBounds};
1717
use core::ptr::{self, NonNull};
1818
use core::slice;
@@ -181,16 +181,28 @@ impl<T, A: Allocator> VecDeque<T, A> {
181181
}
182182
}
183183

184-
/// Turn ptr into a slice
184+
/// Turn ptr into a slice, since the elements of the backing buffer may be uninitialized,
185+
/// we will return a slice of [`MaybeUninit<T>`].
186+
///
187+
/// See [`MaybeUninit::zeroed`][zeroed] for examples of correct and
188+
/// incorrect usage of this method.
189+
///
190+
/// [zeroed]: mem::MaybeUninit::zeroed
185191
#[inline]
186-
unsafe fn buffer_as_slice(&self) -> &[T] {
187-
unsafe { slice::from_raw_parts(self.ptr(), self.cap()) }
192+
unsafe fn buffer_as_slice(&self) -> &[MaybeUninit<T>] {
193+
unsafe { slice::from_raw_parts(self.ptr() as *mut MaybeUninit<T>, self.cap()) }
188194
}
189195

190-
/// Turn ptr into a mut slice
196+
/// Turn ptr into a mut slice, since the elements of the backing buffer may be uninitialized,
197+
/// we will return a slice of [`MaybeUninit<T>`].
198+
///
199+
/// See [`MaybeUninit::zeroed`][zeroed] for examples of correct and
200+
/// incorrect usage of this method.
201+
///
202+
/// [zeroed]: mem::MaybeUninit::zeroed
191203
#[inline]
192-
unsafe fn buffer_as_mut_slice(&mut self) -> &mut [T] {
193-
unsafe { slice::from_raw_parts_mut(self.ptr(), self.cap()) }
204+
unsafe fn buffer_as_mut_slice(&mut self) -> &mut [MaybeUninit<T>] {
205+
unsafe { slice::from_raw_parts_mut(self.ptr() as *mut MaybeUninit<T>, self.cap()) }
194206
}
195207

196208
/// Moves an element out of the buffer
@@ -1055,9 +1067,13 @@ impl<T, A: Allocator> VecDeque<T, A> {
10551067
#[inline]
10561068
#[stable(feature = "deque_extras_15", since = "1.5.0")]
10571069
pub fn as_slices(&self) -> (&[T], &[T]) {
1070+
// Safety:
1071+
// - `self.head` and `self.tail` in a ring buffer are always valid indices.
1072+
// - `RingSlices::ring_slices` guarantees that the slices split according to `self.head` and `self.tail` are initialized.
10581073
unsafe {
10591074
let buf = self.buffer_as_slice();
1060-
RingSlices::ring_slices(buf, self.head, self.tail)
1075+
let (front, back) = RingSlices::ring_slices(buf, self.head, self.tail);
1076+
(MaybeUninit::slice_assume_init_ref(front), MaybeUninit::slice_assume_init_ref(back))
10611077
}
10621078
}
10631079

@@ -1089,11 +1105,15 @@ impl<T, A: Allocator> VecDeque<T, A> {
10891105
#[inline]
10901106
#[stable(feature = "deque_extras_15", since = "1.5.0")]
10911107
pub fn as_mut_slices(&mut self) -> (&mut [T], &mut [T]) {
1108+
// Safety:
1109+
// - `self.head` and `self.tail` in a ring buffer are always valid indices.
1110+
// - `RingSlices::ring_slices` guarantees that the slices split according to `self.head` and `self.tail` are initialized.
10921111
unsafe {
10931112
let head = self.head;
10941113
let tail = self.tail;
10951114
let buf = self.buffer_as_mut_slice();
1096-
RingSlices::ring_slices(buf, head, tail)
1115+
let (front, back) = RingSlices::ring_slices(buf, head, tail);
1116+
(MaybeUninit::slice_assume_init_mut(front), MaybeUninit::slice_assume_init_mut(back))
10971117
}
10981118
}
10991119

@@ -2327,7 +2347,14 @@ impl<T, A: Allocator> VecDeque<T, A> {
23272347
if self.is_contiguous() {
23282348
let tail = self.tail;
23292349
let head = self.head;
2330-
return unsafe { RingSlices::ring_slices(self.buffer_as_mut_slice(), head, tail).0 };
2350+
// Safety:
2351+
// - `self.head` and `self.tail` in a ring buffer are always valid indices.
2352+
// - `RingSlices::ring_slices` guarantees that the slices split according to `self.head` and `self.tail` are initialized.
2353+
return unsafe {
2354+
MaybeUninit::slice_assume_init_mut(
2355+
RingSlices::ring_slices(self.buffer_as_mut_slice(), head, tail).0,
2356+
)
2357+
};
23312358
}
23322359

23332360
let buf = self.buf.ptr();
@@ -2413,7 +2440,14 @@ impl<T, A: Allocator> VecDeque<T, A> {
24132440

24142441
let tail = self.tail;
24152442
let head = self.head;
2416-
unsafe { RingSlices::ring_slices(self.buffer_as_mut_slice(), head, tail).0 }
2443+
// Safety:
2444+
// - `self.head` and `self.tail` in a ring buffer are always valid indices.
2445+
// - `RingSlices::ring_slices` guarantees that the slices split according to `self.head` and `self.tail` are initialized.
2446+
unsafe {
2447+
MaybeUninit::slice_assume_init_mut(
2448+
RingSlices::ring_slices(self.buffer_as_mut_slice(), head, tail).0,
2449+
)
2450+
}
24172451
}
24182452

24192453
/// Rotates the double-ended queue `mid` places to the left.

0 commit comments

Comments
 (0)