[unsafe-fields] Initial commit #1929
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1929 +/- ##
=======================================
Coverage 89.46% 89.46%
=======================================
Files 16 16
Lines 5838 5838
=======================================
Hits 5223 5223
Misses 615 615 ☔ View full report in Codecov by Sentry. |
joshlf
force-pushed
the
If0e198c377137dd941ebd5dc68787766a593e1eb
branch
from
3ef0d34 to
a4d4368
Compare
joshlf
force-pushed
the
If0e198c377137dd941ebd5dc68787766a593e1eb
branch
6 times, most recently
from
18fe347 to
fff281b
Compare
joshlf
force-pushed
the
If0e198c377137dd941ebd5dc68787766a593e1eb
branch
2 times, most recently
from
ecfc8ec to
27bf796
Compare
unsafe-fields/src/lib.rs
Outdated
| use core::mem::ManuallyDrop; | ||
|
|
||
| let slf = ManuallyDrop::new(self); | ||
|
|
||
| #[repr(C)] | ||
| union Transmute<O: ?Sized, F, const NAME_HASH: u128> { | ||
| src: ManuallyDrop<Unsafe<O, F, { NAME_HASH }>>, | ||
| dst: ManuallyDrop<F>, | ||
| } | ||
|
|
||
| // SAFETY: `ManuallyDrop<Unsafe<_, F, _>>` has the same size and bit | ||
| // validity as `Unsafe<_, F, _>`. [1] `Unsafe<_, F, _>` is | ||
| // `#[repr(transparent)]` and has no other fields, and so it has the | ||
| // same size and bit validity as `F`. | ||
| // | ||
| // [1] Per https://doc.rust-lang.org/1.81.0/core/mem/struct.ManuallyDrop.html: | ||
| // | ||
| // `ManuallyDrop<T>` is guaranteed to have the same layout and bit | ||
| // validity as `T` | ||
| let dst = unsafe { Transmute { src: slf }.dst }; | ||
|
|
||
| // SAFETY (satisfaction of `Unsafe`'s field invariant): This method is | ||
| // unsafe to call. | ||
| ManuallyDrop::into_inner(dst) |
There was a problem hiding this comment.
Could you just add a comment explaining why we need to go through all of this ceremony?
unsafe-fields/src/lib.rs
Outdated
| #[repr(C)] | ||
| union Transmute<O: ?Sized, F, const NAME_HASH: u128> { | ||
| src: ManuallyDrop<Unsafe<O, F, { NAME_HASH }>>, | ||
| dst: ManuallyDrop<F>, | ||
| } |
There was a problem hiding this comment.
Unless there's good reason to do otherwise, I'd prefer to see this written as:
| #[repr(C)] | |
| union Transmute<O: ?Sized, F, const NAME_HASH: u128> { | |
| src: ManuallyDrop<Unsafe<O, F, { NAME_HASH }>>, | |
| dst: ManuallyDrop<F>, | |
| } | |
| #[repr(C)] | |
| union Transmute<Src, Dst> { | |
| src: ManuallyDrop<Src>, | |
| dst: ManuallyDrop<Dst>, | |
| } |
| /// Gets a reference to the inner value. | ||
| /// | ||
| /// # Safety | ||
| /// | ||
| /// The caller is responsible for upholding any safety invariants associated | ||
| /// with this field. | ||
| #[inline(always)] | ||
| pub const unsafe fn as_ref(&self) -> &F { | ||
| // SAFETY: This method is unsafe to call. | ||
| &self.field | ||
| } |
There was a problem hiding this comment.
With Immutable, we could provide as_ref safely. Perhaps — in the interest of leaving that name open for the safe accessor — this should be called something like as_ref_unchecked?
There was a problem hiding this comment.
IMO we should keep this totally separate from zerocopy. We can also easily release breaking changes since this will never be in anyone's API, so I don't think we need to be too concerned about predicting future API changes.
There was a problem hiding this comment.
Couldn't zerocopy be an option dependency of this?
| // the user is still able to provide a manual impl, so this does not | ||
| // fundamentally restrict what behavior can be supported. | ||
| impl<O: ?Sized, F: Copy, const NAME_HASH: u128> Copy for Unsafe<O, F, { NAME_HASH }> {} | ||
| impl<O: ?Sized, F: Copy, const NAME_HASH: u128> Clone for Unsafe<O, F, { NAME_HASH }> { |
There was a problem hiding this comment.
Food for thought: With Immutable, I think we could provide basically all of the standard trait impls. No need to take action on this in this PR.
joshlf
force-pushed
the
If0e198c377137dd941ebd5dc68787766a593e1eb
branch
from
27bf796 to
cf569b2
Compare
joshlf
force-pushed
the
If0e198c377137dd941ebd5dc68787766a593e1eb
branch
3 times, most recently
from
b57a14b to
667d5db
Compare
Makes progress on #1931 gherrit-pr-id: If0e198c377137dd941ebd5dc68787766a593e1eb
joshlf
force-pushed
the
If0e198c377137dd941ebd5dc68787766a593e1eb
branch
from
667d5db to
e5f23a3
Compare
Makes progress on #1931
This PR is on branch unsafe-fields.