Add initial support for unsized `MaybeUninit` wrapper type by jswrenn · Pull Request #2055 · google/zerocopy

jswrenn · 2024-11-13T20:39:58Z

This is achieved by adding a MaybeUninit associated type to KnownLayout, whose layout is identical to Self except that it admits uninitialized bytes in all positions.

For sized types, this is bound to mem::MaybeUninit<Self>. For potentially unsized structs, we synthesize a doppelganger with the same repr, whose leading fields are wrapped in mem::MaybeUninit and whose trailing field is the MaybeUninit associated type of struct's original trailing field type. This type-level recursion bottoms out at [T], whose MaybeUninit associated type is bound to [mem::MaybeUninit<T>].

Next Steps and Future Possibilities

`MaybeUninit<T: ?Sized>`

In the near term, we may remove doc(hidden) from our MaybeUninit wrapper. In doing so, we'd be quick-to-ship a gadget that extends the present capabilities of Rust. We might, then, be able to use this feature as a demonstration of our approach, potentially suitable for upstreaming into rustc.

`UnalignUnsized<T: ?Sized>`

Presently, our Unalign wrapper requires T: Sized, because we could not figure out how to drop unsized values. For sized values, we copy them onto the stack, then run their destructor. With MaybeUninit<T: ?Sized>, we can extend unalign support to unsized values, by first copying them into a Box<MaybeUninit<T>>, casting the box to Box<T>, and dropping it. This process would be skipped for types with trivial drops, keeping the common case simple and efficient.

`Value<T, I> where I: Invariants<Validity = Any>`

Combining the above two approaches, we could create an invariant-parameterized Ptr analogue for values that generalizes over initialization and alignment.

joshlf

Haven't looked at most of zerocopy-derive yet, but looked at all of zerocopy.

src/lib.rs

src/util/mod.rs

src/wrappers.rs

zerocopy-derive/src/lib.rs

codecov-commenter · 2024-11-14T14:50:12Z

Codecov Report

❌ Patch coverage is 79.54545% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.42%. Comparing base (664b976) to head (60f0a43).
⚠️ Report is 42 commits behind head on v0.8.x.

Files with missing lines	Patch %	Lines
src/wrappers.rs	61.95%	35 Missing ⚠️
src/util/mod.rs	98.48%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           v0.8.x    #2055      +/-   ##
==========================================
- Coverage   88.04%   87.42%   -0.63%     
==========================================
  Files          16       16              
  Lines        5983     6115     +132     
==========================================
+ Hits         5268     5346      +78     
- Misses        715      769      +54

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jswrenn · 2024-11-14T20:32:33Z

The remaining CI failures are from cargo-semver-checks; it complains that we've added a new item to KnownLayout:

--- failure trait_associated_type_added: non-sealed public trait added associated type without default value ---

Description:
A non-sealed trait has gained an associated type without a default value, which breaks downstream implementations of the trait
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#trait-new-item-no-default
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.36.0/src/lints/trait_associated_type_added.ron

Failed in:
  trait associated type zerocopy::KnownLayout::MaybeUninit in file /home/runner/work/zerocopy/zerocopy/src/lib.rs:742

joshlf · 2024-11-19T23:51:46Z

The remaining CI failures are from cargo-semver-checks; it complains that we've added a new item to KnownLayout:

--- failure trait_associated_type_added: non-sealed public trait added associated type without default value ---

Description:
A non-sealed trait has gained an associated type without a default value, which breaks downstream implementations of the trait
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#trait-new-item-no-default
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.36.0/src/lints/trait_associated_type_added.ron

Failed in:
  trait associated type zerocopy::KnownLayout::MaybeUninit in file /home/runner/work/zerocopy/zerocopy/src/lib.rs:742

We should report this upstream. In particular, given that KnownLayout already has #[doc(hidden)] items, we know that there can be no downstream implementations of KnownLayout which have not already opted out of semver stability.

