Skip to content

Commit e28215f

Browse files
jswrennjoshlf
authored andcommitted
Add HasField::project; simplify is_bit_valid
This method projects from `PtrInner<_, Self>` to a `PtrInner` of a field of `Self`. In this commit, we use `HasField::project` to considerably simplify `is_bit_valid` implementations. gherrit-pr-id: G9e7039c715e1ec53e6d860909bb0d618898fd46b
1 parent 1834846 commit e28215f

File tree

7 files changed

+1760
-406
lines changed

7 files changed

+1760
-406
lines changed

Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,8 @@ glob = "=0.3.2"
130130
itertools = "0.11"
131131
rand = { version = "0.8.5", default-features = false, features = ["small_rng"] }
132132
rustversion = "1.0"
133+
# More recent versions of `ryu` have an MSRV higher than ours.
134+
ryu = "=1.0.20"
133135
static_assertions = "1.1"
134136
testutil = { path = "testutil" }
135137
# Pinned to a specific version so that the version used for local development

src/impls.rs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -794,6 +794,54 @@ impl_for_transmute_from!(T: ?Sized + IntoBytes => IntoBytes for ManuallyDrop<T>[
794794
const _: () = unsafe { unsafe_impl!(T: ?Sized + Unaligned => Unaligned for ManuallyDrop<T>) };
795795
assert_unaligned!(ManuallyDrop<()>, ManuallyDrop<u8>);
796796

797+
const _: () = {
798+
#[allow(
799+
non_camel_case_types,
800+
missing_copy_implementations,
801+
missing_debug_implementations,
802+
missing_docs
803+
)]
804+
pub enum value {}
805+
806+
// SAFETY: `ManuallyDrop<T>` has a field of type `T` at offset `0` without
807+
// any safety invariants beyond those of `T`. Its existence is not
808+
// explicitly documented, but it can be inferred; per [1] `ManuallyDrop<T>`
809+
// has the same size and bit validity as `T`. This field is not literally
810+
// public, but is effectively so; the field can be transparently:
811+
//
812+
// - initialized via `ManuallyDrop::new`
813+
// - moved via `ManuallyDrop::into_inner`
814+
// - referenced via `ManuallyDrop::deref`
815+
// - exclusively referenced via `ManuallyDrop::deref_mut`
816+
//
817+
// We call this field `value`, both because that is both the name of this
818+
// private field, and because it is the name it is referred to in the public
819+
// documentation of `ManuallyDrop::new`, `ManuallyDrop::into_inner`,
820+
// `ManuallyDrop::take` and `ManuallyDrop::drop`.
821+
unsafe impl<T> HasField<value, 0, { crate::ident_id!(value) }> for ManuallyDrop<T> {
822+
type Type = T;
823+
824+
#[inline]
825+
fn only_derive_is_allowed_to_implement_this_trait()
826+
where
827+
Self: Sized,
828+
{
829+
}
830+
831+
#[inline(always)]
832+
fn project(slf: PtrInner<'_, Self>) -> PtrInner<'_, T> {
833+
// SAFETY: `ManuallyDrop<T>` has the same layout and bit validity as
834+
// `T` [1].
835+
//
836+
// [1] Per https://doc.rust-lang.org/1.85.0/std/mem/struct.ManuallyDrop.html:
837+
//
838+
// `ManuallyDrop<T>` is guaranteed to have the same layout and bit
839+
// validity as `T`
840+
unsafe { slf.cast() }
841+
}
842+
}
843+
};
844+
797845
impl_for_transmute_from!(T: ?Sized + TryFromBytes => TryFromBytes for Cell<T>[UnsafeCell<T>]);
798846
impl_for_transmute_from!(T: ?Sized + FromZeros => FromZeros for Cell<T>[UnsafeCell<T>]);
799847
impl_for_transmute_from!(T: ?Sized + FromBytes => FromBytes for Cell<T>[UnsafeCell<T>]);

src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,8 @@ use core::{
372372
use std::io;
373373

374374
use crate::pointer::invariant::{self, BecauseExclusive};
375+
#[doc(hidden)]
376+
pub use crate::pointer::PtrInner;
375377
pub use crate::{
376378
byte_slice::*,
377379
byteorder::*,
@@ -1109,6 +1111,9 @@ pub unsafe trait HasField<Field, const VARIANT_ID: u128, const FIELD_ID: u128> {
11091111

11101112
/// The type of the field.
11111113
type Type: ?Sized;
1114+
1115+
/// Projects from `slf` to the field.
1116+
fn project(slf: PtrInner<'_, Self>) -> PtrInner<'_, Self::Type>;
11121117
}
11131118

11141119
/// Analyzes whether a type is [`FromZeros`].

src/pointer/inner.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use crate::util::polyfills::NumExt as _;
1515
use crate::{
1616
layout::{CastType, MetadataCastError},
1717
util::AsAddress,
18-
AlignmentError, CastError, KnownLayout, MetadataOf, SizeError, SplitAt,
18+
AlignmentError, CastError, HasField, KnownLayout, MetadataOf, SizeError, SplitAt,
1919
};
2020

2121
mod _def {
@@ -196,6 +196,16 @@ impl<'a, T: ?Sized> PtrInner<'a, T> {
196196
// then `A` is guaranteed to live for at least `'a`.
197197
unsafe { PtrInner::new(ptr) }
198198
}
199+
200+
/// Projects a field.
201+
#[must_use]
202+
#[inline(always)]
203+
pub fn project<F, const VARIANT_ID: u128, const FIELD_ID: u128>(self) -> PtrInner<'a, T::Type>
204+
where
205+
T: HasField<F, VARIANT_ID, FIELD_ID>,
206+
{
207+
<T as HasField<F, VARIANT_ID, FIELD_ID>>::project(self)
208+
}
199209
}
200210

201211
#[allow(clippy::needless_lifetimes)]

zerocopy-derive/src/enum.rs

Lines changed: 72 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,13 @@
99
use proc_macro2::TokenStream;
1010
use quote::quote;
1111
use syn::{
12-
parse_quote, spanned::Spanned as _, DataEnum, DeriveInput, Error, Fields, Generics, Ident, Path,
12+
parse_quote, spanned::Spanned as _, DataEnum, DeriveInput, Error, Fields, Generics, Ident,
13+
Index, Path,
1314
};
1415

1516
use crate::{
16-
derive_try_from_bytes_inner, repr::EnumRepr, DataExt, FieldBounds, ImplBlockBuilder, Trait,
17+
derive_has_field_struct_union, derive_try_from_bytes_inner, repr::EnumRepr, DataExt,
18+
FieldBounds, ImplBlockBuilder, Trait,
1719
};
1820

1921
/// Generates a tag enum for the given enum. This generates an enum with the
@@ -162,7 +164,18 @@ fn generate_variant_structs(
162164
}
163165
}
164166

165-
fn generate_variants_union(generics: &Generics, data: &DataEnum) -> TokenStream {
167+
fn variants_union_field_ident(ident: &Ident) -> Ident {
168+
let variant_ident_str = crate::ext::to_ident_str(ident);
169+
// Field names are prefixed with `__field_` to prevent name collision
170+
// with the `__nonempty` field.
171+
Ident::new(&format!("__field_{}", variant_ident_str), ident.span())
172+
}
173+
174+
fn generate_variants_union(
175+
generics: &Generics,
176+
data: &DataEnum,
177+
zerocopy_crate: &Path,
178+
) -> TokenStream {
166179
let (_, ty_generics, _) = generics.split_for_impl();
167180

168181
let fields = data.variants.iter().filter_map(|variant| {
@@ -172,10 +185,7 @@ fn generate_variants_union(generics: &Generics, data: &DataEnum) -> TokenStream
172185
return None;
173186
}
174187

175-
// Field names are prefixed with `__field_` to prevent name collision
176-
// with the `__nonempty` field.
177-
let field_name_str = crate::ext::to_ident_str(&variant.ident);
178-
let field_name = Ident::new(&format!("__field_{}", field_name_str), variant.ident.span());
188+
let field_name = variants_union_field_ident(&variant.ident);
179189
let variant_struct_ident = variant_struct_ident(&variant.ident);
180190

181191
Some(quote! {
@@ -185,7 +195,7 @@ fn generate_variants_union(generics: &Generics, data: &DataEnum) -> TokenStream
185195
})
186196
});
187197

188-
quote! {
198+
let variants_union = parse_quote! {
189199
#[repr(C)]
190200
#[allow(non_snake_case)]
191201
union ___ZerocopyVariants #generics {
@@ -197,6 +207,14 @@ fn generate_variants_union(generics: &Generics, data: &DataEnum) -> TokenStream
197207
// affect the layout.
198208
__nonempty: (),
199209
}
210+
};
211+
212+
let has_field =
213+
derive_has_field_struct_union(&variants_union, &variants_union.data, zerocopy_crate);
214+
215+
quote! {
216+
#variants_union
217+
#has_field
200218
}
201219
}
202220

@@ -246,18 +264,20 @@ pub(crate) fn derive_is_bit_valid(
246264
};
247265

248266
let variant_structs = generate_variant_structs(enum_ident, generics, data, zerocopy_crate);
249-
let variants_union = generate_variants_union(generics, data);
267+
let variants_union = generate_variants_union(generics, data, zerocopy_crate);
250268

251-
let (_, ty_generics, _) = generics.split_for_impl();
269+
let (_, ref ty_generics, _) = generics.split_for_impl();
252270

253271
let has_fields = data.variants().into_iter().flat_map(|(variant, fields)| {
254272
let variant_ident = &variant.unwrap().ident;
273+
let variants_union_field_ident = variants_union_field_ident(variant_ident);
255274
let field: Box<syn::Type> = parse_quote!(());
256-
fields.into_iter().map(move |(vis, ident, ty)| {
275+
fields.into_iter().enumerate().map(move |(idx, (vis, ident, ty))| {
257276
// Rust does not presently support explicit visibility modifiers on
258277
// enum fields, but we guard against the possibility to ensure this
259278
// derive remains sound.
260279
assert!(matches!(vis, syn::Visibility::Inherited));
280+
let variant_struct_field_index = Index::from(idx + 1);
261281
ImplBlockBuilder::new(
262282
ast,
263283
data,
@@ -274,6 +294,18 @@ pub(crate) fn derive_is_bit_valid(
274294
)
275295
.inner_extras(quote! {
276296
type Type = #ty;
297+
298+
#[inline(always)]
299+
fn project(slf: #zerocopy_crate::PtrInner<'_, Self>) -> #zerocopy_crate::PtrInner<'_, Self::Type> {
300+
// SAFETY: By invariant on `___ZerocopyRawEnum`,
301+
// `___ZerocopyRawEnum` has the same layout as `Self`.
302+
let slf = unsafe { slf.cast::<___ZerocopyRawEnum #ty_generics>() };
303+
304+
slf.project::<_, 0, { #zerocopy_crate::ident_id!(variants) }>()
305+
.project::<_, 0, { #zerocopy_crate::ident_id!(#variants_union_field_ident) }>()
306+
.project::<_, 0, { #zerocopy_crate::ident_id!(value) }>()
307+
.project::<_, 0, { #zerocopy_crate::ident_id!(#variant_struct_field_index) }>()
308+
}
277309
})
278310
.build()
279311
})
@@ -323,6 +355,22 @@ pub(crate) fn derive_is_bit_valid(
323355
}
324356
});
325357

358+
let raw_enum = parse_quote! {
359+
#[repr(C)]
360+
struct ___ZerocopyRawEnum #generics {
361+
tag: ___ZerocopyOuterTag,
362+
variants: ___ZerocopyVariants #ty_generics,
363+
}
364+
};
365+
366+
let raw_enum_projections =
367+
derive_has_field_struct_union(&raw_enum, &raw_enum.data, zerocopy_crate);
368+
369+
let raw_enum = quote! {
370+
#raw_enum
371+
#raw_enum_projections
372+
};
373+
326374
Ok(quote! {
327375
// SAFETY: We use `is_bit_valid` to validate that the bit pattern of the
328376
// enum's tag corresponds to one of the enum's discriminants. Then, we
@@ -351,11 +399,7 @@ pub(crate) fn derive_is_bit_valid(
351399

352400
#variants_union
353401

354-
#[repr(C)]
355-
struct ___ZerocopyRawEnum #generics {
356-
tag: ___ZerocopyOuterTag,
357-
variants: ___ZerocopyVariants #ty_generics,
358-
}
402+
#raw_enum
359403

360404
#(#has_fields)*
361405

@@ -399,26 +443,23 @@ pub(crate) fn derive_is_bit_valid(
399443
// invariant from `p`, so we re-assert that all of the bytes are
400444
// initialized.
401445
let raw_enum = unsafe { raw_enum.assume_initialized() };
446+
402447
// SAFETY:
403-
// - This projection returns a subfield of `this` using
404-
// `addr_of_mut!`.
405-
// - Because the subfield pointer is derived from `this`, it has the
406-
// same provenance.
448+
// - This projection returns a subfield of `raw_enum` using
449+
// `project`.
450+
// - Because the subfield pointer is derived from `raw_enum`, it has
451+
// the same provenance.
407452
// - The locations of `UnsafeCell`s in the subfield match the
408-
// locations of `UnsafeCell`s in `this`. This is because the
453+
// locations of `UnsafeCell`s in `raw_enum`. This is because the
409454
// subfield pointer just points to a smaller portion of the
410455
// overall struct.
456+
let project = #zerocopy_crate::pointer::PtrInner::project::<
457+
_,
458+
0,
459+
{ #zerocopy_crate::ident_id!(variants) }
460+
>;
411461
let variants = unsafe {
412-
use #zerocopy_crate::pointer::PtrInner;
413-
raw_enum.cast_unsized_unchecked(|p: PtrInner<'_, ___ZerocopyRawEnum #ty_generics>| {
414-
let p = p.as_non_null().as_ptr();
415-
let ptr = core_reexport::ptr::addr_of_mut!((*p).variants);
416-
// SAFETY: `ptr` is a projection into `p`, which is
417-
// `NonNull`, and guaranteed not to wrap around the address
418-
// space. Thus, `ptr` cannot be null.
419-
let ptr = unsafe { core_reexport::ptr::NonNull::new_unchecked(ptr) };
420-
unsafe { PtrInner::new(ptr) }
421-
})
462+
raw_enum.cast_unsized_unchecked(project)
422463
};
423464

424465
#[allow(non_upper_case_globals)]

0 commit comments

Comments
 (0)