Skip to content

Commit 1c5db28

Browse files
Rollup merge of rust-lang#123254 - stepancheg:thin-box-0-const-alloc, r=oli-obk
Do not allocate for ZST ThinBox (attempt 2 using const_allocate) There's PR rust-lang#123184 which avoids allocation for ZST ThinBox. That PR has an issue with unsoundness with padding in `MaybeUninit` (see comments in that PR). Also that PR relies on `Freeze` trait. This PR is much simpler implementation which does not have this problem, but it uses `const_allocate` feature. `@oli-obk` suggested that `const_allocate` should not be used for that feature. But I like how easy it to do this feature with `const_allocate`. Maybe it's OK to use `const_allocate` while `ThinBox` is unstable? Or, well, we can abandon this PR. r? `@oli-obk`
2 parents bb78dba + f539134 commit 1c5db28

File tree

2 files changed

+85
-13
lines changed

2 files changed

+85
-13
lines changed

library/alloc/src/boxed/thin.rs

+83-13
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,14 @@
44
use crate::alloc::{self, Layout, LayoutError};
55
use core::error::Error;
66
use core::fmt::{self, Debug, Display, Formatter};
7+
#[cfg(not(no_global_oom_handling))]
8+
use core::intrinsics::const_allocate;
79
use core::marker::PhantomData;
810
#[cfg(not(no_global_oom_handling))]
911
use core::marker::Unsize;
10-
use core::mem::{self, SizedTypeProperties};
12+
use core::mem;
13+
#[cfg(not(no_global_oom_handling))]
14+
use core::mem::SizedTypeProperties;
1115
use core::ops::{Deref, DerefMut};
1216
use core::ptr::Pointee;
1317
use core::ptr::{self, NonNull};
@@ -109,9 +113,14 @@ impl<Dyn: ?Sized> ThinBox<Dyn> {
109113
where
110114
T: Unsize<Dyn>,
111115
{
112-
let meta = ptr::metadata(&value as &Dyn);
113-
let ptr = WithOpaqueHeader::new(meta, value);
114-
ThinBox { ptr, _marker: PhantomData }
116+
if mem::size_of::<T>() == 0 {
117+
let ptr = WithOpaqueHeader::new_unsize_zst::<Dyn, T>(value);
118+
ThinBox { ptr, _marker: PhantomData }
119+
} else {
120+
let meta = ptr::metadata(&value as &Dyn);
121+
let ptr = WithOpaqueHeader::new(meta, value);
122+
ThinBox { ptr, _marker: PhantomData }
123+
}
115124
}
116125
}
117126

@@ -200,6 +209,16 @@ impl WithOpaqueHeader {
200209
Self(ptr.0)
201210
}
202211

212+
#[cfg(not(no_global_oom_handling))]
213+
fn new_unsize_zst<Dyn, T>(value: T) -> Self
214+
where
215+
Dyn: ?Sized,
216+
T: Unsize<Dyn>,
217+
{
218+
let ptr = WithHeader::<<Dyn as Pointee>::Metadata>::new_unsize_zst::<Dyn, T>(value);
219+
Self(ptr.0)
220+
}
221+
203222
fn try_new<H, T>(header: H, value: T) -> Result<Self, core::alloc::AllocError> {
204223
WithHeader::try_new(header, value).map(|ptr| Self(ptr.0))
205224
}
@@ -288,6 +307,58 @@ impl<H> WithHeader<H> {
288307
}
289308
}
290309