joshlf

A few more nits, but otherwise LGTM!

src/util/mod.rs

joshlf · 2024-11-19T23:47:40Z

src/util/mod.rs

+    // fields both start at the safe offset and the types of those fields are
+    // transparent wrappers around `Src` and `Dst` [1]. Consequently,
+    // initializng `Transmute` with with `src` and then reading out `dst` is
+    // equivalent to transmuting from `Src` to `Dst` [2].


Also need to prove that such a transmute is sound (guaranteed by the caller).

Done. Also note this change.

Is that latter change semantically meaningful? Or do you just prefer that wording?

zerocopy-derive/src/lib.rs

jswrenn · 2024-11-20T16:02:25Z

We should report this upstream. In particular, given that KnownLayout already has #[doc(hidden)] items, we know that there can be no downstream implementations of KnownLayout which have not already opted out of semver stability.

Filed: obi1kenobi/cargo-semver-checks#997

In the meantime, I think we should permit cargo-semver-checks to fail in CI. Otherwise, our only recourse is to bypass branch protections altogether, which would allow this PR to skip some of our more intensive CI steps that are run only in the merge queue.

.github/workflows/ci.yml

This is achieved by adding a `MaybeUninit` associated type to `KnownLayout`, whose layout is identical to `Self` except that it admits uninitialized bytes in all positions. For sized types, this is bound to `mem::MaybeUninit<Self>`. For potentially unsized structs, we synthesize a doppelganger with the same `repr`, whose leading fields are wrapped in `mem::MaybeUninit` and whose trailing field is the `MaybeUninit` associated type of struct's original trailing field type. This type-level recursion bottoms out at `[T]`, whose `MaybeUninit` associated type is bound to `[mem::MaybeUninit<T>]`. Makes progress towards #1797 SKIP_CARGO_SEMVER_CHECKS=1

This is achieved by adding a `MaybeUninit` associated type to `KnownLayout`, whose layout is identical to `Self` except that it admits uninitialized bytes in all positions. For sized types, this is bound to `mem::MaybeUninit<Self>`. For potentially unsized structs, we synthesize a doppelganger with the same `repr`, whose leading fields are wrapped in `mem::MaybeUninit` and whose trailing field is the `MaybeUninit` associated type of struct's original trailing field type. This type-level recursion bottoms out at `[T]`, whose `MaybeUninit` associated type is bound to `[mem::MaybeUninit<T>]`. Makes progress towards #1797 SKIP_CARGO_SEMVER_CHECKS=1 gherrit-pr-id: Idfc357094e28b54a15d947141241ca2da83dcc91

joshlf · 2025-02-06T19:39:10Z

Backporting in #2310

This is achieved by adding a `MaybeUninit` associated type to `KnownLayout`, whose layout is identical to `Self` except that it admits uninitialized bytes in all positions. For sized types, this is bound to `mem::MaybeUninit<Self>`. For potentially unsized structs, we synthesize a doppelganger with the same `repr`, whose leading fields are wrapped in `mem::MaybeUninit` and whose trailing field is the `MaybeUninit` associated type of struct's original trailing field type. This type-level recursion bottoms out at `[T]`, whose `MaybeUninit` associated type is bound to `[mem::MaybeUninit<T>]`. Makes progress towards #1797 SKIP_CARGO_SEMVER_CHECKS=1 gherrit-pr-id: Idfc357094e28b54a15d947141241ca2da83dcc91

…2310) This is achieved by adding a `MaybeUninit` associated type to `KnownLayout`, whose layout is identical to `Self` except that it admits uninitialized bytes in all positions. For sized types, this is bound to `mem::MaybeUninit<Self>`. For potentially unsized structs, we synthesize a doppelganger with the same `repr`, whose leading fields are wrapped in `mem::MaybeUninit` and whose trailing field is the `MaybeUninit` associated type of struct's original trailing field type. This type-level recursion bottoms out at `[T]`, whose `MaybeUninit` associated type is bound to `[mem::MaybeUninit<T>]`. Makes progress towards #1797 SKIP_CARGO_SEMVER_CHECKS=1 gherrit-pr-id: Idfc357094e28b54a15d947141241ca2da83dcc91 Co-authored-by: Jack Wrenn <[email protected]>

