Skip to content

Commit

Permalink
Add some SAFETY comments, some TODO comments (#66)
Browse files Browse the repository at this point in the history 
Deny the `clippy::undocumented_unsafe_blocks` lint. Add SAFETY comments
to some unsafe code, and add `#[allow(...)]` to the rest along with TODO
comments to follow up.

This is the first step of #61.
  • Loading branch information
@joshlf
joshlf committed Aug 3, 2023
1 parent 3aeb26b commit c000fc8
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 18 deletions.
3 changes: 3 additions & 0 deletions src/byteorder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,9 @@ example of how it can be used for parsing UDP packets.

// TODO(#10): Replace this with `#[derive(AsBytes)]` once that derive
// supports type parameters.
//
// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe impl<O: ByteOrder> AsBytes for $name<O> {
fn only_derive_is_allowed_to_implement_this_trait()
where
Expand Down
82 changes: 64 additions & 18 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
clippy::style,
clippy::suspicious,
clippy::todo,
clippy::undocumented_unsafe_blocks,
clippy::unimplemented,
clippy::unnested_or_patterns,
clippy::unwrap_used,
Expand Down Expand Up @@ -143,22 +144,26 @@ mod zerocopy {
// Implements an unsafe trait for a range of container types.
macro_rules! impl_for_composite_types {
($trait:ident) => {
// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe impl<T> $trait for PhantomData<T> {
fn only_derive_is_allowed_to_implement_this_trait()
where
Self: Sized,
{
}
}
// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe impl<T: $trait> $trait for [T] {
fn only_derive_is_allowed_to_implement_this_trait()
where
Self: Sized,
{
}
}
// According to the `Wrapping` docs, "`Wrapping<T>` is guaranteed to
// have the same layout and ABI as `T`."
// SAFETY: According to the `Wrapping` docs, "`Wrapping<T>` is
// guaranteed to have the same layout and ABI as `T`."
unsafe impl<T: $trait> $trait for Wrapping<T> {
fn only_derive_is_allowed_to_implement_this_trait()
where
Expand All @@ -167,6 +172,9 @@ macro_rules! impl_for_composite_types {
}
}
// Unit type has an empty representation.
//
// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe impl $trait for () {
fn only_derive_is_allowed_to_implement_this_trait()
where
Expand All @@ -175,6 +183,9 @@ macro_rules! impl_for_composite_types {
}
}
// Constant sized array with elements implementing `$trait`.
//
// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe impl<T: $trait, const N: usize> $trait for [T; N] {
fn only_derive_is_allowed_to_implement_this_trait()
where
Expand All @@ -189,6 +200,8 @@ macro_rules! impl_for_composite_types {
macro_rules! impl_for_types {
($trait:ident, $($types:ty),* $(,)?) => (
$(
// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe impl $trait for $types {
fn only_derive_is_allowed_to_implement_this_trait() {}
}
Expand Down Expand Up @@ -339,11 +352,9 @@ pub unsafe trait FromBytes {
where
Self: Sized,
{
unsafe {
// SAFETY: `FromBytes` says all bit patterns (including zeroes) are
// legal.
mem::zeroed()
}
// SAFETY: `FromBytes` says all bit patterns (including zeroes) are
// legal.
unsafe { mem::zeroed() }
}

/// Creates a `Box<Self>` from zeroed bytes.
Expand Down Expand Up @@ -502,6 +513,8 @@ pub unsafe trait AsBytes {
/// `as_bytes` provides access to the bytes of this value as an immutable
/// byte slice.
fn as_bytes(&self) -> &[u8] {
// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe {
// Note that this method does not have a `Self: Sized` bound;
// `size_of_val` works for unsized values too.
Expand All @@ -519,6 +532,8 @@ pub unsafe trait AsBytes {
where
Self: FromBytes,
{
// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe {
// Note that this method does not have a `Self: Sized` bound;
// `size_of_val` works for unsized values too.
Expand Down Expand Up @@ -889,10 +904,10 @@ macro_rules! transmute {
}
transmute(e)
} else {
// `core::mem::transmute` ensures that the type of `e` and the type
// of this macro invocation expression have the same size. We know
// this transmute is safe thanks to the `AsBytes` and `FromBytes`
// bounds enforced by the `false` branch.
// SAFETY: `core::mem::transmute` ensures that the type of `e` and
// the type of this macro invocation expression have the same size.
// We know this transmute is safe thanks to the `AsBytes` and
// `FromBytes` bounds enforced by the `false` branch.
//
// We use `$crate::__real_transmute` because we know it will always
// be available for crates which are using the 2015 edition of Rust.
Expand Down Expand Up @@ -1613,7 +1628,11 @@ where
/// and no mutable references to the same memory may be constructed during
/// `'a`.
unsafe fn deref_helper<'a>(&self) -> &'a T {
unsafe { &*self.0.as_ptr().cast::<T>() }
// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe {
&*self.0.as_ptr().cast::<T>()
}
}
}

Expand All @@ -1634,7 +1653,11 @@ where
/// and no other references - mutable or immutable - to the same memory may
/// be constructed during `'a`.
unsafe fn deref_mut_helper<'a>(&mut self) -> &'a mut T {
unsafe { &mut *self.0.as_mut_ptr().cast::<T>() }
// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe {
&mut *self.0.as_mut_ptr().cast::<T>()
}
}
}

Expand All @@ -1659,7 +1682,11 @@ where
debug_assert_eq!(len % elem_size, 0);
len / elem_size
};
unsafe { slice::from_raw_parts(self.0.as_ptr().cast::<T>(), elems) }
// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe {
slice::from_raw_parts(self.0.as_ptr().cast::<T>(), elems)
}
}
}

Expand All @@ -1685,7 +1712,11 @@ where
debug_assert_eq!(len % elem_size, 0);
len / elem_size
};
unsafe { slice::from_raw_parts_mut(self.0.as_mut_ptr().cast::<T>(), elems) }
// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe {
slice::from_raw_parts_mut(self.0.as_mut_ptr().cast::<T>(), elems)
}
}
}