310+
// `Dyn` is `?Sized` type like `[u32]`, and `T` is ZST type like `[u32; 0]`.
311+
#[cfg(not(no_global_oom_handling))]
312+
fn new_unsize_zst<Dyn, T>(value: T) -> WithHeader<H>
313+
where
314+
Dyn: Pointee<Metadata = H> + ?Sized,
315+
T: Unsize<Dyn>,
316+
{
317+
assert!(mem::size_of::<T>() == 0);
318+
319+
const fn max(a: usize, b: usize) -> usize {
320+
if a > b { a } else { b }
321+
}
322+
323+
// Compute a pointer to the right metadata. This will point to the beginning
324+
// of the header, past the padding, so the assigned type makes sense.
325+
// It also ensures that the address at the end of the header is sufficiently
326+
// aligned for T.
327+
let alloc: &<Dyn as Pointee>::Metadata = const {
328+
// FIXME: just call `WithHeader::alloc_layout` with size reset to 0.
329+
// Currently that's blocked on `Layout::extend` not being `const fn`.
330+
331+
let alloc_align =
332+
max(mem::align_of::<T>(), mem::align_of::<<Dyn as Pointee>::Metadata>());
333+
334+
let alloc_size =
335+
max(mem::align_of::<T>(), mem::size_of::<<Dyn as Pointee>::Metadata>());
336+
337+
unsafe {
338+
// SAFETY: align is power of two because it is the maximum of two alignments.
339+
let alloc: *mut u8 = const_allocate(alloc_size, alloc_align);
340+
341+
let metadata_offset =
342+
alloc_size.checked_sub(mem::size_of::<<Dyn as Pointee>::Metadata>()).unwrap();
343+
// SAFETY: adding offset within the allocation.
344+
let metadata_ptr: *mut <Dyn as Pointee>::Metadata =
345+
alloc.add(metadata_offset).cast();
346+
// SAFETY: `*metadata_ptr` is within the allocation.
347+
metadata_ptr.write(ptr::metadata::<Dyn>(ptr::dangling::<T>() as *const Dyn));
348+
349+
// SAFETY: we have just written the metadata.
350+
&*(metadata_ptr)
351+
}
352+
};
353+
354+
// SAFETY: `alloc` points to `<Dyn as Pointee>::Metadata`, so addition stays in-bounds.
355+
let value_ptr =
356+
unsafe { (alloc as *const <Dyn as Pointee>::Metadata).add(1) }.cast::<T>().cast_mut();
357+
debug_assert!(value_ptr.is_aligned());
358+
mem::forget(value);
359+
WithHeader(NonNull::new(value_ptr.cast()).unwrap(), PhantomData)
360+
}
361+
291362
// Safety:
292363
// - Assumes that either `value` can be dereferenced, or is the
293364
// `NonNull::dangling()` we use when both `T` and `H` are ZSTs.
@@ -300,20 +371,19 @@ impl<H> WithHeader<H> {
300371

301372
impl<H> Drop for DropGuard<H> {
302373
fn drop(&mut self) {
374+
// All ZST are allocated statically.
375+
if self.value_layout.size() == 0 {
376+
return;
377+
}
378+
303379
unsafe {
304380
// SAFETY: Layout must have been computable if we're in drop
305381
let (layout, value_offset) =
306382
WithHeader::<H>::alloc_layout(self.value_layout).unwrap_unchecked();
307383

308-
// Note: Don't deallocate if the layout size is zero, because the pointer
309-
// didn't come from the allocator.
310-
if layout.size() != 0 {
311-
alloc::dealloc(self.ptr.as_ptr().sub(value_offset), layout);
312-
} else {
313-
debug_assert!(
314-
value_offset == 0 && H::IS_ZST && self.value_layout.size() == 0
315-
);
316-
}
384+
// Since we only allocate for non-ZSTs, the layout size cannot be zero.
385+
debug_assert!(layout.size() != 0);
386+
alloc::dealloc(self.ptr.as_ptr().sub(value_offset), layout);
317387
}
318388
}
319389
}

library/alloc/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,10 @@
108108
#![feature(const_box)]
109109
#![feature(const_cow_is_borrowed)]
110110
#![feature(const_eval_select)]
111+
#![feature(const_heap)]
111112
#![feature(const_maybe_uninit_as_mut_ptr)]
112113
#![feature(const_maybe_uninit_write)]
114+
#![feature(const_option)]
113115
#![feature(const_pin)]
114116
#![feature(const_refs_to_cell)]
115117
#![feature(const_size_of_val)]

0 commit comments

Comments
 (0)