Skip to content

Commit 0dc2769

Browse files
TimDiekmannTimDiekmann
committed
Fix regression for Rc and Arc
For this, the generic parameter in`PrefixAllocator::prefix` was removed and `(A)Rc::new_uninit` isn't called in `new` anymore as this doubles the code generation for `(A)Rc`
1 parent 251d44f commit 0dc2769

File tree

4 files changed

+84
-56
lines changed

4 files changed

+84
-56
lines changed

‎library/alloc/src/raw_vec.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
#![unstable(feature = "raw_vec_internals", reason = "implementation detail", issue = "none")]
22
#![doc(hidden)]
33

4-
use core::alloc::{helper::AllocInit, LayoutError};
4+
#[cfg(not(no_global_oom_handling))]
5+
use core::alloc::helper::AllocInit;
6+
use core::alloc::LayoutError;
57
use core::cmp;
68
use core::intrinsics;
79
use core::mem::{self, ManuallyDrop, MaybeUninit};

‎library/alloc/src/rc.rs

Lines changed: 41 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ use core::intrinsics::abort;
259259
#[cfg(not(no_global_oom_handling))]
260260
use core::iter;
261261
use core::marker::{self, PhantomData, Unpin, Unsize};
262-
use core::mem::{self, forget};
262+
use core::mem;
263263
use core::ops::{CoerceUnsized, Deref, DispatchFromDyn, Receiver};
264264
use core::pin::Pin;
265265
use core::ptr::{self, NonNull};
@@ -389,8 +389,19 @@ impl<T: ?Sized + Unsize<U>, U: ?Sized> DispatchFromDyn<Rc<U>> for Rc<T> {}
389389

390390
impl<T: ?Sized> Rc<T> {
391391
#[inline]
392+
fn metadata_ptr(ptr: NonNull<T>) -> NonNull<RcMetadata> {
393+
// SAFETY: since the only unsized types possible are slices, trait objects,
394+
// and extern types, the input safety requirement is currently enough to
395+
// satisfy the requirements of for_value_raw; this is an implementation
396+
// detail of the language that may not be relied upon outside of std.
397+
let align = unsafe { mem::align_of_val_raw(ptr.as_ptr()) };
398+
399+
unsafe { RcAllocator::<Global>::prefix(ptr.cast(), align) }
400+
}
401+
402+
#[inline(always)]
392403
fn metadata(&self) -> &RcMetadata {
393-
unsafe { RcAllocator::<Global>::prefix(self.ptr).as_ref() }
404+
unsafe { Self::metadata_ptr(self.ptr).as_ref() }
394405
}
395406
}
396407

