Skip to content

Commit b44135c

Browse files
committed
[derive] Allow TryFromBytes on non-Immutable unions
Makes progress on #5 Closes #1831 Closes #1832 gherrit-pr-id: G27dd847426e3572e87b20bd64642c9fcb0e51ec8
1 parent 09a0c88 commit b44135c

6 files changed

Lines changed: 44 additions & 18 deletions

File tree

zerocopy-derive/src/derive/from_bytes.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,7 @@ fn derive_from_zeros_enum(ctx: &Ctx, enm: &DataEnum) -> Result<TokenStream, Erro
159159
.build())
160160
}
161161
fn derive_from_zeros_union(ctx: &Ctx, unn: &DataUnion) -> TokenStream {
162-
// FIXME(#5): Remove the `Immutable` bound. It's only necessary for
163-
// compatibility with `derive(TryFromBytes)` on unions; not for soundness.
164-
let field_type_trait_bounds =
165-
FieldBounds::All(&[TraitBound::Slf, TraitBound::Other(Trait::Immutable)]);
162+
let field_type_trait_bounds = FieldBounds::All(&[TraitBound::Slf]);
166163
ImplBlockBuilder::new(ctx, unn, Trait::FromZeros, field_type_trait_bounds).build()
167164
}
168165
fn derive_from_bytes_struct(ctx: &Ctx, strct: &DataStruct) -> TokenStream {
@@ -186,9 +183,6 @@ fn derive_from_bytes_enum(ctx: &Ctx, enm: &DataEnum) -> Result<TokenStream, Erro
186183
Ok(ImplBlockBuilder::new(ctx, enm, Trait::FromBytes, FieldBounds::ALL_SELF).build())
187184
}
188185
fn derive_from_bytes_union(ctx: &Ctx, unn: &DataUnion) -> TokenStream {
189-
// FIXME(#5): Remove the `Immutable` bound. It's only necessary for
190-
// compatibility with `derive(TryFromBytes)` on unions; not for soundness.
191-
let field_type_trait_bounds =
192-
FieldBounds::All(&[TraitBound::Slf, TraitBound::Other(Trait::Immutable)]);
186+
let field_type_trait_bounds = FieldBounds::All(&[TraitBound::Slf]);
193187
ImplBlockBuilder::new(ctx, unn, Trait::FromBytes, field_type_trait_bounds).build()
194188
}

zerocopy-derive/src/derive/try_from_bytes.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -584,9 +584,7 @@ fn derive_try_from_bytes_struct(
584584
.build())
585585
}
586586
fn derive_try_from_bytes_union(ctx: &Ctx, unn: &DataUnion, top_level: Trait) -> TokenStream {
587-
// FIXME(#5): Remove the `Immutable` bound.
588-
let field_type_trait_bounds =
589-
FieldBounds::All(&[TraitBound::Slf, TraitBound::Other(Trait::Immutable)]);
587+
let field_type_trait_bounds = FieldBounds::All(&[TraitBound::Slf]);
590588

591589
let zerocopy_crate = &ctx.zerocopy_crate;
592590
let variant_id: Box<Expr> = {
@@ -615,10 +613,9 @@ fn derive_try_from_bytes_union(ctx: &Ctx, unn: &DataUnion, top_level: Trait) ->
615613
) -> #core::primitive::bool {
616614
false #(|| {
617615
// SAFETY:
618-
// - Since `Self: Immutable` is enforced by
619-
// `self_type_trait_bounds`, neither `*slf` nor the
620-
// returned pointer's referent permit interior
621-
// mutation.
616+
// - Since `ReadOnly<Self>: Immutable` unconditionally,
617+
// neither `*slf` nor the returned pointer's referent
618+
// permit interior mutation.
622619
// - Both source and destination validity are
623620
// `Initialized`, which is always a sound
624621
// transmutation.

zerocopy-derive/src/output_tests/expected/from_bytes_union.expected.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
const _: () = {
1414
unsafe impl ::zerocopy::TryFromBytes for Foo
1515
where
16-
u8: ::zerocopy::TryFromBytes + ::zerocopy::Immutable,
16+
u8: ::zerocopy::TryFromBytes,
1717
{
1818
fn only_derive_is_allowed_to_implement_this_trait() {}
1919
fn is_bit_valid(
@@ -100,7 +100,7 @@ const _: () = {
100100
const _: () = {
101101
unsafe impl ::zerocopy::FromZeros for Foo
102102
where
103-
u8: ::zerocopy::FromZeros + ::zerocopy::Immutable,
103+
u8: ::zerocopy::FromZeros,
104104
{
105105
fn only_derive_is_allowed_to_implement_this_trait() {}
106106
}
@@ -120,7 +120,7 @@ const _: () = {
120120
const _: () = {
121121
unsafe impl ::zerocopy::FromBytes for Foo
122122
where
123-
u8: ::zerocopy::FromBytes + ::zerocopy::Immutable,
123+
u8: ::zerocopy::FromBytes,
124124
{
125125
fn only_derive_is_allowed_to_implement_this_trait() {}
126126
}

zerocopy-derive/tests/union_from_bytes.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,13 @@ where
7777

7878
util_assert_impl_all!(WithParams<'static, 'static, u8, 42>: imp::FromBytes);
7979
test_trivial_is_bit_valid!(WithParams<'static, 'static, u8, 42> => test_with_params_trivial_is_bit_valid);
80+
81+
#[derive(imp::FromBytes)]
82+
#[zerocopy(crate = "zerocopy_renamed")]
83+
#[repr(C)]
84+
union UnsafeCellUnion {
85+
a: imp::ManuallyDrop<imp::UnsafeCell<u8>>,
86+
}
87+
88+
util_assert_impl_all!(UnsafeCellUnion: imp::FromBytes);
89+
test_trivial_is_bit_valid!(UnsafeCellUnion => test_unsafe_cell_union_trivial_is_bit_valid);

zerocopy-derive/tests/union_from_zeros.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,3 +72,12 @@ where
7272
}
7373

7474
util_assert_impl_all!(WithParams<'static, 'static, u8, 42>: imp::FromZeros);
75+
76+
#[derive(imp::FromZeros)]
77+
#[zerocopy(crate = "zerocopy_renamed")]
78+
#[repr(C)]
79+
union UnsafeCellUnion {
80+
a: imp::ManuallyDrop<imp::UnsafeCell<u8>>,
81+
}
82+
83+
util_assert_impl_all!(UnsafeCellUnion: imp::FromZeros);

zerocopy-derive/tests/union_try_from_bytes.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,3 +122,19 @@ struct A;
122122
union B {
123123
a: A,
124124
}
125+
126+
#[derive(imp::TryFromBytes)]
127+
#[zerocopy(crate = "zerocopy_renamed")]
128+
#[repr(C)]
129+
union UnsafeCellUnion {
130+
a: imp::ManuallyDrop<imp::UnsafeCell<bool>>,
131+
}
132+
133+
util_assert_impl_all!(UnsafeCellUnion: imp::TryFromBytes);
134+
135+
#[test]
136+
fn unsafe_cell_union() {
137+
crate::util::test_is_bit_valid::<UnsafeCellUnion, _>([0u8], true);
138+
crate::util::test_is_bit_valid::<UnsafeCellUnion, _>([1u8], true);
139+
crate::util::test_is_bit_valid::<UnsafeCellUnion, _>([2u8], false);
140+
}

0 commit comments

Comments
 (0)