Move IntoByteSlice[Mut]: Into into methods (#1299)

Remove the `Into<&[u8]>` and `Into<&mut [u8]>` bounds on `IntoByteSlice`
and `IntoByteSliceMut` and replace them with methods. This allows
`IntoByteSlice` to be implemented for `&mut [u8]` (and, as a
consequence, permits `Ref::<&mut [u8], _>::into_ref`), which was
previously barred.

Closes #1260

src/byte_slice.rs

@@ -160,6 +160,7 @@ where
 pub trait SplitByteSliceMut: SplitByteSlice + ByteSliceMut {}
 impl<B: SplitByteSlice + ByteSliceMut> SplitByteSliceMut for B {}
 
+#[allow(clippy::missing_safety_doc)] // There's a `Safety` section on `into_byte_slice`.
 /// A [`ByteSlice`] that conveys no ownership, and so can be converted into a
 /// byte slice.
 ///
@@ -169,19 +170,22 @@ impl<B: SplitByteSlice + ByteSliceMut> SplitByteSliceMut for B {}
 /// are only compatible with `ByteSlice` types without these ownership
 /// semantics.
 ///
-/// # Safety
-///
-/// Invoking `self.into()` produces a `&[u8]` with identical address and length
-/// as the slice produced by `self.deref()`. Note that this implies that the
-/// slice produced by `self.into()` is "stable" in the same sense as defined by
-/// [`ByteSlice`]'s safety invariant.
-///
-/// If `Self: Into<&mut [u8]>`, then the same stability requirement holds of
-/// `<Self as Into<&mut [u8]>>::into`.
-///
 /// [`Ref`]: core::cell::Ref
-pub unsafe trait IntoByteSlice<'a>: ByteSlice + Into<&'a [u8]> {}
+pub unsafe trait IntoByteSlice<'a>: ByteSlice {
+    /// Coverts `self` into a `&[u8]`.
+    ///
+    /// # Safety
+    ///
+    /// The returned reference has the same address and length as `self.deref()`
+    /// or `self.deref_mut()`.
+    ///
+    /// Note that, combined with the safety invariant on [`ByteSlice`], this
+    /// safety invariant implies that the returned reference is "stable" in the
+    /// sense described in the `ByteSlice` docs.
+    fn into_byte_slice(self) -> &'a [u8];
+}
 
+#[allow(clippy::missing_safety_doc)] // There's a `Safety` section on `into_byte_slice_mut`.
 /// A [`ByteSliceMut`] that conveys no ownership, and so can be converted into a
 /// mutable byte slice.
 ///
@@ -192,8 +196,19 @@ pub unsafe trait IntoByteSlice<'a>: ByteSlice + Into<&'a [u8]> {}
 /// these ownership semantics.
 ///
 /// [`RefMut`]: core::cell::RefMut
-pub trait IntoByteSliceMut<'a>: ByteSliceMut + Into<&'a mut [u8]> {}
-impl<'a, B: ByteSliceMut + Into<&'a mut [u8]>> IntoByteSliceMut<'a> for B {}
+pub unsafe trait IntoByteSliceMut<'a>: IntoByteSlice<'a> + ByteSliceMut {
+    /// Coverts `self` into a `&mut [u8]`.
+    ///
+    /// # Safety
+    ///
+    /// The returned reference has the same address and length as `self.deref()`
+    /// or `self.deref_mut()`.
+    ///
+    /// Note that, combined with the safety invariant on [`ByteSlice`], this
+    /// safety invariant implies that the returned reference is "stable" in the
+    /// sense described in the `ByteSlice` docs.
+    fn into_byte_slice_mut(self) -> &'a mut [u8];
+}
 
 // TODO(#429): Add a "SAFETY" comment and remove this `allow`.
 #[allow(clippy::undocumented_unsafe_blocks)]
@@ -218,16 +233,17 @@ unsafe impl<'a> SplitByteSlice for &'a [u8] {
     }
 }
 
