[derive] Allow TryFromBytes on non-Immutable unions

Makes progress on #5
Closes #1831
Closes #1832

gherrit-pr-id: G27dd847426e3572e87b20bd64642c9fcb0e51ec8

Author: joshlf

zerocopy-derive/src/lib.rs

@@ -922,16 +922,14 @@ fn derive_try_from_bytes_struct(
 }
 
 /// A union is `TryFromBytes` if:
-/// - all of its fields are `TryFromBytes` and `Immutable`
+/// - all of its fields are `TryFromBytes`
 fn derive_try_from_bytes_union(
     ast: &DeriveInput,
     unn: &DataUnion,
     top_level: Trait,
     zerocopy_crate: &Path,
 ) -> TokenStream {
-    // FIXME(#5): Remove the `Immutable` bound.
-    let field_type_trait_bounds =
-        FieldBounds::All(&[TraitBound::Slf, TraitBound::Other(Trait::Immutable)]);
+    let field_type_trait_bounds = FieldBounds::All(&[TraitBound::Slf]);
     let extras =
         try_gen_trivial_is_bit_valid(ast, top_level, zerocopy_crate).unwrap_or_else(|| {
             let fields = unn.fields();
@@ -950,10 +948,9 @@ fn derive_try_from_bytes_union(
 
                     false #(|| {
                         // SAFETY:
-                        // - Since `Self: Immutable` is enforced by
-                        //   `self_type_trait_bounds`, neither `*slf` nor the
-                        //   returned pointer's referent contain any
-                        //   `UnsafeCell`s
+                        // - Since `ReadOnly<Self>: Immutable` unconditionally,
+                        //   neither `*slf` nor the returned pointer's referent
+                        //   permit interior mutation.
                         // - Both source and destination validity are
                         //   `Initialized`, which is always a sound
                         //   transmutation.
@@ -1250,16 +1247,13 @@ fn derive_from_zeros_enum(
 }
 
 /// Unions are `FromZeros` if
-/// - all fields are `FromZeros` and `Immutable`
+/// - all fields are `FromZeros`
 fn derive_from_zeros_union(
     ast: &DeriveInput,
     unn: &DataUnion,
     zerocopy_crate: &Path,
 ) -> TokenStream {
-    // FIXME(#5): Remove the `Immutable` bound. It's only necessary for
-    // compatibility with `derive(TryFromBytes)` on unions; not for soundness.
-    let field_type_trait_bounds =
-        FieldBounds::All(&[TraitBound::Slf, TraitBound::Other(Trait::Immutable)]);
+    let field_type_trait_bounds = FieldBounds::All(&[TraitBound::Slf]);
     ImplBlockBuilder::new(ast, unn, Trait::FromZeros, field_type_trait_bounds, zerocopy_crate)
         .build()
 }
@@ -1330,16 +1324,13 @@ fn enum_size_from_repr(repr: &EnumRepr) -> Result<usize, Error> {
 }
 
 /// Unions are `FromBytes` if
-/// - all fields are `FromBytes` and `Immutable`
+/// - all fields are `FromBytes`
 fn derive_from_bytes_union(
     ast: &DeriveInput,
     unn: &DataUnion,
     zerocopy_crate: &Path,
 ) -> TokenStream {
-    // FIXME(#5): Remove the `Immutable` bound. It's only necessary for
-    // compatibility with `derive(TryFromBytes)` on unions; not for soundness.
-    let field_type_trait_bounds =
-        FieldBounds::All(&[TraitBound::Slf, TraitBound::Other(Trait::Immutable)]);
+    let field_type_trait_bounds = FieldBounds::All(&[TraitBound::Slf]);
     ImplBlockBuilder::new(ast, unn, Trait::FromBytes, field_type_trait_bounds, zerocopy_crate)
         .build()
 }

zerocopy-derive/tests/union_from_bytes.rs

@@ -72,3 +72,12 @@ where
 
 util_assert_impl_all!(WithParams<'static, 'static, u8, 42>: imp::FromBytes);
 test_trivial_is_bit_valid!(WithParams<'static, 'static, u8, 42> => test_with_params_trivial_is_bit_valid);
+
+#[derive(imp::FromBytes)]
+#[repr(C)]
+union UnsafeCellUnion {
+    a: imp::ManuallyDrop<imp::UnsafeCell<u8>>,
+}
+
+util_assert_impl_all!(UnsafeCellUnion: imp::FromBytes);
+test_trivial_is_bit_valid!(UnsafeCellUnion => test_unsafe_cell_union_trivial_is_bit_valid);

zerocopy-derive/tests/union_from_zeros.rs

@@ -67,3 +67,11 @@ where
 }
 
 util_assert_impl_all!(WithParams<'static, 'static, u8, 42>: imp::FromZeros);
+
+#[derive(imp::FromZeros)]
+#[repr(C)]
+union UnsafeCellUnion {
+    a: imp::ManuallyDrop<imp::UnsafeCell<u8>>,
+}
+
+util_assert_impl_all!(UnsafeCellUnion: imp::FromZeros);

zerocopy-derive/tests/union_try_from_bytes.rs

@@ -114,3 +114,18 @@ struct A;
 union B {
     a: A,
 }
+
+#[derive(imp::TryFromBytes)]
+#[repr(C)]
+union UnsafeCellUnion {
+    a: imp::ManuallyDrop<imp::UnsafeCell<bool>>,
+}
+
+util_assert_impl_all!(UnsafeCellUnion: imp::TryFromBytes);
+
+#[test]
+fn unsafe_cell_union() {
+    crate::util::test_is_bit_valid::<UnsafeCellUnion>([0u8], true);
+    crate::util::test_is_bit_valid::<UnsafeCellUnion>([1u8], true);
+    crate::util::test_is_bit_valid::<UnsafeCellUnion>([2u8], false);
+}