Expand Down Expand Up @@ -2032,32 +2063,48 @@ pub unsafe trait ByteSliceMut: ByteSlice + DerefMut {
}
}

// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe impl<'a> ByteSlice for &'a [u8] {
#[inline]
fn split_at(self, mid: usize) -> (Self, Self) {
<[u8]>::split_at(self, mid)
}
}

// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe impl<'a> ByteSlice for &'a mut [u8] {
#[inline]
fn split_at(self, mid: usize) -> (Self, Self) {
<[u8]>::split_at_mut(self, mid)
}
}

// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe impl<'a> ByteSlice for Ref<'a, [u8]> {
#[inline]
fn split_at(self, mid: usize) -> (Self, Self) {
Ref::map_split(self, |slice| <[u8]>::split_at(slice, mid))
}
}

// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe impl<'a> ByteSlice for RefMut<'a, [u8]> {
#[inline]
fn split_at(self, mid: usize) -> (Self, Self) {
RefMut::map_split(self, |slice| <[u8]>::split_at_mut(slice, mid))
}
}

// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe impl<'a> ByteSliceMut for &'a mut [u8] {}

// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe impl<'a> ByteSliceMut for RefMut<'a, [u8]> {}

#[cfg(feature = "alloc")]
Expand Down Expand Up @@ -2326,7 +2373,7 @@ pub use alloc_support::*;
mod tests {
#![allow(clippy::unreadable_literal)]

use core::ops::Deref;
use core::{convert::TryInto, ops::Deref};

use super::*;

Expand All @@ -2346,8 +2393,7 @@ mod tests {

// Converts a `u64` to bytes using this platform's endianness.
fn u64_to_bytes(u: u64) -> [u8; 8] {
let u: *const u64 = &u;
unsafe { ptr::read(u.cast::<[u8; 8]>()) }
U64::<NativeEndian>::new(u).as_bytes().try_into().unwrap()
}

#[test]
Expand Down

0 comments on commit c000fc8

Please sign in to comment.