UnalignUnsized

[jswrenn]

This PR introduces UnalignUnsized<T: ?Sized + KnownLayout>, a wrapper around a value T that makes it trivially aligned. This nearly obsoletes zerocopy's existing Unalign<T> wrapper, which cannot support unsized types but also does not require T: KnownLayout.

Background

Zerocopy presently restricts Unalign's T to Sized because of language-level shortcomings of Rust. For example, this naive attempt to unsize Unalign:

#[repr(packed)]
struct Unalign<T: ?Sized>(T);

...produces a compilation error:

error[E0277]: the size for values of type `T` cannot be known at compilation time
 --> src/lib.rs:2:27
  |
2 | struct Unalign<T: ?Sized>(T);
  |                -          ^ doesn't have a size known at compile-time
  |                |
  |                this type parameter needs to be `Sized`
  |
  = note: the last field of a packed struct may only have a dynamically sized type if it does not need drop to be run
  = help: change the field's type to have a statically known size
help: consider removing the `?Sized` bound to make the type parameter `Sized`

This may be circumvented either by bounding T with Copy (which precludes it from having a non-trivial destructor), or by wrapping T in ManuallyDrop; e.g.:

use core::mem::ManuallyDrop;

#[repr(packed)]
struct Unalign<T: ?Sized>(ManuallyDrop<T>);

Rust accepts this, but at the cost of Drop: The wrapped value's destructor will not be executed. To recover T's destructor, we must find a way to first align it so it may be dropped.

Rust achieves this for Unalign with T: Sized by first reading its wrapped value onto the stack, then dropping it. We cannot immediately apply the same technique for T: ?Sized; doing this:

use core::mem::ManuallyDrop;

#[repr(packed)]
struct Unalign<T: ?Sized>(ManuallyDrop<T>);

impl<T: ?Sized> Drop for Unalign<T> {
    fn drop(&mut self) {
        let inner = self.0; 
    }
}

...produces the error:

error[E0277]: the size for values of type `T` cannot be known at compilation time
   --> src/lib.rs:8:13
    |
6   | impl<T: ?Sized> Drop for Unalign<T> {
    |      - this type parameter needs to be `Sized`
7   |     fn drop(&mut self) {
8   |         let inner = self.0; 
    |             ^^^^^ doesn't have a size known at compile-time
    |
note: required because it appears within the type `ManuallyDrop<T>`
   --> /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/mem/manually_drop.rs:157:12
    |
157 | pub struct ManuallyDrop<T: ?Sized> {
    |            ^^^^^^^^^^^^
    = note: all local variables must have a statically known size
    = help: unsized locals are gated as an unstable feature
help: consider removing the `?Sized` bound to make the type parameter `Sized`
    |
6   - impl<T: ?Sized> Drop for Unalign<T> {
6   + impl<T> Drop for Unalign<T> {

On nightly, we could perhaps circumvent this with #[feature(unsized_locals)] (rust-lang/rust#48055) if Rust bounded core::ptr::read_unaligned with T: ?Sized. For now, it does not.

Although we trivially can "fix" this problem by not dropping T, this strikes us as a design smell: Unalign and ManuallyDrop are orthogonal, and the former should not imply the latter.

Design

An acceptable design of Unalign<T: ?Sized> will:

1. admit Ts with non-trivial drops
2. be no less performant than Unalign<T: ?Sized + Copy> for T with trivial drops
3. be no less performant than Unalign<T: Sized> for T with non-trivial drops
4. avoid silently forgetting T

Approach Sketch

This approach of this PR is guided by two insights:

1. An unaligned, stack-allocated DST may be aligned by copying it into a Box then dropping the Box
2. Absent a heap allocator, silent forgets can be minimized by emitting a post-monomorphization compilation error.

We anticipate that occurrences of the post-monomorphization compilation error will be relatively rare, since it will require that:

• the compilation environment is no_std
• the wrapped type type is non-trivially aligned
• the wrapped type has a non-trivial Drop

Dropping DSTs

To drop a DST, we simply:

1. Allocate a Box<MaybeUninit<T>> (made possible for T: ?Sized in Add initial support for unsized MaybeUninit wrapper type #2055)
2. Copy the DST into that Box
3. Cast the Box to Box<T>
4. Drop the Box

Eliminating Silent Forgets

The above approach for dropping DSTs depends on having a global allocator. In contexts where this is unavailable and the wrapped type has both a non-trivial alignment and non-trivial destructor, we emit a post-monomorhpization errors. Consider this toy example:

use core::mem;

struct Undroppable<T: ?Sized>(mem::ManuallyDrop<T>);

impl<T> Undroppable<T> {
    fn new(val: T) -> Self {
        Self(mem::ManuallyDrop::new(val))
    }
}

impl<T:? Sized> Drop for Undroppable<T> {
    fn drop(&mut self) {
        const { panic!("This value must not be dropped.") }
    }
}

fn main() {
    let undroppable = Undroppable::new(42);
    core::mem::forget(undroppable);
}

Commenting out the mem::forget produces a compilation error:

error[E0080]: evaluation of `<Undroppable<i32> as std::ops::Drop>::drop::{constant#0}` failed
  --> src/main.rs:13:17
   |
13 |         const { panic!("This value must not be dropped.") }
   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'This value must not be dropped.', src/main.rs:13:17
   |
   = note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)

note: erroneous constant encountered
  --> src/main.rs:13:9
   |
13 |         const { panic!("This value must not be dropped.") }
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

note: the above error was encountered while instantiating `fn <Undroppable<i32> as std::ops::Drop>::drop`
   --> /playground/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:574:1
    |
574 | pub unsafe fn drop_in_place<T: ?Sized>(to_drop: *mut T) {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This error, though admittedly poor, is better than a panic in production. We can additionally take steps to minimize needless errors. We can amend drop to only error if T has a non-trivial drop; e.g.:

impl<T:? Sized> Drop for Undroppable<T> {
    fn drop(&mut self) {
        const {
            if mem::needs_drop::<T>() {
                panic!("This value must not be dropped.") 
            }
        }
    }
}

And, leveraging KnownLayout, to only error if T is also non-trivially aligned (otherwise it can be dropped in-place); e.g.:

impl<T:? Sized + KnownLayout> Drop for Undroppable<T> {
    fn drop(&mut self) {
        const {
            if mem::needs_drop::<T>() && !T::is_trivially_aligned() {
                panic!("This value must not be dropped.") 
            }
        }
        if T::is_trivially_aligned() {
            unsafe { mem::drop_in_place(self) }
        }
    }
}

[joshlf]

Does this play nicely with Pin'd types?

[jswrenn]

Yes, Pin bars you from moving the referent before running its destructor. It doesn't place any restrictions on what you do in the destructor.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 28.02768% with 208 lines in your changes missing coverage. Please review.

Project coverage is 84.74%. Comparing base (7431cbd) to head (b2f8e08).

Files with missing lines	Patch %	Lines
src/util/mod.rs	0.00%	94 Missing ⚠️
src/pointer/ptr.rs	61.06%	51 Missing ⚠️
src/wrappers.rs	0.00%	33 Missing ⚠️
src/pointer/mod.rs	0.00%	12 Missing ⚠️
src/lib.rs	0.00%	9 Missing ⚠️
src/util/macros.rs	14.28%	6 Missing ⚠️
src/layout.rs	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           v0.8.x    #1828      +/-   ##
==========================================
- Coverage   87.42%   84.74%   -2.69%     
==========================================
  Files          16       16              
  Lines        6115     6403     +288     
==========================================
+ Hits         5346     5426      +80     
- Misses        769      977     +208     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.