Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move IntoByteSlice[Mut]: Into into methods #1299

Merged
merged 1 commit into from
May 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 63 additions & 23 deletions src/byte_slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -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.
///
Expand All @@ -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)]
Expand All @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤣

// &[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)]
Expand Down Expand Up @@ -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]> {}
Expand Down
25 changes: 17 additions & 8 deletions src/ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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
Expand Down
Loading