jswrenn requested a review from joshlf November 13, 2024 20:39

jswrenn force-pushed the maybe-uninit branch from 271b768 to 96b54d1 Compare November 13, 2024 20:47

joshlf requested changes Nov 13, 2024

View reviewed changes

jswrenn force-pushed the maybe-uninit branch 2 times, most recently from 9844c1f to 9751180 Compare November 14, 2024 14:42

jswrenn force-pushed the maybe-uninit branch 4 times, most recently from 235989d to bb140d0 Compare November 14, 2024 20:17

jswrenn requested a review from joshlf November 19, 2024 15:24

joshlf requested changes Nov 20, 2024

View reviewed changes

jswrenn force-pushed the maybe-uninit branch from bb140d0 to b573d83 Compare November 20, 2024 15:41

jswrenn mentioned this pull request Nov 20, 2024

Adding doc(hidden) non-defaulted item is permissible if other doc(hidden) non-defaulted items exist obi1kenobi/cargo-semver-checks#997

Closed

1 task

joshlf approved these changes Nov 20, 2024

View reviewed changes

joshlf mentioned this pull request Nov 20, 2024

[ci] Permit skipping cargo-semver-checks for a particular PR #2083

Closed

jswrenn force-pushed the maybe-uninit branch from b573d83 to 9f151e8 Compare November 21, 2024 14:02

jswrenn enabled auto-merge November 21, 2024 14:02

jswrenn force-pushed the maybe-uninit branch 3 times, most recently from d94bda0 to 944a13e Compare November 21, 2024 18:38

joshlf reviewed Nov 21, 2024

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

jswrenn force-pushed the maybe-uninit branch from 944a13e to 167b002 Compare November 21, 2024 21:14

jswrenn force-pushed the maybe-uninit branch from 167b002 to 60f0a43 Compare November 21, 2024 21:24

jswrenn added this pull request to the merge queue Nov 21, 2024

Merged via the queue into v0.8.x with commit d159e36 Nov 21, 2024

jswrenn deleted the maybe-uninit branch November 21, 2024 21:58

joshlf added the needs-backport label Nov 21, 2024

jswrenn mentioned this pull request Nov 25, 2024

UnalignUnsized #1828

Open

joshlf mentioned this pull request Dec 21, 2024

Inconsistent visibility on some derive-synthesized types #2177

Closed

joshlf mentioned this pull request Feb 6, 2025

Add initial support for unsized MaybeUninit wrapper type (#2055) #2310

Merged

joshlf removed the needs-backport label Feb 6, 2025

Comments

Conversation

jswrenn commented Nov 13, 2024

Next Steps and Future Possibilities

MaybeUninit<T: ?Sized>

UnalignUnsized<T: ?Sized>

Value<T, I> where I: Invariants<Validity = Any>

Uh oh!

joshlf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Nov 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jswrenn commented Nov 14, 2024

Uh oh!

joshlf commented Nov 19, 2024

Uh oh!

joshlf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joshlf Nov 19, 2024

Choose a reason for hiding this comment

Uh oh!

jswrenn Nov 20, 2024

Choose a reason for hiding this comment

Uh oh!

joshlf Nov 20, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jswrenn commented Nov 20, 2024

Uh oh!

Uh oh!

joshlf commented Feb 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

`MaybeUninit<T: ?Sized>`

`UnalignUnsized<T: ?Sized>`

`Value<T, I> where I: Invariants<Validity = Any>`

codecov-commenter commented Nov 14, 2024 •

edited

Loading