[pointer] Simplify AliasingSafe, rename to Read (#1908)

`AliasingSafe` is really about whether a pointer permits unsynchronized
reads - either because the referent contains no `UnsafeCell`s or because
the aliasing mode is `Exclusive`. Previously, `AliasingSafe` was not
named consistent with this meaning, and was a function of a *pair* of
types rather than of a single type. This commit fixes both oversights.

While we're here, we also add `Read` bounds in some places, allowing us
to simplify many safety comments.

Author: joshlf

src/impls.rs

@@ -950,6 +950,7 @@ mod simd {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::pointer::invariant;
 
     #[test]
     fn test_impls() {

src/lib.rs

@@ -360,7 +360,7 @@ use core::{
     slice,
 };
 
-use crate::pointer::{invariant, BecauseExclusive};
+use crate::pointer::invariant::{self, BecauseExclusive};
 
 #[cfg(any(feature = "alloc", test))]
 extern crate alloc;
@@ -372,7 +372,7 @@ use core::alloc::Layout;
 
 // Used by `TryFromBytes::is_bit_valid`.
 #[doc(hidden)]
-pub use crate::pointer::{BecauseImmutable, Maybe, MaybeAligned, Ptr};
+pub use crate::pointer::{invariant::BecauseImmutable, Maybe, MaybeAligned, Ptr};
 // Used by `KnownLayout`.
 #[doc(hidden)]
 pub use crate::layout::*;

src/pointer/invariant.rs

@@ -170,78 +170,41 @@ impl Validity for Initialized {}
 pub enum Valid {}
 impl Validity for Valid {}
 
-pub mod aliasing_safety {
-    use super::*;
-    use crate::Immutable;
-
-    /// Pointer conversions which do not violate aliasing.
-    ///
-    /// `U: AliasingSafe<T, A, R>` implies that a pointer conversion from `T` to
-    /// `U` does not violate the aliasing invariant, `A`. This can be because
-    /// `A` is [`Exclusive`] or because neither `T` nor `U` permit interior
-    /// mutability.
-    ///
-    /// # Safety
-    ///
-    /// `U: AliasingSafe<T, A, R>` if either of the following conditions holds:
-    /// - `A` is [`Exclusive`]
-    /// - `T` and `U` both implement [`Immutable`]
-    #[doc(hidden)]
-    pub unsafe trait AliasingSafe<T: ?Sized, A: Aliasing, R: AliasingSafeReason> {}
-
-    #[doc(hidden)]
-    pub trait AliasingSafeReason: sealed::Sealed {}
-    impl<R: AliasingSafeReason> AliasingSafeReason for (R,) {}
-
-    /// The conversion is safe because only one live `Ptr` or reference may exist to
-    /// the referent bytes at a time.
-    #[derive(Copy, Clone, Debug)]
-    #[doc(hidden)]
-    pub enum BecauseExclusive {}
-    impl AliasingSafeReason for BecauseExclusive {}
-
-    /// The conversion is safe because no live `Ptr`s or references permit mutation.
-    #[derive(Copy, Clone, Debug)]
-    #[doc(hidden)]
-    pub enum BecauseImmutable {}
-    impl AliasingSafeReason for BecauseImmutable {}
-
-    /// SAFETY: `T: AliasingSafe<Exclusive, BecauseExclusive>` because for all
-    /// `Ptr<'a, T, I>` such that `I::Aliasing = Exclusive`, there cannot exist
-    /// other live references to the memory referenced by `Ptr`.
-    unsafe impl<T: ?Sized, U: ?Sized> AliasingSafe<T, Exclusive, BecauseExclusive> for U {}
-
-    /// SAFETY: `U: AliasingSafe<T, A, BecauseNoCell>` because for all `Ptr<'a, T,
-    /// I>` and `Ptr<'a, U, I>` such that `I::Aliasing = A`, all live references and
-    /// live `Ptr`s agree, by invariant on `Immutable`, that the referenced bytes
-    /// contain no `UnsafeCell`s, and thus do not permit mutation except via
-    /// exclusive aliasing.
-    unsafe impl<A, T: ?Sized, U: ?Sized> AliasingSafe<T, A, BecauseImmutable> for U
-    where
-        A: Aliasing,
-        T: Immutable,
-        U: Immutable,
-    {
-    }
-
-    /// This ensures that `U: AliasingSafe<T, A>` implies `T: AliasingSafe<U, A>` in
-    /// a manner legible to rustc, which in turn means we can write simpler bounds in
-    /// some places.
-    ///
-    /// SAFETY: Per `U: AliasingSafe<T, A, R>`, either:
-    /// - `A` is `Exclusive`
-    /// - `T` and `U` both implement `Immutable`
-    ///
-    /// Neither property depends on which of `T` and `U` are in the `Self` position
-    /// vs the first type parameter position.
-    unsafe impl<A, T: ?Sized, U: ?Sized, R> AliasingSafe<U, A, (R,)> for T
-    where
-        A: Aliasing,
-        R: AliasingSafeReason,
-        U: AliasingSafe<T, A, R>,
-    {
-    }
-}
+/// [`Ptr`](crate::Ptr) referents that permit unsynchronized read operations.
+///
+/// `T: Read<A, R>` implies that a pointer to `T` with aliasing `A` permits
+/// unsynchronized read oeprations. This can be because `A` is [`Exclusive`] or
+/// because `T` does not permit interior mutation.
+///
+/// # Safety
+///
+/// `T: Read<A, R>` if either of the following conditions holds:
+/// - `A` is [`Exclusive`]
+/// - `T` implements [`Immutable`](crate::Immutable)
+///
+/// As a consequence, if `T: Read<A, R>`, then any `Ptr<T, (A, ...)>` is
+/// permitted to perform unsynchronized reads from its referent.
+pub trait Read<A: Aliasing, R: ReadReason> {}
+
+impl<A: Reference, T: ?Sized + crate::Immutable> Read<A, BecauseImmutable> for T {}
+impl<T: ?Sized> Read<Exclusive, BecauseExclusive> for T {}
+
+/// Used to disambiguate [`Read`] impls.
+pub trait ReadReason: Sealed {}
+
+/// Unsynchronized reads are permitted because only one live [`Ptr`](crate::Ptr)
+/// or reference may exist to the referent bytes at a time.
+#[derive(Copy, Clone, Debug)]
+#[doc(hidden)]
+pub enum BecauseExclusive {}
+impl ReadReason for BecauseExclusive {}
+
+/// Unsynchronized reads are permitted because no live [`Ptr`](crate::Ptr)s or
+/// references permit interior mutation.
+#[derive(Copy, Clone, Debug)]
+#[doc(hidden)]
+pub enum BecauseImmutable {}
+impl ReadReason for BecauseImmutable {}
 
 use sealed::Sealed;
 mod sealed {
@@ -262,7 +225,6 @@ mod sealed {
 
     impl<A: Sealed, AA: Sealed, V: Sealed> Sealed for (A, AA, V) {}
 
-    impl Sealed for super::aliasing_safety::BecauseExclusive {}
-    impl Sealed for super::aliasing_safety::BecauseImmutable {}
-    impl<S: Sealed> Sealed for (S,) {}
+    impl Sealed for BecauseImmutable {}
+    impl Sealed for BecauseExclusive {}
 }

src/pointer/mod.rs

@@ -14,9 +14,7 @@ pub mod invariant;
 mod ptr;
 
 #[doc(hidden)]
-pub use invariant::aliasing_safety::{
-    AliasingSafe, AliasingSafeReason, BecauseExclusive, BecauseImmutable,
-};
+pub use invariant::{BecauseExclusive, BecauseImmutable, Read, ReadReason};
 #[doc(hidden)]
 pub use ptr::Ptr;
 
@@ -50,11 +48,11 @@ where
     pub fn read_unaligned<R>(self) -> T
     where
         T: Copy,
-        R: AliasingSafeReason,
-        T: AliasingSafe<T, Aliasing, R>,
+        R: invariant::ReadReason,
+        T: invariant::Read<Aliasing, R>,
     {
         // SAFETY: By invariant on `MaybeAligned`, `raw` contains
-        // validly-initialized data for `T`. By `T: AliasingSafe`, we are
+        // validly-initialized data for `T`. By `T: Read<Aliasing>`, we are
         // permitted to perform a read of `self`'s referent.
         unsafe { self.as_inner().read_unaligned() }
     }

src/pointer/ptr.rs

@@ -377,7 +377,7 @@ mod _conversions {
             //   byte ranges. Since `p` and the returned pointer address the
             //   same byte range, they refer to `UnsafeCell`s at the same byte
             //   ranges.
-            let c = unsafe { self.cast_unsized(|p| T::cast_into_inner(p)) };
+            let c = unsafe { self.cast_unsized_unchecked(|p| T::cast_into_inner(p)) };
             // SAFETY: By invariant on `TransparentWrapper`, since `self`
             // satisfies the alignment invariant `I::Alignment`, `c` (of type
             // `T::Inner`) satisfies the given "applied" alignment invariant.
@@ -411,7 +411,7 @@ mod _conversions {
             //   `UnsafeCell`s at the same locations as `p`.
             let ptr = unsafe {
                 #[allow(clippy::as_conversions)]
-                self.cast_unsized(|p: *mut T| p as *mut crate::Unalign<T>)
+                self.cast_unsized_unchecked(|p: *mut T| p as *mut crate::Unalign<T>)
             };
             // SAFETY: `Unalign<T>` promises to have the same bit validity as
             // `T`.
@@ -651,13 +651,14 @@ mod _transitions {
         /// On error, unsafe code may rely on this method's returned
         /// `ValidityError` containing `self`.
         #[inline]
-        pub(crate) fn try_into_valid(
+        pub(crate) fn try_into_valid<R>(
             mut self,
         ) -> Result<Ptr<'a, T, I::WithValidity<Valid>>, ValidityError<Self, T>>
         where
-            T: TryFromBytes,
+            T: TryFromBytes + Read<I::Aliasing, R>,
             I::Aliasing: Reference,
             I: Invariants<Validity = Initialized>,
+            R: crate::pointer::ReadReason,
         {
             // This call may panic. If that happens, it doesn't cause any soundness
             // issues, as we have not generated any invalid state which we need to
@@ -685,14 +686,19 @@ mod _transitions {
 /// Casts of the referent type.
 mod _casts {
     use super::*;
-    use crate::{pointer::invariant::aliasing_safety::*, CastError, SizeError};
+    use crate::{CastError, SizeError};
 
     impl<'a, T, I> Ptr<'a, T, I>
     where
         T: 'a + ?Sized,
         I: Invariants,
     {
-        /// Casts to a different (unsized) target type.
+        /// Casts to a different (unsized) target type without checking interior
+        /// mutability.
+        ///
+        /// Callers should prefer [`cast_unsized`] where possible.
+        ///
+        /// [`cast_unsized`]: Ptr::cast_unsized
         ///
         /// # Safety
         ///
@@ -705,7 +711,7 @@ mod _casts {
         ///   exist in `*p`
         #[doc(hidden)]
         #[inline]
-        pub unsafe fn cast_unsized<U: 'a + ?Sized, F: FnOnce(*mut T) -> *mut U>(
+        pub unsafe fn cast_unsized_unchecked<U: 'a + ?Sized, F: FnOnce(*mut T) -> *mut U>(
             self,
             cast: F,
         ) -> Ptr<'a, U, (I::Aliasing, Any, Any)> {
@@ -767,6 +773,34 @@ mod _casts {
             // 8. `ptr`, trivially, conforms to the validity invariant of `Any`.
             unsafe { Ptr::new(ptr) }
         }
+
+        /// Casts to a different (unsized) target type.
+        ///
+        /// # Safety
+        ///
+        /// The caller promises that `u = cast(p)` is a pointer cast with the
+        /// following properties:
+        /// - `u` addresses a subset of the bytes addressed by `p`
+        /// - `u` has the same provenance as `p`
+        #[doc(hidden)]
+        #[inline]
+        pub unsafe fn cast_unsized<U, F, R, S>(self, cast: F) -> Ptr<'a, U, (I::Aliasing, Any, Any)>
+        where
+            T: Read<I::Aliasing, R>,
+            U: 'a + ?Sized + Read<I::Aliasing, S>,
+            R: ReadReason,
+            S: ReadReason,
+            F: FnOnce(*mut T) -> *mut U,
+        {
+            // SAFETY: Because `T` and `U` both implement `Read<I::Aliasing, _>`,
+            // either:
+            // - `I::Aliasing` is `Exclusive`
+            // - `T` and `U` are both `Immutable`, in which case they trivially
+            //   contain `UnsafeCell`s at identical locations
+            //
+            // The caller promises all other safety preconditions.
+            unsafe { self.cast_unsized_unchecked(cast) }
+        }
     }
 
     impl<'a, T, I> Ptr<'a, T, I>
@@ -778,8 +812,9 @@ mod _casts {
         #[allow(clippy::wrong_self_convention)]
         pub(crate) fn as_bytes<R>(self) -> Ptr<'a, [u8], (I::Aliasing, Aligned, Valid)>
         where
-            [u8]: AliasingSafe<T, I::Aliasing, R>,
-            R: AliasingSafeReason,
+            R: ReadReason,
+            T: Read<I::Aliasing, R>,
+            I::Aliasing: Reference,
         {
             let bytes = match T::size_of_val_raw(self.as_inner().as_non_null()) {
                 Some(bytes) => bytes,
@@ -796,10 +831,6 @@ mod _casts {
             //   pointer's address, and `bytes` is the length of `p`, so the
             //   returned pointer addresses the same bytes as `p`
             // - `slice_from_raw_parts_mut` and `.cast` both preserve provenance
-            // - Because `[u8]: AliasingSafe<T, I::Aliasing, _>`, either:
-            //   - `I::Aliasing` is `Exclusive`
-            //   - `T` and `[u8]` are both `Immutable`, in which case they
-            //     trivially contain `UnsafeCell`s at identical locations
             let ptr: Ptr<'a, [u8], _> = unsafe {
                 self.cast_unsized(|p: *mut T| {
                     #[allow(clippy::as_conversions)]
@@ -878,9 +909,9 @@ mod _casts {
             CastError<Self, U>,
         >
         where
-            R: AliasingSafeReason,
+            R: ReadReason,
             I::Aliasing: Reference,
-            U: 'a + ?Sized + KnownLayout + AliasingSafe<[u8], I::Aliasing, R>,
+            U: 'a + ?Sized + KnownLayout + Read<I::Aliasing, R>,
         {
             let (inner, remainder) =
                 self.as_inner().try_cast_into(cast_type, meta).map_err(|err| {
@@ -893,12 +924,13 @@ mod _casts {
                 })?;
 
             // SAFETY:
-            // 0. Since `U: AliasingSafe<[u8], I::Aliasing, _>`, either:
+            // 0. Since `U: Read<I::Aliasing, _>`, either:
             //    - `I::Aliasing` is `Exclusive`, in which case both `src` and
             //      `ptr` conform to `Exclusive`
-            //    - `I::Aliasing` is `Shared` or `Any` and both `U` and `[u8]`
-            //      are `Immutable`. In this case, neither pointer permits
-            //      mutation, and so `Shared` aliasing is satisfied.
+            //    - `I::Aliasing` is `Shared` or `Any` and `U` is `Immutable`
+            //      (we already know that `[u8]: Immutable`). In this case,
+            //      neither `U` nor `[u8]` permit mutation, and so `Shared`
+            //      aliasing is satisfied.
             // 1. `ptr` conforms to the alignment invariant of `Aligned` because
             //    it is derived from `try_cast_into`, which promises that the
             //    object described by `target` is validly aligned for `U`.
@@ -939,8 +971,8 @@ mod _casts {
         ) -> Result<Ptr<'a, U, (I::Aliasing, Aligned, Initialized)>, CastError<Self, U>>
         where
             I::Aliasing: Reference,
-            U: 'a + ?Sized + KnownLayout + AliasingSafe<[u8], I::Aliasing, R>,
-            R: AliasingSafeReason,
+            U: 'a + ?Sized + KnownLayout + Read<I::Aliasing, R>,
+            R: ReadReason,
         {
             match self.try_cast_into(CastType::Prefix, meta) {
                 Ok((slf, remainder)) => {
@@ -984,11 +1016,8 @@ mod _casts {
         #[must_use]
         #[inline(always)]
         pub fn get_mut(self) -> Ptr<'a, T, I> {
-            // SAFETY:
-            // - The closure uses an `as` cast, which preserves address range
-            //   and provenance.
-            // - We require `I: Invariants<Aliasing = Exclusive>`, so we are not
-            //   required to uphold `UnsafeCell` equality.
+            // SAFETY: The closure uses an `as` cast, which preserves address
+            // range and provenance.
             #[allow(clippy::as_conversions)]
             let ptr = unsafe { self.cast_unsized(|p| p as *mut T) };
 
@@ -1034,7 +1063,8 @@ mod _project {
         ///
         /// # Safety
         ///
-        /// `project` has the same safety preconditions as `cast_unsized`.
+        /// `project` has the same safety preconditions as
+        /// `cast_unsized_unchecked`.
         #[doc(hidden)]
         #[inline]
         pub unsafe fn project<U: 'a + ?Sized>(
@@ -1046,8 +1076,8 @@ mod _project {
             // `Initialized` pointer, we could remove this method entirely.
 
             // SAFETY: This method has the same safety preconditions as
-            // `cast_unsized`.
-            let ptr = unsafe { self.cast_unsized(projector) };
+            // `cast_unsized_unchecked`.
+            let ptr = unsafe { self.cast_unsized_unchecked(projector) };
 
             // SAFETY: If all of the bytes of `self` are initialized (as
             // promised by `I: Invariants<Validity = Initialized>`), then any