@@ -404,13 +415,21 @@ impl<T> Rc<T> {
404415
///
405416
/// let five = Rc::new(5);
406417
/// ```
407-
#[inline]
418+
#[cfg(not(no_global_oom_handling))]
408419
#[stable(feature = "rust1", since = "1.0.0")]
409420
pub fn new(value: T) -> Rc<T> {
410-
let mut rc = Self::new_uninit();
421+
let alloc = RcAllocator::new(Global);
422+
let layout = Layout::new::<T>();
423+
let ptr = Self::allocate(
424+
&alloc,
425+
layout,
426+
RcMetadata::new_strong(),
427+
AllocInit::Uninitialized,
428+
NonNull::cast,
429+
);
411430
unsafe {
412-
Rc::get_mut_unchecked(&mut rc).as_mut_ptr().write(value);
413-
rc.assume_init()
431+
ptr.as_ptr().write(value);
432+
Self::from_raw_in(ptr.as_ptr().cast(), alloc)
414433
}
415434
}
416435

@@ -439,6 +458,7 @@ impl<T> Rc<T> {
439458
/// }
440459
/// ```
441460
#[inline]
461+
#[cfg(not(no_global_oom_handling))]
442462
#[unstable(feature = "arc_new_cyclic", issue = "75861")]
443463
pub fn new_cyclic(data_fn: impl FnOnce(&Weak<T>) -> T) -> Rc<T> {
444464
// Construct the inner in the "uninitialized" state with a single
@@ -466,7 +486,7 @@ impl<T> Rc<T> {
466486
ptr.as_ptr().write(data_fn(&weak));
467487
}
468488

469-
let meta = unsafe { RcAllocator::<Global>::prefix(ptr).as_ref() };
489+
let meta = unsafe { Self::metadata_ptr(ptr).as_ref() };
470490
debug_assert_eq!(meta.strong.get(), 0, "No prior strong references should exist");
471491
meta.strong.set(1);
472492

@@ -1295,9 +1315,7 @@ impl<T: ?Sized> Rc<T> {
12951315
mem_to_ptr: impl FnOnce(NonNull<u8>) -> NonNull<T>,
12961316
) -> NonNull<T> {
12971317
let ptr = mem_to_ptr(allocate(alloc, layout, init));
1298-
unsafe {
1299-
RcAllocator::<Global>::prefix(ptr).as_ptr().write(meta);
1300-
}
1318+
unsafe { Self::metadata_ptr(ptr).as_ptr().write(meta) }
13011319
ptr
13021320
}
13031321

@@ -1316,9 +1334,7 @@ impl<T: ?Sized> Rc<T> {
13161334
mem_to_ptr: impl FnOnce(NonNull<u8>) -> NonNull<T>,
13171335
) -> Result<NonNull<T>, AllocError> {
13181336
let ptr = mem_to_ptr(try_allocate(alloc, layout, init)?);
1319-
unsafe {
1320-
RcAllocator::<Global>::prefix(ptr).as_ptr().write(meta);
1321-
}
1337+
unsafe { Self::metadata_ptr(ptr).as_ptr().write(meta) }
13221338
Ok(ptr)
13231339
}
13241340

@@ -1419,7 +1435,7 @@ impl<T> Rc<[T]> {
14191435
}
14201436

14211437
// All clear. Forget the guard so it doesn't free the new RcBox.
1422-
forget(guard);
1438+
mem::forget(guard);
14231439

14241440
Self::from_raw_in(ptr.as_ptr(), alloc)
14251441
}
@@ -1488,17 +1504,22 @@ unsafe impl<#[may_dangle] T: ?Sized> Drop for Rc<T> {
14881504
/// drop(foo2); // Prints "dropped!"
14891505
/// ```
14901506
fn drop(&mut self) {
1491-
self.metadata().dec_strong();
1492-
if self.metadata().strong() == 0 {
1507+
let metadata = self.metadata();
1508+
1509+
metadata.dec_strong();
1510+
if metadata.strong() == 0 {
14931511
// destroy the contained object
14941512
unsafe {
14951513
ptr::drop_in_place(Self::get_mut_unchecked(self));
14961514
}
14971515

1516+
// Due to the borrow checker, we have to read the metadata again
1517+
let metadata = self.metadata();
1518+
14981519
// remove the implicit "strong weak" pointer now that we've
14991520
// destroyed the contents.
1500-
self.metadata().dec_weak();
1501-
if self.metadata().weak() == 0 {
1521+
metadata.dec_weak();
1522+
if metadata.weak() == 0 {
15021523
unsafe {
15031524
let layout = Layout::for_value_raw(self.ptr.as_ptr());
15041525
self.alloc.deallocate(self.ptr.cast(), layout);
@@ -2071,11 +2092,7 @@ impl<T: ?Sized + Unsize<U>, U: ?Sized> DispatchFromDyn<Weak<U>> for Weak<T> {}
20712092
impl<T: ?Sized> Weak<T> {
20722093
#[inline]
20732094
fn metadata(&self) -> Option<&RcMetadata> {
2074-
if is_dangling(self.ptr.as_ptr()) {
2075-
None
2076-
} else {
2077-
Some(unsafe { RcAllocator::<Global>::prefix(self.ptr).as_ref() })
2078-
}
2095+
(!is_dangling(self.ptr.as_ptr())).then(|| unsafe { Rc::metadata_ptr(self.ptr).as_ref() })
20792096
}
20802097
}
20812098

@@ -2426,6 +2443,7 @@ impl<T> Default for Weak<T> {
24262443

24272444
/// Dediated function for allocating to prevent generating a function for every `T`
24282445
#[inline]
2446+
#[cfg(not(no_global_oom_handling))]
24292447
fn allocate(alloc: &RcAllocator<Global>, layout: Layout, init: AllocInit) -> NonNull<u8> {
24302448
try_allocate(alloc, layout, init).unwrap_or_else(|_| handle_alloc_error(layout))
24312449
}

‎library/alloc/src/sync.rs

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use core::intrinsics::abort;
1616
#[cfg(not(no_global_oom_handling))]
1717
use core::iter;
1818
use core::marker::{PhantomData, Unpin, Unsize};
19-
use core::mem::{self, forget};
19+
use core::mem;
2020
use core::ops::{CoerceUnsized, Deref, DispatchFromDyn, Receiver};
2121
use core::pin::Pin;
2222
use core::ptr::{self, NonNull};
@@ -31,6 +31,7 @@ use crate::alloc::handle_alloc_error;
3131
use crate::alloc::{box_free, WriteCloneIntoRaw};
3232
use crate::alloc::{AllocError, Allocator, Global, Layout};
3333
use crate::borrow::{Cow, ToOwned};
34+
#[cfg(not(no_global_oom_handling))]
3435
use crate::boxed::Box;
3536
use crate::rc::is_dangling;
3637
#[cfg(not(no_global_oom_handling))]
@@ -278,8 +279,19 @@ impl<T: ?Sized + Unsize<U>, U: ?Sized> DispatchFromDyn<Arc<U>> for Arc<T> {}
278279

279280
impl<T: ?Sized> Arc<T> {
280281
#[inline]
282+
fn metadata_ptr(ptr: NonNull<T>) -> NonNull<ArcMetadata> {
283+
// SAFETY: since the only unsized types possible are slices, trait objects,
284+
// and extern types, the input safety requirement is currently enough to
285+
// satisfy the requirements of for_value_raw; this is an implementation
286+
// detail of the language that may not be relied upon outside of std.
287+
let align = unsafe { mem::align_of_val_raw(ptr.as_ptr()) };
288+
289+
unsafe { ArcAllocator::<Global>::prefix(ptr.cast(), align) }
290+
}
291+
292+
#[inline(always)]
281293
fn metadata(&self) -> &ArcMetadata {
282-
unsafe { ArcAllocator::<Global>::prefix(self.ptr).as_ref() }
294+
unsafe { Self::metadata_ptr(self.ptr).as_ref() }
283295
}
284296
}
285297

@@ -335,11 +347,7 @@ impl<T: ?Sized + fmt::Debug> fmt::Debug for Weak<T> {
335347
impl<T: ?Sized> Weak<T> {
336348
#[inline]
337349
fn metadata(&self) -> Option<&ArcMetadata> {
338-
if is_dangling(self.ptr.as_ptr()) {
339-
None
340-
} else {
341-
Some(unsafe { ArcAllocator::<Global>::prefix(self.ptr).as_ref() })
342-
}
350+
(!is_dangling(self.ptr.as_ptr())).then(|| unsafe { Arc::metadata_ptr(self.ptr).as_ref() })
343351
}
344352
}
345353

@@ -354,12 +362,21 @@ impl<T> Arc<T> {
354362
/// let five = Arc::new(5);
355363
/// ```
356364
#[inline]
365+
#[cfg(not(no_global_oom_handling))]
357366
#[stable(feature = "rust1", since = "1.0.0")]
358-
pub fn new(data: T) -> Self {
359-
let mut arc = Self::new_uninit();
367+
pub fn new(value: T) -> Self {
368+
let alloc = ArcAllocator::new(Global);
369+
let layout = Layout::new::<T>();
370+
let ptr = Self::allocate(
371+
&alloc,
372+
layout,
373+
ArcMetadata::new_strong(),
374+
AllocInit::Uninitialized,
375+
NonNull::cast,
376+
);
360377
unsafe {
361-
Arc::get_mut_unchecked(&mut arc).as_mut_ptr().write(data);
362-
arc.assume_init()
378+
ptr.as_ptr().write(value);
379+
Self::from_raw_in(ptr.as_ptr().cast(), alloc)
363380
}
364381
}
365382

@@ -384,6 +401,7 @@ impl<T> Arc<T> {
384401
/// });
385402
/// ```
386403
#[inline]
404+
#[cfg(not(no_global_oom_handling))]
387405
#[unstable(feature = "arc_new_cyclic", issue = "75861")]
388406
pub fn new_cyclic(data_fn: impl FnOnce(&Weak<T>) -> T) -> Self {
389407
// Construct the inner in the "uninitialized" state with a single
@@ -411,7 +429,8 @@ impl<T> Arc<T> {
411429
ptr.as_ptr().write(data_fn(&weak));
412430
}
413431

414-
let meta = unsafe { ArcAllocator::<Global>::prefix(ptr).as_ref() };
432+
// SAFETY: `ptr` was just allocated with the allocator with the alignment of `T`
433+
let meta = unsafe { Self::metadata_ptr(ptr).as_ref() };
415434
let prev_value = meta.strong.fetch_add(1, Release);
416435
debug_assert_eq!(prev_value, 0, "No prior strong references should exist");
417436

@@ -1097,6 +1116,7 @@ impl<T: ?Sized> Arc<T> {
10971116
/// The function `mem_to_rcbox` is called with the data pointer
10981117
/// and must return back a (potentially fat)-pointer for the `RcBox<T>`.
10991118
#[inline]
1119+
#[cfg(not(no_global_oom_handling))]
11001120
fn allocate(
11011121
alloc: &ArcAllocator<Global>,
11021122
layout: Layout,
@@ -1105,9 +1125,7 @@ impl<T: ?Sized> Arc<T> {
11051125
mem_to_ptr: impl FnOnce(NonNull<u8>) -> NonNull<T>,
11061126
) -> NonNull<T> {
11071127
let ptr = mem_to_ptr(allocate(alloc, layout, init));
1108-
unsafe {
1109-
ArcAllocator::<Global>::prefix(ptr).as_ptr().write(meta);
1110-
}
1128+
unsafe { Self::metadata_ptr(ptr).as_ptr().write(meta) }
11111129
ptr
11121130
}
11131131

@@ -1125,9 +1143,7 @@ impl<T: ?Sized> Arc<T> {
11251143
mem_to_ptr: impl FnOnce(NonNull<u8>) -> NonNull<T>,
11261144
) -> Result<NonNull<T>, AllocError> {
11271145
let ptr = mem_to_ptr(try_allocate(alloc, layout, init)?);
1128-
unsafe {
1129-
ArcAllocator::<Global>::prefix(ptr).as_ptr().write(meta);
1130-
}
1146+
unsafe { Self::metadata_ptr(ptr).as_ptr().write(meta) }
11311147
Ok(ptr)
11321148
}
11331149

@@ -1228,7 +1244,7 @@ impl<T> Arc<[T]> {
12281244
}
12291245

12301246
// All clear. Forget the guard so it doesn't free the new RcBox.
1231-
forget(guard);
1247+
mem::forget(guard);
12321248

12331249
Self::from_raw_in(ptr.as_ptr(), alloc)
12341250
}
@@ -2473,6 +2489,7 @@ impl<T, I: iter::TrustedLen<Item = T>> ToArcSlice<T> for I {
24732489

24742490
/// Dediated function for allocating to prevent generating a function for every `T`
24752491
#[inline]
2492+
#[cfg(not(no_global_oom_handling))]
24762493
fn allocate(alloc: &ArcAllocator<Global>, layout: Layout, init: AllocInit) -> NonNull<u8> {
24772494
try_allocate(alloc, layout, init).unwrap_or_else(|_| handle_alloc_error(layout))
24782495
}

‎library/core/src/alloc/helper.rs

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use crate::{
55
alloc::{AllocError, Allocator, Layout},
66
fmt,
77
marker::PhantomData,
8-
mem,
98
ptr::NonNull,
109
};
1110

@@ -91,31 +90,23 @@ impl<Alloc, Prefix> PrefixAllocator<Alloc, Prefix> {
9190
prefix_layout.size() + prefix_layout.padding_needed_for(layout.align())
9291
}
9392

94-
/// Returns a pointer to the prefix.
93+
/// Returns a pointer to the prefix for an allocated pointer and it's used alignment.
9594
///
9695
/// # Safety
9796
///
9897
/// * `ptr` must denote a block of memory *[currently allocated]* via this allocator, and
99-
/// * `ptr` must point to (and have valid metadata for) a previously valid instance of `T`,
100-
/// but the `T` is allowed to be dropped.
98+
/// * `align` has to be the alignment used for allocating `ptr`.
10199
///
102100
/// [currently allocated]: https://doc.rust-lang.org/nightly/core/alloc/trait.AllocRef.html#currently-allocated-memory
103101
#[inline]
104-
pub unsafe fn prefix<T: ?Sized>(ptr: NonNull<T>) -> NonNull<Prefix> {
102+
pub unsafe fn prefix(ptr: NonNull<u8>, align: usize) -> NonNull<Prefix> {
105103
let prefix_layout = Layout::new::<Prefix>();
106104

107-
// SAFETY: since the only unsized types possible are slices, trait objects,
108-
// and extern types, the input safety requirement is currently enough to
109-
// satisfy the requirements of for_value_raw; this is an implementation
110-
// detail of the language that may not be relied upon outside of std.
111-
let align = unsafe { mem::align_of_val_raw(ptr.as_ptr()) };
112-
113105
let offset = prefix_layout.size() + prefix_layout.padding_needed_for(align);
114-
let ptr = ptr.as_ptr() as *mut u8;
115106

116107
// SAFETY: `ptr` was allocated with this allocator thus, `ptr - offset` points to the
117108
// prefix and is non-null.
118-
unsafe { NonNull::new_unchecked(ptr.sub(offset)).cast() }
109+
unsafe { NonNull::new_unchecked(ptr.as_ptr().sub(offset).cast()) }
119110
}
120111

121112
fn create_ptr(ptr: NonNull<[u8]>, offset_prefix: usize) -> NonNull<[u8]> {

0 commit comments

Comments
 (0)