Skip to content

Commit

Permalink
Adress feedback in google#433
Browse files Browse the repository at this point in the history
  • Loading branch information
samuelselleck committed Oct 13, 2023
1 parent fd09ac9 commit 1b27133
Showing 1 changed file with 38 additions and 30 deletions.
68 changes: 38 additions & 30 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -652,10 +652,10 @@ impl_known_layout!(const N: usize, T => [T; N]);

safety_comment! {
/// SAFETY:
// TODO(#429): Add reference to docs + quotes
/// `str` and `ManuallyDrop<[T]>` have the same representations as `[u8]`
/// and `[T]` repsectively. `str` has different bit validity than `[u8]`,
/// but that doesn't affect the soundness of this impl.
/// TODO(#429): Add references to docs and quotes.
unsafe_impl_known_layout!(#[repr([u8])] str);
unsafe_impl_known_layout!(T: ?Sized + KnownLayout => #[repr(T)] ManuallyDrop<T>);
}
Expand Down Expand Up @@ -757,13 +757,13 @@ pub unsafe trait FromZeroes {
let slf: *mut Self = self;
let len = mem::size_of_val(self);
// SAFETY:
// TODO(#429): Add reference to docs + quotes
// - `self` is guaranteed by the type system to be valid for writes of
// size `size_of_val(self)`.
// - `u8`'s alignment is 1, and thus `self` is guaranteed to be aligned
// as required by `u8`.
// - Since `Self: FromZeroes`, the all-zeroes instance is a valid
// instance of `Self.`
// TODO(#429): Add references to docs and quotes.
unsafe { ptr::write_bytes(slf.cast::<u8>(), 0, len) };
}

Expand Down Expand Up @@ -1145,7 +1145,6 @@ pub unsafe trait AsBytes {
let slf: *const Self = self;

// SAFETY:
// TODO(#429): Add reference to docs + quotes
// - `slf.cast::<u8>()` is valid for reads for `len *
// mem::size_of::<u8>()` many bytes because...
// - `slf` is the same pointer as `self`, and `self` is a reference
Expand All @@ -1164,6 +1163,7 @@ pub unsafe trait AsBytes {
// - The total size of the resulting slice is no larger than
// `isize::MAX` because no allocation produced by safe code can be
// larger than `isize::MAX`.
// TODO(#429): Add references to docs and quotes.
unsafe { slice::from_raw_parts(slf.cast::<u8>(), len) }
}

Expand Down Expand Up @@ -1199,6 +1199,7 @@ pub unsafe trait AsBytes {
// - The total size of the resulting slice is no larger than
// `isize::MAX` because no allocation produced by safe code can be
// larger than `isize::MAX`.
// TODO(#429): Add references to docs and quotes.
unsafe { slice::from_raw_parts_mut(slf.cast::<u8>(), len) }
}

Expand Down Expand Up @@ -1286,7 +1287,6 @@ safety_comment! {

safety_comment! {
/// SAFETY:
/// TODO(#429): Add quotes
/// - `FromZeroes`, `FromBytes`: all bit patterns are valid for integers [1]
/// - `AsBytes`: integers have no padding bytes [1]
/// - `Unaligned` (`u8` and `i8` only): The reference [2] specifies the size
Expand All @@ -1296,6 +1296,8 @@ safety_comment! {
/// - The only value >= 1 for which 1 is an integer multiple is 1
/// Therefore, the only possible alignment for `u8` and `i8` is 1.
///
/// TODO(#429): Add quotes from documentation.
///
/// [1] TODO(https://github.com/rust-lang/reference/issues/1291): Once the
/// reference explicitly guarantees these properties, cite it.
/// [2] https://doc.rust-lang.org/reference/type-layout.html#primitive-data-layout
Expand All @@ -1322,8 +1324,9 @@ safety_comment! {
/// - `AsBytes`: the `{f32,f64}::to_bits` methods' documentation [4,5]
/// states that they are currently equivalent to `transmute`. [3]
///
/// TODO(#429): Make these arguments more precisely in terms of the documentation
/// TODO(#429): Add quotes
/// TODO(#429):
/// - Make these arguments more precisely in terms of the documentation.
/// - Add quotes from documentation.
///
/// [1] https://doc.rust-lang.org/nightly/std/primitive.f32.html#method.from_bits
/// [2] https://doc.rust-lang.org/nightly/std/primitive.f64.html#method.from_bits
Expand All @@ -1337,11 +1340,12 @@ safety_comment! {

safety_comment! {
/// SAFETY:
/// - `FromZeroes`: Valid since "The value false has the bit pattern 0x00" [1].
/// - `AsBytes`: Since "the boolean type has a size and
/// alignment of 1 each" and "The value false has the bit pattern 0x00 and
/// the value true has the bit pattern 0x01" [1]. Note that it is undefined
/// behavior for an object with the boolean type to have any other bit pattern.
/// - `FromZeroes`: Valid since "The value false has the bit pattern
/// 0x00" [1].
/// - `AsBytes`: Since "the boolean type has a size and alignment of 1 each"
/// and "The value false has the bit pattern 0x00 and the value true has
/// the bit pattern 0x01" [1]. Thus, the only byte of the bool is always
/// initialized.
/// - `Unaligned`: Per the reference [1], "[a]n object with the boolean type
/// has a size and alignment of 1 each."
///
Expand All @@ -1351,28 +1355,28 @@ safety_comment! {
}
safety_comment! {
/// SAFETY:
/// - `FromZeroes`: Per reference [1], "A value of type char is
/// a Unicode scalar value (i.e. a code point that is not a surrogate),
/// represented as a 32-bit unsigned word in the 0x0000 to 0xD7FF or 0xE000
/// to 0x10FFFF range" which contains 0x0000.
/// - `AsBytes`: `char` is per reference [1] "represented as a 32-bit unsigned
/// word" (`u32`)
/// which is `AsBytes`. Note that unlike `u32`, not all bit patterns
/// are valid for `char`.
/// - `FromZeroes`: Per reference [1], "A value of type char is a Unicode
/// scalar value (i.e. a code point that is not a surrogate), represented
/// as a 32-bit unsigned word in the 0x0000 to 0xD7FF or 0xE000 to
/// 0x10FFFF range" which contains 0x0000.
/// - `AsBytes`: `char` is per reference [1] "represented as a 32-bit
/// unsigned word" (`u32`) which is `AsBytes`. Note that unlike `u32`, not
/// all bit patterns are valid for `char`.
///
/// [1] https://doc.rust-lang.org/reference/types/textual.html
unsafe_impl!(char: FromZeroes, AsBytes);
}
safety_comment! {
/// SAFETY:
/// TODO(#429): Add quotes
/// - `FromZeroes`, `AsBytes`, `Unaligned`: Per the reference [1], `str` has
/// the same layout as `[u8]`, and `[u8]` is `FromZeroes`, `AsBytes`, and
/// `Unaligned`.
/// - `FromZeroes`, `AsBytes`, `Unaligned`: Per the reference [1], `str`
/// has the same layout as `[u8]`, and `[u8]` is `FromZeroes`, `AsBytes`,
/// and `Unaligned`.
///
/// Note that we don't `assert_unaligned!(str)` because `assert_unaligned!`
/// uses `align_of`, which only works for `Sized` types.
///
/// TODO(#429): Add quotes from documentation.
///
/// [1] https://doc.rust-lang.org/reference/type-layout.html#str-layout
unsafe_impl!(str: FromZeroes, AsBytes, Unaligned);
}
Expand All @@ -1381,7 +1385,6 @@ safety_comment! {
// `NonZeroXxx` is `AsBytes`, but not `FromZeroes` or `FromBytes`.
//
/// SAFETY:
/// TODO(#429): Add quotes
/// - `AsBytes`: `NonZeroXxx` has the same layout as its associated
/// primitive. Since it is the same size, this guarantees it has no
/// padding - integers have no padding, and there's no room for padding
Expand All @@ -1396,6 +1399,8 @@ safety_comment! {
/// be 0 bytes, which means that they must be 1 byte. The only valid
/// alignment for a 1-byte type is 1.
///
/// TODO(#429): Add quotes from documentation.
///
/// [1] https://doc.rust-lang.org/stable/std/num/struct.NonZeroU8.html
/// [2] https://doc.rust-lang.org/stable/std/num/struct.NonZeroI8.html
/// TODO(https://github.com/rust-lang/rust/pull/104082): Cite documentation
Expand All @@ -1416,7 +1421,6 @@ safety_comment! {
}
safety_comment! {
/// SAFETY:
/// TODO(#429): Add quotes
/// - `FromZeroes`, `FromBytes`, `AsBytes`: The Rust compiler reuses `0`
/// value to represent `None`, so `size_of::<Option<NonZeroXxx>>() ==
/// size_of::<xxx>()`; see `NonZeroXxx` documentation.
Expand All @@ -1427,6 +1431,8 @@ safety_comment! {
/// unthinkable that that would ever change. The only valid alignment for
/// a 1-byte type is 1.
///
/// TODO(#429): Add quotes from documentation.
///
/// [1] https://doc.rust-lang.org/stable/std/num/struct.NonZeroU8.html
/// [2] https://doc.rust-lang.org/stable/std/num/struct.NonZeroI8.html
///
Expand Down Expand Up @@ -1470,14 +1476,16 @@ safety_comment! {
}
safety_comment! {
/// SAFETY:
/// TODO(#429): Add quotes
/// `Wrapping<T>` is guaranteed by its docs [1] to have the same layout as
/// `T`. Also, `Wrapping<T>` is `#[repr(transparent)]`, and has a single
/// field, which is `pub`. Per the reference [2], this means that the
/// `#[repr(transparent)]` attribute is "considered part of the public ABI".
///
/// references nightly?:
/// TODO(#429): Add quotes from documentation.
///
/// [1] TODO(https://doc.rust-lang.org/nightly/core/num/struct.Wrapping.html#layout-1):
/// Reference this documentation once it's available on stable.
///
/// [2] https://doc.rust-lang.org/nomicon/other-reprs.html#reprtransparent
unsafe_impl!(T: FromZeroes => FromZeroes for Wrapping<T>);
unsafe_impl!(T: FromBytes => FromBytes for Wrapping<T>);
Expand All @@ -1496,8 +1504,8 @@ safety_comment! {
/// Thus, we require `T: FromZeroes` and `T: FromBytes` in order to ensure
/// that `T` - and thus `MaybeUninit<T>` - contains to `UnsafeCell`s.
/// Thus, requiring that `T` implement each of these traits is sufficient
/// - `Unaligned`: "MaybeUninit<T> is guaranteed to have the same size, alignment,
/// and ABI as T" [1]
/// - `Unaligned`: "MaybeUninit<T> is guaranteed to have the same size,
/// alignment, and ABI as T" [1]
///
/// [1] https://doc.rust-lang.org/stable/core/mem/union.MaybeUninit.html#layout-1
///
Expand All @@ -1512,7 +1520,6 @@ safety_comment! {
}
safety_comment! {
/// SAFETY:
// TODO(#429): Add reference to docs + quotes
/// `ManuallyDrop` has the same layout as `T`, and accessing the inner value
/// is safe (meaning that it's unsound to leave the inner value
/// uninitialized while exposing the `ManuallyDrop` to safe code).
Expand All @@ -1528,6 +1535,7 @@ safety_comment! {
/// code can only ever access a `ManuallyDrop` with all initialized bytes.
/// - `Unaligned`: `ManuallyDrop` has the same layout (and thus alignment)
/// as `T`, and `T: Unaligned` guarantees that that alignment is 1.
/// TODO(#429): Add references to docs and quotes.
unsafe_impl!(T: ?Sized + FromZeroes => FromZeroes for ManuallyDrop<T>);
unsafe_impl!(T: ?Sized + FromBytes => FromBytes for ManuallyDrop<T>);
unsafe_impl!(T: ?Sized + AsBytes => AsBytes for ManuallyDrop<T>);
Expand Down

0 comments on commit 1b27133

Please sign in to comment.