Repository: Lokathor/bytemuck
Pull request #15 (2020-01-10 19:37:48)

Add TransparentWrapper marker trait
That transmute_copy bothers me, and I'd like have a "do sizes match" sanity check for broken impls... But supporting &[T] to &SomeStruct([T]) is too useful to give up IMO.

Unsure on where wrap_ref/wrap_mut should go. I don't really like "hey don't implement this" functions on the trait, but maybe they'd be better there.


Patch:
diff --git a/src/lib.rs b/src/lib.rs
index 88336bc..7cfb450 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -66,6 +66,9 @@ pub use pod::*;
 mod contiguous;
 pub use contiguous::*;
 
+mod transparent;
+pub use transparent::*;
+
 // Used from macros to ensure we aren't using some locally defined name and
 // actually are referencing libcore. This also would allow pre-2018 edition
 // crates to use our macros, but I'm not sure how important that is.
diff --git a/src/transparent.rs b/src/transparent.rs
new file mode 100644
index 0000000..b77a870
--- /dev/null
+++ b/src/transparent.rs
@@ -0,0 +1,133 @@
+use super::*;
+
+/// A trait which indicates that a type is a `repr(transparent)` wrapper around
+/// the `Wrapped` value.
+///
+/// This allows safely creating references to `T` from those to the `Wrapped`
+/// type, using the `wrap_ref` and `wrap_mut` functions.
+///
+/// # Safety
+///
+/// The safety contract of `TransparentWrapper` is relatively simple:
+///
+/// For a given `Wrapper` which implements `TransparentWrapper<Wrapped>`:
+///
+/// 1. Wrapper must be a `#[repr(transparent)]` wrapper around `Wrapped`. This
+///    either means that it must be a `#[repr(transparent)]` struct which
+///    contains a either a field of type `Wrapped` (or a field of some other
+///    transparent wrapper for `Wrapped`) as the only non-ZST field.
+///
+/// 2. Any fields *other* than the `Wrapped` field must be trivially
+///    constructable ZSTs, for example `PhantomData`, `PhantomPinned`, etc.
+///
+/// 3. The `Wrapper` may not impose additional alignment requirements over
+///    `Wrapped`.
+///     - Note: this is currently guaranteed by repr(transparent), but there
+///       have been discussions of lifting it, so it's stated here explictly.
+///
+/// 4. The `wrap_ref` and `wrap_mut` functions on `TransparentWrapper` may not
+///    be overridden.
+///
+/// ## Caveats
+///
+/// If the wrapper imposes additional constraints upon the wrapped type which
+/// are required for safety, it's responsible for ensuring those still hold --
+/// this generally requires preventing access to instances of the wrapped type,
+/// as implementing `TransparentWrapper<U> for T` means anybody can call
+/// `T::cast_ref(any_instance_of_u)`.
+///
+/// For example, it would be invalid to implement TransparentWrapper for `str`
+/// to implement `TransparentWrapper` around `[u8]` because of this.
+///
+/// # Examples
+///
+/// ## Basic
+///
+/// ```
+/// use bytemuck::TransparentWrapper;
+/// # #[derive(Default)]
+/// # struct SomeStruct(u32);
+///
+/// #[repr(transparent)]
+/// struct MyWrapper(SomeStruct);
+///
+/// unsafe impl TransparentWrapper<SomeStruct> for MyWrapper {}
+///
+/// // interpret a reference to &SomeStruct as a &MyWrapper
+/// let thing = SomeStruct::default();
+/// let wrapped_ref: &MyWrapper = MyWrapper::wrap_ref(&thing);
+///
+/// // Works with &mut too.
+/// let mut mut_thing = SomeStruct::default();
+/// let wrapped_mut: &mut MyWrapper = MyWrapper::wrap_mut(&mut mut_thing);
+///
+/// # let _ = (wrapped_ref, wrapped_mut); // silence warnings
+/// ```
+///
+/// ## Use with dynamically sized types
+///
+/// ```
+/// use bytemuck::TransparentWrapper;
+///
+/// #[repr(transparent)]
+/// struct Slice<T>([T]);
+///
+/// unsafe impl<T> TransparentWrapper<[T]> for Slice<T> {}
+///
+/// let s = Slice::wrap_ref(&[1u32, 2, 3]);
+/// assert_eq!(&s.0, &[1, 2, 3]);
+///
+/// let mut buf = [1, 2, 3u8];
+/// let sm = Slice::wrap_mut(&mut buf);
+/// ```
+pub unsafe trait TransparentWrapper<Wrapped: ?Sized> {
+  /// Convert a reference to a wrapped type into a reference to the wrapper.
+  ///
+  /// This is a trait method so that you can write `MyType::wrap_ref(...)` in
+  /// your code. It is part of the safety contract for this trait that if you
+  /// implement `TransparentWrapper<_>` for your type you **must not** override
+  /// this method.
+  #[inline]
+  fn wrap_ref(s: &Wrapped) -> &Self {
+    unsafe {
+      assert!(size_of::<*const Wrapped>() == size_of::<*const Self>());
+      // Using a pointer cast doesn't work here because rustc can't tell that the
+      // vtables match (if we lifted the ?Sized restriction, this would go away),
+      // and transmute doesn't work for the usual reasons it doesn't work inside
+      // generic functions.
+      //
+      // SAFETY: The unsafe contract requires that these have identical
+      // representations. Using this transmute_copy instead of transmute here is
+      // annoying, but is required as `Self` and `Wrapped` have unspecified
+      // sizes still.
+      let wrapped_ptr = s as *const Wrapped;
+      let wrapper_ptr: *const Self = transmute_copy(&wrapped_ptr);
+      &*wrapper_ptr
+    }
+  }
+
+  /// Convert a mut reference to a wrapped type into a mut reference to the
+  /// wrapper.
+  ///
+  /// This is a trait method so that you can write `MyType::wrap_mut(...)` in
+  /// your code. It is part of the safety contract for this trait that if you implement
+  /// `TransparentWrapper<_>` for your type you **must not** override this method.
+  #[inline]
+  fn wrap_mut(s: &mut Wrapped) -> &mut Self {
+    unsafe {
+      assert!(size_of::<*mut Wrapped>() == size_of::<*mut Self>());
+      // Using a pointer cast doesn't work here because rustc can't tell that the
+      // vtables match (if we lifted the ?Sized restriction, this would go away),
+      // and transmute doesn't work for the usual reasons it doesn't work inside
+      // generic functions.
+      //
+      // SAFETY: The unsafe contract requires that these have identical
+      // representations. Using this transmute_copy instead of transmute here is
+      // annoying, but is required as `Self` and `Wrapped` have unspecified
+      // sizes still.
+      let wrapped_ptr = s as *mut Wrapped;
+      let wrapper_ptr: *mut Self = transmute_copy(&wrapped_ptr);
+      &mut *wrapper_ptr
+    }
+  }
+}


