Skip to content

Commit 1c5cebf

Browse files
committed
[project] Distinguish structs/union/enums
For the `HasField` `VARIANT_ID` and `FIELD_ID` constants, use `i128` instead of `u128`. For structs and unions, set `VARIANT_ID = -1` and `VARIANT_ID = -2` (respectively) instead of 0. This ensures that we will never accidentally confuse the first (ie, 0th) variant of an enum with a struct or union. It also paves the way for us to define methods which only support a subset of structs/unions/enums. Makes progress on #196, #2856 gherrit-pr-id: G03596047b3e8a9931799295d1e707540561c4e46
1 parent 77f1c47 commit 1c5cebf

File tree

7 files changed

+235
-205
lines changed

7 files changed

+235
-205
lines changed

src/impls.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -818,7 +818,9 @@ const _: () = {
818818
// private field, and because it is the name it is referred to in the public
819819
// documentation of `ManuallyDrop::new`, `ManuallyDrop::into_inner`,
820820
// `ManuallyDrop::take` and `ManuallyDrop::drop`.
821-
unsafe impl<T> HasField<value, 0, { crate::ident_id!(value) }> for ManuallyDrop<T> {
821+
unsafe impl<T> HasField<value, { crate::STRUCT_VARIANT_ID }, { crate::ident_id!(value) }>
822+
for ManuallyDrop<T>
823+
{
822824
type Type = T;
823825

824826
#[inline]

src/lib.rs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,6 +1089,17 @@ const _: () = unsafe {
10891089
unsafe_impl_known_layout!(T: ?Sized + KnownLayout => #[repr(T::MaybeUninit)] MaybeUninit<T>)
10901090
};
10911091

1092+
// FIXME(#196, #2856): Eventually, we'll want to support enums variants and
1093+
// union fields being treated uniformly since they behave similarly to each
1094+
// other in terms of projecting validity – specifically, for a type `T` with
1095+
// validity `V`, if `T` is a struct type, then its fields straightforwardly also
1096+
// have validity `V`. By contrast, if `T` is an enum or union type, then
1097+
// validity is not straightforwardly recursive in this way.
1098+
#[doc(hidden)]
1099+
pub const STRUCT_VARIANT_ID: i128 = -1;
1100+
#[doc(hidden)]
1101+
pub const UNION_VARIANT_ID: i128 = -2;
1102+
10921103
/// Projects a given field from `Self`.
10931104
///
10941105
/// All implementations of `HasField` for a particular field `f` in `Self`
@@ -1099,12 +1110,16 @@ const _: () = unsafe {
10991110
///
11001111
/// A field `f` is `HasField` for `Self` if and only if:
11011112
///
1102-
/// - if `f` has name `n`, `FIELD_ID` is `zerocopy::ident_id!(n)`; otherwise, if
1103-
/// `f` is at index `i`, `FIELD_ID` is `zerocopy::ident_id!(i)`.
1113+
/// - If `Self` is a struct or union type, then `VARIANT_ID` is
1114+
/// `STRUCT_VARIANT_ID` or `UNION_VARIANT_ID` respectively; otherwise, if
1115+
/// `Self` is an enum type, `VARIANT_ID` is the numerical index of the enum
1116+
/// variant in which `f` appears.
1117+
/// - If `f` has name `n`, `FIELD_ID` is `zerocopy::ident_id!(n)`; otherwise,
1118+
/// if `f` is at index `i`, `FIELD_ID` is `zerocopy::ident_id!(i)`.
11041119
/// - `Field` is a type with the same visibility as `f`.
11051120
/// - `Type` has the same type as `f`.
11061121
#[doc(hidden)]
1107-
pub unsafe trait HasField<Field, const VARIANT_ID: u128, const FIELD_ID: u128> {
1122+
pub unsafe trait HasField<Field, const VARIANT_ID: i128, const FIELD_ID: i128> {
11081123
fn only_derive_is_allowed_to_implement_this_trait()
11091124
where
11101125
Self: Sized;

src/pointer/inner.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ impl<'a, T: ?Sized> PtrInner<'a, T> {
200200
/// Projects a field.
201201
#[must_use]
202202
#[inline(always)]
203-
pub fn project<F, const VARIANT_ID: u128, const FIELD_ID: u128>(self) -> PtrInner<'a, T::Type>
203+
pub fn project<F, const VARIANT_ID: i128, const FIELD_ID: i128>(self) -> PtrInner<'a, T::Type>
204204
where
205205
T: HasField<F, VARIANT_ID, FIELD_ID>,
206206
{

src/util/macro_util.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,7 @@ macro_rules! ident_id {
553553
#[inline(always)]
554554
#[must_use]
555555
#[allow(clippy::as_conversions, clippy::indexing_slicing, clippy::arithmetic_side_effects)]
556-
pub const fn hash_name(name: &str) -> u128 {
556+
pub const fn hash_name(name: &str) -> i128 {
557557
let name = name.as_bytes();
558558

559559
// We guarantee freedom from hash collisions between any two strings of
@@ -569,7 +569,7 @@ pub const fn hash_name(name: &str) -> u128 {
569569
i += 1;
570570
}
571571

572-
return u128::from_ne_bytes(bytes);
572+
return i128::from_ne_bytes(bytes);
573573
};
574574

575575
// An implementation of FxHasher, although returning a u128. Probably
@@ -584,7 +584,7 @@ pub const fn hash_name(name: &str) -> u128 {
584584
hash = (hash.rotate_left(5) ^ (name[i] as u128)).wrapping_mul(K);
585585
i += 1;
586586
}
587-
hash
587+
i128::from_ne_bytes(hash.to_ne_bytes())
588588
}
589589

590590
/// Is a given source a valid instance of `Dst`?

zerocopy-derive/src/enum.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -301,10 +301,10 @@ pub(crate) fn derive_is_bit_valid(
301301
// `___ZerocopyRawEnum` has the same layout as `Self`.
302302
let slf = unsafe { slf.cast::<___ZerocopyRawEnum #ty_generics>() };
303303

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) }>()
304+
slf.project::<_, { #zerocopy_crate::STRUCT_VARIANT_ID }, { #zerocopy_crate::ident_id!(variants) }>()
305+
.project::<_, { #zerocopy_crate::UNION_VARIANT_ID }, { #zerocopy_crate::ident_id!(#variants_union_field_ident) }>()
306+
.project::<_, { #zerocopy_crate::STRUCT_VARIANT_ID }, { #zerocopy_crate::ident_id!(value) }>()
307+
.project::<_, { #zerocopy_crate::STRUCT_VARIANT_ID }, { #zerocopy_crate::ident_id!(#variant_struct_field_index) }>()
308308
}
309309
})
310310
.build()
@@ -455,7 +455,7 @@ pub(crate) fn derive_is_bit_valid(
455455
// overall struct.
456456
let project = #zerocopy_crate::pointer::PtrInner::project::<
457457
_,
458-
0,
458+
{ #zerocopy_crate::STRUCT_VARIANT_ID },
459459
{ #zerocopy_crate::ident_id!(variants) }
460460
>;
461461
let variants = unsafe {

zerocopy-derive/src/lib.rs

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -758,14 +758,19 @@ fn derive_has_field_struct_union(
758758
)
759759
});
760760

761-
let has_fields = fields.iter().map(|(_, ident, ty)| {
761+
let variant_id: Box<Expr> = match &ast.data {
762+
Data::Struct(_) => parse_quote!({ #zerocopy_crate::STRUCT_VARIANT_ID }),
763+
Data::Union(_) => parse_quote!({ #zerocopy_crate::UNION_VARIANT_ID }),
764+
_ => unreachable!(),
765+
};
766+
767+
let has_fields = fields.iter().map(move |(_, ident, ty)| {
762768
let field_token = Ident::new(&format!("ẕ{}", ident), ident.span());
763769
ImplBlockBuilder::new(
764770
ast,
765771
data,
766772
Trait::HasField {
767-
// Use `0` to denote the sole 'variant' of structs and unions.
768-
variant_id: parse_quote!({ #zerocopy_crate::ident_id!(0) }),
773+
variant_id: variant_id.clone(),
769774
field: parse_quote!(#field_token),
770775
field_id: parse_quote!({ #zerocopy_crate::ident_id!(#ident) }),
771776
},
@@ -844,7 +849,11 @@ fn derive_try_from_bytes_struct(
844849
use #zerocopy_crate::pointer::PtrInner;
845850

846851
true #(&& {
847-
let project = <Self as #zerocopy_crate::HasField<_, 0, { #zerocopy_crate::ident_id!(#field_names) }>>::project;
852+
let project = <Self as #zerocopy_crate::HasField<
853+
_,
854+
{ #zerocopy_crate::STRUCT_VARIANT_ID },
855+
{ #zerocopy_crate::ident_id!(#field_names) }
856+
>>::project;
848857
// SAFETY:
849858
// - `project` is a field projection, and so it
850859
// addresses a subset of the bytes addressed by `slf`
@@ -905,7 +914,11 @@ fn derive_try_from_bytes_union(
905914
use #zerocopy_crate::pointer::PtrInner;
906915

907916
false #(|| {
908-
let project = <Self as #zerocopy_crate::HasField<_, 0, { #zerocopy_crate::ident_id!(#field_names) }>>::project;
917+
let project = <Self as #zerocopy_crate::HasField<
918+
_,
919+
{ #zerocopy_crate::UNION_VARIANT_ID },
920+
{ #zerocopy_crate::ident_id!(#field_names) }
921+
>>::project;
909922
// SAFETY:
910923
// - `project` is a field projection, and so it
911924
// addresses a subset of the bytes addressed by `slf`

0 commit comments

Comments
 (0)