improve safety comments with additional safety docs in pin-init module

Signed-off-by: Liam Davis <[email protected]>

Author: liamjdavis

src/__internal.rs

@@ -51,7 +51,10 @@ where
 ///
 /// # Safety
 ///
-/// Only the `init` module is allowed to use this trait.
+/// Implementers (typically the `#[pin_data]` macro) must ensure that this method
+/// returns a `PinData` instance that accurately reflects the structural pinning
+/// and field layout of `Self`. Incorrect metadata can lead to undefined behavior
+/// when used by the `pin-init` macros.
 pub unsafe trait HasPinData {
     type PinData: PinData;
 
@@ -63,7 +66,12 @@ pub unsafe trait HasPinData {
 ///
 /// # Safety
 ///
-/// Only the `init` module is allowed to use this trait.
+/// This `unsafe trait` is implemented by the `#[pin_data]` macro for its generated metadata
+/// struct (e.g., `__ThePinData`). The macro MUST ensure `Self` is `Copy`.
+/// It must also ensure `Self::Datee` is the correct struct type implementing
+/// `HasPinData<PinData = Self>`. Moreover, the macro must generate sound field accessors
+/// on `Self` that uphold their safety contracts and correctly use `$crate::PinInit::__pinned_init`
+/// or `$crate::Init::__init`. For internal `pin-init` use only.
 pub unsafe trait PinData: Copy {
     type Datee: ?Sized + HasPinData;
 
@@ -81,19 +89,24 @@ pub unsafe trait PinData: Copy {
 ///
 /// # Safety
 ///
-/// Only the `init` module is allowed to use this trait.
+/// Implementers (typically the `#[pin_data]` macro) must ensure that this method
+/// returns a `PinData` instance that accurately reflects the structural pinning
+/// and field layout of `Self`. Incorrect metadata can lead to undefined behavior
+/// when used by the `pin-init` macros.
 pub unsafe trait HasInitData {
     type InitData: InitData;
 
-    #[expect(clippy::missing_safety_doc)]
     unsafe fn __init_data() -> Self::InitData;
 }
 
 /// Same function as `PinData`, but for arbitrary data.
 ///
 /// # Safety
 ///
-/// Only the `init` module is allowed to use this trait.
+/// This `unsafe trait` is a component of the `pin-init` macro system.
+/// Implementers must ensure `Self` is `Copy`, and`Self::Datee` correctly corresponds to the type
+/// `Self` represents for initialization purposes. This trait is primarily intended for internal use
+/// by the `pin-init` crate.
 pub unsafe trait InitData: Copy {
     type Datee: ?Sized + HasInitData;
 
@@ -116,12 +129,16 @@ impl<T: ?Sized> Clone for AllData<T> {
 
 impl<T: ?Sized> Copy for AllData<T> {}
 
-// SAFETY: TODO.
+// SAFETY: This implementation upholds the `InitData` invariants that `AllData<T>` is `Copy`, and
+// `Self::Datee` is `T`, which correctly represents the type `AllData<T>` is concerned with
+// for initialization, as used by the `pin_init!` macros. The `make_closure` method is inherited
+// and is a safe identity function, fulfilling trait expectations.
 unsafe impl<T: ?Sized> InitData for AllData<T> {
     type Datee = T;
 }
 
-// SAFETY: TODO.
+// SAFETY: `__init_data` returns `AllData<T>` which is a correct `InitData` implementation
+// for type `T`. The function itself performs no unsafe memory operations.
 unsafe impl<T: ?Sized> HasInitData for T {
     type InitData = AllData<T>;
 

src/lib.rs

@@ -763,7 +763,10 @@ macro_rules! stack_try_pin_init {
 ///
 /// let init = pin_init!(&this in Buf {
 ///     buf: [0; 64],
-///     // SAFETY: TODO.
+///     // SAFETY: `this.as_ptr()` (within `pin_init!(&this in ...)` context) yields a `*mut Buf`
+///     // that the macro guarantees is valid for initialization (non-null, aligned, points to memory for `Buf`).
+///     // This makes it dereferenceable for `addr_of_mut!`, which safely creates a raw pointer to the `buf` field,
+///     // even if uninitialized, as it avoids creating a reference.
 ///     ptr: unsafe { addr_of_mut!((*this.as_ptr()).buf).cast() },
 ///     pin: PhantomPinned,
 /// });
@@ -1390,19 +1393,19 @@ where
     unsafe { pin_init_from_closure(init) }
 }
 
-// SAFETY: Every type can be initialized by-value.
+// SAFETY: TODO.
 unsafe impl<T, E> Init<T, E> for T {
     unsafe fn __init(self, slot: *mut T) -> Result<(), E> {
-        // SAFETY: TODO.
+        // SAFETY: `slot` points to a valid, uninitialized memory of the correct size and alignment for type `T`.
         unsafe { slot.write(self) };
         Ok(())
     }
 }
 
-// SAFETY: Every type can be initialized by-value. `__pinned_init` calls `__init`.
+// SAFETY: TODO.
 unsafe impl<T, E> PinInit<T, E> for T {
     unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E> {
-        // SAFETY: TODO.
+        // SAFETY: `self` is a valid value of type `T`, and all requirements for `__init` are met.
         unsafe { self.__init(slot) }
     }
 }

src/macros.rs

@@ -518,7 +518,11 @@ macro_rules! __pinned_drop {
             }
         ),
     ) => {
-        // SAFETY: TODO.
+        // SAFETY: This macro generates the `unsafe impl PinnedDrop`. This is safe as it's
+        // an internal part of the `#[pinned_drop]` proc-macro, which ensures correct
+        // transformation of the user's `impl`. The added `OnlyCallFromDrop` token restricts
+        // `PinnedDrop::drop` calls to the actual drop process (via the `Drop::drop` impl
+        // generated by `#[pin_data(PinnedDrop)]`) where `self` is pinned and valid.
         unsafe $($impl_sig)* {
             // Inherit all attributes and the type/ident tokens for the signature.
             $(#[$($attr)*])*
@@ -878,7 +882,12 @@ macro_rules! __pin_data {
                 }
             }
 
-            // SAFETY: TODO.
+            // SAFETY: The `__pin_data` macro ensures this `impl PinData` for `__ThePinData` is
+            // safe by guaranteeing that `__ThePinData` is `Copy`, `PinData::Datee` is correctly
+            // set to `$name` (which itself implements `HasPinData<PinData = __ThePinData>`), and
+            // that all generated field accessor methods on `__ThePinData` are sound, uphold their
+            // individual safety contracts (such as ensuring valid `slot` pointers), and correctly
+            // use either `$crate::PinInit::__pinned_init` or `$crate::Init::__init`.
             unsafe impl<$($impl_generics)*>
                 $crate::__internal::PinData for __ThePinData<$($ty_generics)*>
             where $($whr)*
@@ -1000,23 +1009,32 @@ macro_rules! __pin_data {
         {
             $(
                 $(#[$($p_attr)*])*
+                /// # Safety
+                ///
+                /// The caller must ensure that `slot` is a valid pointer to uninitialized memory
+                /// that is properly aligned and large enough to hold a value of type `$p_type`.
                 $pvis unsafe fn $p_field<E>(
                     self,
                     slot: *mut $p_type,
                     init: impl $crate::PinInit<$p_type, E>,
                 ) -> ::core::result::Result<(), E> {
-                    // SAFETY: TODO.
+                    // SAFETY: `slot` points to valid, uninitialized memory for a `$p_type`.
                     unsafe { $crate::PinInit::__pinned_init(init, slot) }
                 }
             )*
             $(
                 $(#[$($attr)*])*
+                /// # Safety
+                ///
+                /// The caller must ensure that `slot` is a valid pointer to uninitialized memory
+                /// that is properly aligned and large enough to hold a value of type `$type`.
+                /// The memory at `slot` must also be valid for writes.
                 $fvis unsafe fn $field<E>(
                     self,
                     slot: *mut $type,
                     init: impl $crate::Init<$type, E>,
                 ) -> ::core::result::Result<(), E> {
-                    // SAFETY: TODO.
+                    // SAFETY: `slot` points to valid, uninitialized memory for a `$type`.
                     unsafe { $crate::Init::__init(init, slot) }
                 }
             )*
@@ -1132,7 +1150,12 @@ macro_rules! __init_internal {
         struct __InitOk;
         // Get the data about fields from the supplied type.
         //
-        // SAFETY: TODO.
+        // SAFETY: The `$get_data()` call (which resolves to either `HasPinData::__pin_data()`
+        // or `HasInitData::__init_data()`) is safe because the `#[pin_data]` macro, when applied
+        // to type `$t`, correctly implements the respective trait. This ensures that the returned
+        // metadata accurately reflects `$t`'s structure and pinning properties, fulfilling the
+        // `# Safety` contract of the called `__pin_data()` or `__init_data()` method.
+        // The `paste!` macro is used for re-tokenization and does not affect this safety argument.
         let data = unsafe {
             use $crate::__internal::$has_data;
             // Here we abuse `paste!` to retokenize `$t`. Declarative macros have some internal
@@ -1188,7 +1211,12 @@ macro_rules! __init_internal {
         let init = move |slot| -> ::core::result::Result<(), $err> {
             init(slot).map(|__InitOk| ())
         };
-        // SAFETY: TODO.
+        // SAFETY: The `init` closure, generated by this macro, upholds the contract of
+        // `$crate::$construct_closure`. It ensures `slot` is fully initialized on `Ok(())`
+        // or properly cleaned (via `DropGuard`s) on `Err(err)`, leaving `slot` safe to
+        // deallocate. Pinning invariants are maintained using `PinData`/`InitData` methods
+        // for field initialization, and `slot` is not moved (for `!Unpin` types) as it's
+        // accessed via a pointer.
         let init = unsafe { $crate::$construct_closure::<_, $err>(init) };
         init
     }};
@@ -1338,7 +1366,8 @@ macro_rules! __init_internal {
         // Since we are in the closure that is never called, this will never get executed.
         // We abuse `slot` to get the correct type inference here:
         //
-        // SAFETY: TODO.
+        // SAFETY: This is unreachable code that is used solely for compile-time type checking,
+        // it is never executed.
         unsafe {
             // Here we abuse `paste!` to retokenize `$t`. Declarative macros have some internal
             // information that is associated to already parsed fragments, so a path fragment