-// SAFETY: The standard library impl of `From<T> for T` [1] cannot be removed
-// without being a backwards-breaking change. Thus, we can rely on it continuing
-// to exist. So long as that is the impl backing `Into<&[u8]> for &[u8]`, we
-// know (barring a truly ridiculous stdlib implementation) that this impl is
-// simply implemented as `fn into(bytes: &[u8]) -> &[u8] { bytes }` (or
-// something semantically equivalent to it). Thus, `ByteSlice`'s stability
-// guarantees are not violated by the `Into<&[u8]>` impl.
-//
-// [1] https://doc.rust-lang.org/std/convert/trait.From.html#impl-From%3CT%3E-for-T
-unsafe impl<'a> IntoByteSlice<'a> for &'a [u8] {}
+// SAFETY: See inline.
+unsafe impl<'a> IntoByteSlice<'a> for &'a [u8] {
+    #[inline(always)]
+    fn into_byte_slice(self) -> &'a [u8] {
+        // SAFETY: It would be patently insane to implement `<Deref for
+        // &[u8]>::deref` as anything other than `fn deref(&self) -> &[u8] {
+        // *self }`. Assuming this holds, then `self` is stable as required by
+        // `into_byte_slice`.
+        self
+    }
+}
 
 // TODO(#429): Add a "SAFETY" comment and remove this `allow`.
 #[allow(clippy::undocumented_unsafe_blocks)]
@@ -290,6 +306,30 @@ unsafe impl<'a> SplitByteSlice for &'a mut [u8] {
     }
 }
 
+// SAFETY: See inline.
+unsafe impl<'a> IntoByteSlice<'a> for &'a mut [u8] {
+    #[inline(always)]
+    fn into_byte_slice(self) -> &'a [u8] {
+        // SAFETY: It would be patently insane to implement `<Deref for &mut
+        // [u8]>::deref` as anything other than `fn deref(&self) -> &[u8] {
+        // *self }`. Assuming this holds, then `self` is stable as required by
+        // `into_byte_slice`.
+        self
+    }
+}
+
+// SAFETY: See inline.
+unsafe impl<'a> IntoByteSliceMut<'a> for &'a mut [u8] {
+    #[inline(always)]
+    fn into_byte_slice_mut(self) -> &'a mut [u8] {
+        // SAFETY: It would be patently insane to implement `<DerefMut for &mut
+        // [u8]>::deref` as anything other than `fn deref_mut(&mut self) -> &mut
+        // [u8] { *self }`. Assuming this holds, then `self` is stable as
+        // required by `into_byte_slice_mut`.
+        self
+    }
+}
+
 // TODO(#429): Add a "SAFETY" comment and remove this `allow`.
 #[allow(clippy::undocumented_unsafe_blocks)]
 unsafe impl<'a> ByteSlice for cell::Ref<'a, [u8]> {}

src/ref.rs

@@ -757,10 +757,10 @@ where
         let b = unsafe { self.into_byte_slice() };
 
         // PANICS: By post-condition on `into_byte_slice`, `b`'s size and
-        // alignment are valid for `T`. By invariant on `IntoByteSlice`,
-        // `b.into()` produces a byte slice with identical address and length to
-        // that produced by `b.deref()`.
-        let ptr = Ptr::from_ref(b.into())
+        // alignment are valid for `T`. By post-condition, `b.into_byte_slice()`
+        // produces a byte slice with identical address and length to that
+        // produced by `b.deref()`.
+        let ptr = Ptr::from_ref(b.into_byte_slice())
             .try_cast_into_no_leftover::<T, BecauseImmutable>(None)
             .expect("zerocopy internal error: into_ref should be infallible");
         let ptr = ptr.bikeshed_recall_valid();
@@ -787,10 +787,10 @@ where
         let b = unsafe { self.into_byte_slice_mut() };
 
         // PANICS: By post-condition on `into_byte_slice_mut`, `b`'s size and
-        // alignment are valid for `T`. By invariant on `IntoByteSliceMut`,
-        // `b.into()` produces a byte slice with identical address and length to
-        // that produced by `b.deref_mut()`.
-        let ptr = Ptr::from_mut(b.into())
+        // alignment are valid for `T`. By post-condition,
+        // `b.into_byte_slice_mut()` produces a byte slice with identical
+        // address and length to that produced by `b.deref_mut()`.
+        let ptr = Ptr::from_mut(b.into_byte_slice_mut())
             .try_cast_into_no_leftover::<T, BecauseExclusive>(None)
             .expect("zerocopy internal error: into_ref should be infallible");
         let ptr = ptr.bikeshed_recall_valid();
@@ -997,6 +997,15 @@ mod tests {
     use super::*;
     use crate::util::testutil::*;
 
+    #[test]
+    fn test_mut_slice_into_ref() {
+        // Prior to #1260/#1299, calling `into_ref` on a `&mut [u8]`-backed
+        // `Ref` was not supportd.
+        let mut buf = [0u8];
+        let r = Ref::<&mut [u8], u8>::from_bytes(&mut buf).unwrap();
+        assert_eq!(r.into_ref(), &0);
+    }
+
     #[test]
     fn test_address() {
         // Test that the `Deref` and `DerefMut` implementations return a