Test patch:
diff --git a/tests/doc_tests.rs b/tests/doc_tests.rs
new file mode 100644
index 0000000..e5a80db
--- /dev/null
+++ b/tests/doc_tests.rs
@@ -0,0 +1,121 @@
+//! Cargo miri doesn't run doctests yet, so we duplicate these here. It's
+//! probably not that important to sweat keeping these perfectly up to date, but
+//! we should try to catch the cases where the primary tests are doctests.
+use bytemuck::*;
+
+// Miri doesn't run on doctests, so... copypaste to the rescue.
+#[test]
+fn test_transparent_slice() {
+  #[repr(transparent)]
+  struct Slice<T>([T]);
+
+  unsafe impl<T> TransparentWrapper<[T]> for Slice<T> {}
+
+  let s = Slice::wrap_ref(&[1u32, 2, 3]);
+  assert_eq!(&s.0, &[1, 2, 3]);
+
+  let mut buf = [1, 2, 3u8];
+  let _sm = Slice::wrap_mut(&mut buf);
+}
+
+#[test]
+fn test_transparent_basic() {
+  #[derive(Default)]
+  struct SomeStruct(u32);
+
+  #[repr(transparent)]
+  struct MyWrapper(SomeStruct);
+
+  unsafe impl TransparentWrapper<SomeStruct> for MyWrapper {}
+
+  // interpret a reference to &SomeStruct as a &MyWrapper
+  let thing = SomeStruct::default();
+  let wrapped_ref: &MyWrapper = MyWrapper::wrap_ref(&thing);
+
+  // Works with &mut too.
+  let mut mut_thing = SomeStruct::default();
+  let wrapped_mut: &mut MyWrapper = MyWrapper::wrap_mut(&mut mut_thing);
+  let _ = (wrapped_ref, wrapped_mut);
+}
+
+// Work around miri not running doctests
+#[test]
+fn test_contiguous_doc() {
+  #[repr(u8)]
+  #[derive(Debug, Copy, Clone, PartialEq)]
+  enum Foo {
+    A = 0,
+    B = 1,
+    C = 2,
+    D = 3,
+    E = 4,
+  }
+  unsafe impl Contiguous for Foo {
+    type Int = u8;
+    const MIN_VALUE: u8 = Foo::A as u8;
+    const MAX_VALUE: u8 = Foo::E as u8;
+  }
+
+  assert_eq!(Foo::from_integer(3).unwrap(), Foo::D);
+  assert_eq!(Foo::from_integer(8), None);
+  assert_eq!(Foo::C.into_integer(), 2);
+  assert_eq!(Foo::B.into_integer(), Foo::B as u8);
+}
+
+#[test]
+fn test_offsetof_vertex() {
+  #[repr(C)]
+  struct Vertex {
+    pos: [f32; 2],
+    uv: [u16; 2],
+    color: [u8; 4],
+  }
+  unsafe impl Zeroable for Vertex {}
+
+  let pos = offset_of!(Zeroable::zeroed(), Vertex, pos);
+  let uv = offset_of!(Zeroable::zeroed(), Vertex, uv);
+  let color = offset_of!(Zeroable::zeroed(), Vertex, color);
+
+  assert_eq!(pos, 0);
+  assert_eq!(uv, 8);
+  assert_eq!(color, 12);
+}
+
+#[test]
+fn test_offsetof_nonpod() {
+  #[derive(Default)]
+  struct Foo {
+    a: u8,
+    b: &'static str,
+    c: i32,
+  }
+
+  let a_offset = offset_of!(Default::default(), Foo, a);
+  let b_offset = offset_of!(Default::default(), Foo, b);
+  let c_offset = offset_of!(Default::default(), Foo, c);
+
+  assert_ne!(a_offset, b_offset);
+  assert_ne!(b_offset, c_offset);
+  // We can't check against hardcoded values for a repr(Rust) type,
+  // but prove to ourself this way.
+
+  let foo = Foo::default();
+  // Note: offsets are in bytes.
+  let as_bytes = &foo as *const _ as *const u8;
+
+  // we're using wrapping_offset here becasue it's not worth
+  // the unsafe block, but it would be valid to use `add` instead,
+  // as it cannot overflow.
+  assert_eq!(
+    &foo.a as *const _ as usize,
+    as_bytes.wrapping_add(a_offset) as usize
+  );
+  assert_eq!(
+    &foo.b as *const _ as usize,
+    as_bytes.wrapping_add(b_offset) as usize
+  );
+  assert_eq!(
+    &foo.c as *const _ as usize,
+    as_bytes.wrapping_add(c_offset) as usize
+  );
+}
diff --git a/tests/std_tests.rs b/tests/std_tests.rs
new file mode 100644
index 0000000..0e0fb48
--- /dev/null
+++ b/tests/std_tests.rs
@@ -0,0 +1,29 @@
+//! The integration tests seem to always have `std` linked, so things that would
+//! depend on that can go here.
+
+use bytemuck::*;
+
+#[test]
+fn test_transparent_vtabled() {
+  use core::fmt::Display;
+
+  #[repr(transparent)]
+  struct DisplayTraitObj(dyn Display);
+
+  unsafe impl TransparentWrapper<dyn Display> for DisplayTraitObj {}
+
+  impl Display for DisplayTraitObj {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+      self.0.fmt(f)
+    }
+  }
+
+  let v = DisplayTraitObj::wrap_ref(&5i32);
+  let s = format!("{}", v);
+  assert_eq!(s, "5");
+
+  let mut x = 100i32;
+  let v_mut = DisplayTraitObj::wrap_mut(&mut x);
+  let s = format!("{}", v_mut);
+  assert_eq!(s, "100");
+}