Skip to content

Commit a4b629e

Browse files
committed
Revert "Move to unchecked_sub as suggested by @the8472"
This reverts commit 1dfff2e.
1 parent 692788a commit a4b629e

File tree

6 files changed

+47
-96
lines changed

6 files changed

+47
-96
lines changed

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -756,8 +756,8 @@ impl<T, A: Allocator> VecDeque<T, A> {
756756
let old_cap = self.capacity();
757757

758758
if new_cap > old_cap {
759+
self.buf.reserve_exact(self.len, additional);
759760
unsafe {
760-
self.buf.reserve_exact(self.len, additional);
761761
self.handle_capacity_increase(old_cap);
762762
}
763763
}
@@ -787,8 +787,8 @@ impl<T, A: Allocator> VecDeque<T, A> {
787787
if new_cap > old_cap {
788788
// we don't need to reserve_exact(), as the size doesn't have
789789
// to be a power of 2.
790+
self.buf.reserve(self.len, additional);
790791
unsafe {
791-
self.buf.reserve(self.len, additional);
792792
self.handle_capacity_increase(old_cap);
793793
}
794794
}
@@ -838,8 +838,8 @@ impl<T, A: Allocator> VecDeque<T, A> {
838838
let old_cap = self.capacity();
839839

840840
if new_cap > old_cap {
841+
self.buf.try_reserve_exact(self.len, additional)?;
841842
unsafe {
842-
self.buf.try_reserve_exact(self.len, additional)?;
843843
self.handle_capacity_increase(old_cap);
844844
}
845845
}
@@ -886,8 +886,8 @@ impl<T, A: Allocator> VecDeque<T, A> {
886886
let old_cap = self.capacity();
887887

888888
if new_cap > old_cap {
889+
self.buf.try_reserve(self.len, additional)?;
889890
unsafe {
890-
self.buf.try_reserve(self.len, additional)?;
891891
self.handle_capacity_increase(old_cap);
892892
}
893893
}

library/alloc/src/raw_vec.rs

+18-32
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,10 @@ impl<T, A: Allocator> RawVec<T, A> {
276276
/// *O*(1) behavior. Will limit this behavior if it would needlessly cause
277277
/// itself to panic.
278278
///
279+
/// If `len` exceeds `self.capacity()`, this may fail to actually allocate
280+
/// the requested space. This is not really unsafe, but the unsafe
281+
/// code *you* write that relies on the behavior of this function may break.
282+
///
279283
/// This is ideal for implementing a bulk-push operation like `extend`.
280284
///
281285
/// # Panics
@@ -285,13 +289,9 @@ impl<T, A: Allocator> RawVec<T, A> {
285289
/// # Aborts
286290
///
287291
/// Aborts on OOM.
288-
///
289-
/// # Safety
290-
///
291-
/// `len` must be less than or equal to the capacity of this [`RawVec`].
292292
#[cfg(not(no_global_oom_handling))]
293293
#[inline]
294-
pub unsafe fn reserve(&mut self, len: usize, additional: usize) {
294+
pub fn reserve(&mut self, len: usize, additional: usize) {
295295
// Callers expect this function to be very cheap when there is already sufficient capacity.
296296
// Therefore, we move all the resizing and error-handling logic from grow_amortized and
297297
// handle_reserve behind a call, while making sure that this function is likely to be
@@ -305,7 +305,7 @@ impl<T, A: Allocator> RawVec<T, A> {
305305
handle_reserve(slf.grow_amortized(len, additional));
306306
}
307307

308-
if unsafe { self.needs_to_grow(len, additional) } {
308+
if self.needs_to_grow(len, additional) {
309309
do_reserve_and_handle(self, len, additional);
310310
}
311311
unsafe {
@@ -323,16 +323,8 @@ impl<T, A: Allocator> RawVec<T, A> {
323323
}
324324

325325
/// The same as `reserve`, but returns on errors instead of panicking or aborting.
326-
///
327-
/// # Safety
328-
///
329-
/// `len` must be less than or equal to the capacity of this [`RawVec`].
330-
pub unsafe fn try_reserve(
331-
&mut self,
332-
len: usize,
333-
additional: usize,
334-
) -> Result<(), TryReserveError> {
335-
if unsafe { self.needs_to_grow(len, additional) } {
326+
pub fn try_reserve(&mut self, len: usize, additional: usize) -> Result<(), TryReserveError> {
327+
if self.needs_to_grow(len, additional) {
336328
self.grow_amortized(len, additional)?;
337329
}
338330
unsafe {
@@ -348,35 +340,29 @@ impl<T, A: Allocator> RawVec<T, A> {
348340
/// exactly the amount of memory necessary, but in principle the allocator
349341
/// is free to give back more than we asked for.
350342
///
343+
/// If `len` exceeds `self.capacity()`, this may fail to actually allocate
344+
/// the requested space. This is not really unsafe, but the unsafe code
345+
/// *you* write that relies on the behavior of this function may break.
346+
///
351347
/// # Panics
352348
///
353349
/// Panics if the new capacity exceeds `isize::MAX` _bytes_.
354350
///
355351
/// # Aborts
356352
///
357353
/// Aborts on OOM.
358-
///
359-
/// # Safety
360-
///
361-
/// `len` must be less than or equal to the capacity of this [`RawVec`].
362354
#[cfg(not(no_global_oom_handling))]
363-
pub unsafe fn reserve_exact(&mut self, len: usize, additional: usize) {
364-
unsafe {
365-
handle_reserve(self.try_reserve_exact(len, additional));
366-
}
355+
pub fn reserve_exact(&mut self, len: usize, additional: usize) {
356+
handle_reserve(self.try_reserve_exact(len, additional));
367357
}
368358

369359
/// The same as `reserve_exact`, but returns on errors instead of panicking or aborting.
370-
///
371-
/// # Safety
372-
///
373-
/// `len` must be less than or equal to the capacity of this [`RawVec`].
374-
pub unsafe fn try_reserve_exact(
360+
pub fn try_reserve_exact(
375361
&mut self,
376362
len: usize,
377363
additional: usize,
378364
) -> Result<(), TryReserveError> {
379-
if unsafe { self.needs_to_grow(len, additional) } {
365+
if self.needs_to_grow(len, additional) {
380366
self.grow_exact(len, additional)?;
381367
}
382368
unsafe {
@@ -405,8 +391,8 @@ impl<T, A: Allocator> RawVec<T, A> {
405391
impl<T, A: Allocator> RawVec<T, A> {
406392
/// Returns if the buffer needs to grow to fulfill the needed extra capacity.
407393
/// Mainly used to make inlining reserve-calls possible without inlining `grow`.
408-
pub(crate) unsafe fn needs_to_grow(&self, len: usize, additional: usize) -> bool {
409-
unsafe { additional > self.capacity().unchecked_sub(len) }
394+
pub(crate) fn needs_to_grow(&self, len: usize, additional: usize) -> bool {
395+
additional > self.capacity().wrapping_sub(len)
410396
}
411397

412398
/// # Safety:

library/alloc/src/raw_vec/tests.rs

+16-37
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,7 @@ fn allocator_param() {
4343
let a = BoundedAlloc { fuel: Cell::new(500) };
4444
let mut v: RawVec<u8, _> = RawVec::with_capacity_in(50, a);
4545
assert_eq!(v.alloc.fuel.get(), 450);
46-
unsafe {
47-
// (causes a realloc, thus using 50 + 150 = 200 units of fuel)
48-
v.reserve(50, 150);
49-
}
46+
v.reserve(50, 150); // (causes a realloc, thus using 50 + 150 = 200 units of fuel)
5047
assert_eq!(v.alloc.fuel.get(), 250);
5148
}
5249

@@ -55,35 +52,25 @@ fn reserve_does_not_overallocate() {
5552
{
5653
let mut v: RawVec<u32> = RawVec::new();
5754
// First, `reserve` allocates like `reserve_exact`.
58-
unsafe {
59-
v.reserve(0, 9);
60-
}
55+
v.reserve(0, 9);
6156
assert_eq!(9, v.capacity());
6257
}
6358

6459
{
6560
let mut v: RawVec<u32> = RawVec::new();
66-
unsafe {
67-
v.reserve(0, 7);
68-
}
61+
v.reserve(0, 7);
6962
assert_eq!(7, v.capacity());
7063
// 97 is more than double of 7, so `reserve` should work
7164
// like `reserve_exact`.
72-
unsafe {
73-
v.reserve(7, 90);
74-
}
65+
v.reserve(7, 90);
7566
assert_eq!(97, v.capacity());
7667
}
7768

7869
{
7970
let mut v: RawVec<u32> = RawVec::new();
80-
unsafe {
81-
v.reserve(0, 12);
82-
}
71+
v.reserve(0, 12);
8372
assert_eq!(12, v.capacity());
84-
unsafe {
85-
v.reserve(12, 3);
86-
}
73+
v.reserve(12, 3);
8774
// 3 is less than half of 12, so `reserve` must grow
8875
// exponentially. At the time of writing this test grow
8976
// factor is 2, so new capacity is 24, however, grow factor
@@ -129,28 +116,24 @@ fn zst() {
129116

130117
// Check all these operations work as expected with zero-sized elements.
131118

132-
assert!(unsafe { !v.needs_to_grow(100, usize::MAX - 100) });
133-
assert!(unsafe { v.needs_to_grow(101, usize::MAX - 100) });
119+
assert!(!v.needs_to_grow(100, usize::MAX - 100));
120+
assert!(v.needs_to_grow(101, usize::MAX - 100));
134121
zst_sanity(&v);
135122

136-
unsafe {
137-
v.reserve(100, usize::MAX - 100);
138-
}
123+
v.reserve(100, usize::MAX - 100);
139124
//v.reserve(101, usize::MAX - 100); // panics, in `zst_reserve_panic` below
140125
zst_sanity(&v);
141126

142-
unsafe {
143-
v.reserve_exact(100, usize::MAX - 100);
144-
}
127+
v.reserve_exact(100, usize::MAX - 100);
145128
//v.reserve_exact(101, usize::MAX - 100); // panics, in `zst_reserve_exact_panic` below
146129
zst_sanity(&v);
147130

148-
assert_eq!(unsafe { v.try_reserve(100, usize::MAX - 100) }, Ok(()));
149-
assert_eq!(unsafe { v.try_reserve(101, usize::MAX - 100) }, cap_err);
131+
assert_eq!(v.try_reserve(100, usize::MAX - 100), Ok(()));
132+
assert_eq!(v.try_reserve(101, usize::MAX - 100), cap_err);
150133
zst_sanity(&v);
151134

152-
assert_eq!(unsafe { v.try_reserve_exact(100, usize::MAX - 100) }, Ok(()));
153-
assert_eq!(unsafe { v.try_reserve_exact(101, usize::MAX - 100) }, cap_err);
135+
assert_eq!(v.try_reserve_exact(100, usize::MAX - 100), Ok(()));
136+
assert_eq!(v.try_reserve_exact(101, usize::MAX - 100), cap_err);
154137
zst_sanity(&v);
155138

156139
assert_eq!(v.grow_amortized(100, usize::MAX - 100), cap_err);
@@ -168,9 +151,7 @@ fn zst_reserve_panic() {
168151
let mut v: RawVec<ZST> = RawVec::new();
169152
zst_sanity(&v);
170153

171-
unsafe {
172-
v.reserve(101, usize::MAX - 100);
173-
}
154+
v.reserve(101, usize::MAX - 100);
174155
}
175156

176157
#[test]
@@ -179,9 +160,7 @@ fn zst_reserve_exact_panic() {
179160
let mut v: RawVec<ZST> = RawVec::new();
180161
zst_sanity(&v);
181162

182-
unsafe {
183-
v.reserve_exact(101, usize::MAX - 100);
184-
}
163+
v.reserve_exact(101, usize::MAX - 100);
185164
}
186165

187166
#[test]

library/alloc/src/string.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -1152,7 +1152,7 @@ impl String {
11521152
self.vec.reserve(additional);
11531153
unsafe {
11541154
// Inform the optimizer that the reservation has succeeded or wasn't needed
1155-
intrinsics::assume(additional <= self.capacity().unchecked_sub(self.len()));
1155+
intrinsics::assume(additional <= self.capacity().wrapping_sub(self.len()));
11561156
}
11571157
}
11581158

@@ -1206,7 +1206,7 @@ impl String {
12061206
self.vec.reserve_exact(additional);
12071207
unsafe {
12081208
// Inform the optimizer that the reservation has succeeded or wasn't needed
1209-
intrinsics::assume(additional <= self.capacity().unchecked_sub(self.len()));
1209+
intrinsics::assume(additional <= self.capacity().wrapping_sub(self.len()));
12101210
}
12111211
}
12121212

@@ -1246,7 +1246,7 @@ impl String {
12461246
self.vec.try_reserve(additional)?;
12471247
unsafe {
12481248
// Inform the optimizer that the reservation has succeeded or wasn't needed
1249-
intrinsics::assume(additional <= self.capacity().unchecked_sub(self.len()));
1249+
intrinsics::assume(additional <= self.capacity().wrapping_sub(self.len()));
12501250
}
12511251
Ok(())
12521252
}
@@ -1293,7 +1293,7 @@ impl String {
12931293
self.vec.try_reserve_exact(additional)?;
12941294
unsafe {
12951295
// Inform the optimizer that the reservation has succeeded or wasn't needed
1296-
intrinsics::assume(additional <= self.capacity().unchecked_sub(self.len()));
1296+
intrinsics::assume(additional <= self.capacity().wrapping_sub(self.len()));
12971297
}
12981298
Ok(())
12991299
}

library/alloc/src/vec/mod.rs

+4-16
Original file line numberDiff line numberDiff line change
@@ -909,10 +909,7 @@ impl<T, A: Allocator> Vec<T, A> {
909909
#[stable(feature = "rust1", since = "1.0.0")]
910910
#[inline]
911911
pub fn reserve(&mut self, additional: usize) {
912-
// SAFETY: len <= capacity
913-
unsafe {
914-
self.buf.reserve(self.len, additional);
915-
}
912+
self.buf.reserve(self.len, additional);
916913
unsafe {
917914
// Inform the optimizer that the reservation has succeeded or wasn't needed
918915
intrinsics::assume(!self.buf.needs_to_grow(self.len, additional));
@@ -947,10 +944,7 @@ impl<T, A: Allocator> Vec<T, A> {
947944
#[stable(feature = "rust1", since = "1.0.0")]
948945
#[inline]
949946
pub fn reserve_exact(&mut self, additional: usize) {
950-
// SAFETY: len <= capacity
951-
unsafe {
952-
self.buf.reserve_exact(self.len, additional);
953-
}
947+
self.buf.reserve_exact(self.len, additional);
954948
unsafe {
955949
// Inform the optimizer that the reservation has succeeded or wasn't needed
956950
intrinsics::assume(!self.buf.needs_to_grow(self.len, additional));
@@ -992,10 +986,7 @@ impl<T, A: Allocator> Vec<T, A> {
992986
#[stable(feature = "try_reserve", since = "1.57.0")]
993987
#[inline]
994988
pub fn try_reserve(&mut self, additional: usize) -> Result<(), TryReserveError> {
995-
// SAFETY: len <= capacity
996-
unsafe {
997-
self.buf.try_reserve(self.len, additional)?;
998-
}
989+
self.buf.try_reserve(self.len, additional)?;
999990
unsafe {
1000991
// Inform the optimizer that the reservation has succeeded or wasn't needed
1001992
intrinsics::assume(!self.buf.needs_to_grow(self.len, additional));
@@ -1044,10 +1035,7 @@ impl<T, A: Allocator> Vec<T, A> {
10441035
#[stable(feature = "try_reserve", since = "1.57.0")]
10451036
#[inline]
10461037
pub fn try_reserve_exact(&mut self, additional: usize) -> Result<(), TryReserveError> {
1047-
// SAFETY: len <= capacity
1048-
unsafe {
1049-
self.buf.try_reserve_exact(self.len, additional)?;
1050-
}
1038+
self.buf.try_reserve_exact(self.len, additional)?;
10511039
unsafe {
10521040
// Inform the optimizer that the reservation has succeeded or wasn't needed
10531041
intrinsics::assume(!self.buf.needs_to_grow(self.len, additional));

library/alloc/src/vec/splice.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,7 @@ impl<T, A: Allocator> Drain<'_, T, A> {
126126
unsafe fn move_tail(&mut self, additional: usize) {
127127
let vec = unsafe { self.vec.as_mut() };
128128
let len = self.tail_start + self.tail_len;
129-
unsafe {
130-
vec.buf.reserve(len, additional);
131-
}
129+
vec.buf.reserve(len, additional);
132130

133131
let new_tail_start = self.tail_start + additional;
134132
unsafe {

0 commit comments

Comments
 (0)