pin-init: enhance safety documentation for macro-generated code

liamjdavis · liamjdavis · commit db7cf4dfe325 · 2025-07-22T13:37:12.000-04:00
Add comprehensive safety documentation to pin-init macros, particularly
for the #[pin_data] macro which generates unsafe accessor functions.
The enhanced documentation clarifies:

- Safety requirements for slot pointer validity and alignment
- Pinning invariants that must be upheld by implementers
- Memory safety contracts for macro-generated unsafe functions
- Proper usage patterns for the pin-init API

This addresses gaps in the safety documentation that made it difficult
for users to understand the safety contracts when implementing custom
initializers or working with the generated code.

Signed-off-by: Liam Davis &lt;davisliam123@gmail.com&gt;
diff --git a/src/__internal.rs b/src/__internal.rs
@@ -51,19 +51,30 @@ where
 ///
 /// # Safety
 ///
-/// Only the `init` module is allowed to use this trait.
+/// This `unsafe trait` should only be implemented by the `#[pin_data]` macro. The implementer must ensure:
+/// - `Self::PinData` is the correct metadata type for `Self` and implements `PinData<Datee = Self>`
+/// - `__pin_data()` returns a valid instance that accurately reflects the structural pinning and field layout of `Self`
+/// - The metadata provides sound field accessors that uphold their safety contracts
+/// - Incorrect metadata can lead to undefined behavior when used by the pin-init system
 pub unsafe trait HasPinData {
     type PinData: PinData;
 
-    #[expect(clippy::missing_safety_doc)]
+    /// # Safety
+    ///
+    /// This method should only be called by macros in the pin-init crate.
     unsafe fn __pin_data() -> Self::PinData;
 }
 
 /// Marker trait for pinning data of structs.
 ///
 /// # Safety
 ///
-/// Only the `init` module is allowed to use this trait.
+/// This `unsafe trait` should only be implemented by the `#[pin_data]` macro for its generated
+/// metadata struct. The implementer must ensure:
+/// - `Self` is `Copy`
+/// - `Self::Datee` is the correct struct type implementing `HasPinData<PinData = Self>`
+/// - Generated field accessors uphold their safety contracts and correctly use the appropriate
+///   initialization methods
 pub unsafe trait PinData: Copy {
     type Datee: ?Sized + HasPinData;
 
@@ -81,19 +92,27 @@ pub unsafe trait PinData: Copy {
 ///
 /// # Safety
 ///
-/// Only the `init` module is allowed to use this trait.
+/// This `unsafe trait` should only be implemented by the `pin-init` macro system. The implementer must ensure:
+/// - `Self::InitData` accurately represents the initialization structure of `Self`
+/// - `__init_data()` returns a valid metadata instance for this type
+/// - The metadata correctly reflects the field layout and initialization requirements
 pub unsafe trait HasInitData {
     type InitData: InitData;
 
-    #[expect(clippy::missing_safety_doc)]
+    /// # Safety
+    ///
+    /// This method should only be called by macros in the pin-init crate.
     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` should only be implemented by the `pin-init` macro system. The implementer must ensure:
+/// - `Self` is `Copy`
+/// - `Self::Datee` correctly corresponds to the type `Self` represents for initialization purposes
+/// - The trait is used consistently within the pin-init system
 pub unsafe trait InitData: Copy {
     type Datee: ?Sized + HasInitData;
 
@@ -116,12 +135,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>;
 
diff --git a/src/lib.rs b/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,
 /// });
@@ -1409,28 +1412,6 @@ unsafe impl<T> PinInit<T> for T {
     }
 }
 
-// SAFETY: when the `__init` function returns with
-// - `Ok(())`, `slot` was initialized and all pinned invariants of `T` are upheld.
-// - `Err(err)`, slot was not written to.
-unsafe impl<T, E> Init<T, E> for Result<T, E> {
-    unsafe fn __init(self, slot: *mut T) -> Result<(), E> {
-        // SAFETY: `slot` is valid for writes by the safety requirements of this function.
-        unsafe { slot.write(self?) };
-        Ok(())
-    }
-}
-
-// SAFETY: when the `__pinned_init` function returns with
-// - `Ok(())`, `slot` was initialized and all pinned invariants of `T` are upheld.
-// - `Err(err)`, slot was not written to.
-unsafe impl<T, E> PinInit<T, E> for Result<T, E> {
-    unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E> {
-        // SAFETY: `slot` is valid for writes by the safety requirements of this function.
-        unsafe { slot.write(self?) };
-        Ok(())
-    }
-}
-
 /// Smart pointer containing uninitialized memory and that can write a value.
 pub trait InPlaceWrite<T> {
     /// The type `Self` turns into when the contents are initialized.
diff --git a/src/macros.rs b/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,34 @@ 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`.
+                /// `slot` is a pointer valid for writes and any value written to it is pinned.
                 $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.
+                /// `slot` is 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 +1152,11 @@ 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 this macro is part of the pin-init
+        // crate, satisfying the safety requirement that these methods should only be called by
+        // macros in the pin-init crate (as documented in their respective `# Safety` sections).
+        // 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 +1212,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 +1367,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
diff --git a/tests/ui/compile-fail/init/wrong_generics2.stderr b/tests/ui/compile-fail/init/wrong_generics2.stderr
@@ -12,7 +12,7 @@ help: you might have forgotten to add the struct literal inside the block
  --> src/macros.rs
   |
   ~                 ::core::ptr::write($slot, $t { SomeStruct {
-  |9                    $($acc)*
+  |8                    $($acc)*
   ~                 } });
   |
 
diff --git a/tests/ui/expand/many_generics.expanded.rs b/tests/ui/expand/many_generics.expanded.rs
@@ -46,20 +46,34 @@ const _: () = {
     where
         T: Bar<'a, 1>,
     {
+        /// # 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`.
         unsafe fn _pin<E>(
             self,
             slot: *mut PhantomPinned,
             init: impl ::pin_init::PinInit<PhantomPinned, E>,
         ) -> ::core::result::Result<(), E> {
             unsafe { ::pin_init::PinInit::__pinned_init(init, slot) }
         }
+        /// # 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.
         unsafe fn array<E>(
             self,
             slot: *mut [u8; 1024 * 1024],
             init: impl ::pin_init::Init<[u8; 1024 * 1024], E>,
         ) -> ::core::result::Result<(), E> {
             unsafe { ::pin_init::Init::__init(init, slot) }
         }
+        /// # 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.
         unsafe fn r<E>(
             self,
             slot: *mut &'b mut [&'a mut T; SIZE],
diff --git a/tests/ui/expand/pin-data.expanded.rs b/tests/ui/expand/pin-data.expanded.rs
@@ -17,13 +17,22 @@ const _: () = {
     #[allow(dead_code)]
     #[expect(clippy::missing_safety_doc)]
     impl __ThePinData {
+        /// # 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`.
         unsafe fn _pin<E>(
             self,
             slot: *mut PhantomPinned,
             init: impl ::pin_init::PinInit<PhantomPinned, E>,
         ) -> ::core::result::Result<(), E> {
             unsafe { ::pin_init::PinInit::__pinned_init(init, slot) }
         }
+        /// # 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.
         unsafe fn array<E>(
             self,
             slot: *mut [u8; 1024 * 1024],
diff --git a/tests/ui/expand/pinned_drop.expanded.rs b/tests/ui/expand/pinned_drop.expanded.rs
@@ -17,13 +17,22 @@ const _: () = {
     #[allow(dead_code)]
     #[expect(clippy::missing_safety_doc)]
     impl __ThePinData {
+        /// # 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`.
         unsafe fn _pin<E>(
             self,
             slot: *mut PhantomPinned,
             init: impl ::pin_init::PinInit<PhantomPinned, E>,
         ) -> ::core::result::Result<(), E> {
             unsafe { ::pin_init::PinInit::__pinned_init(init, slot) }
         }
+        /// # 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.
         unsafe fn array<E>(
             self,
             slot: *mut [u8; 1024 * 1024],

Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ help: you might have forgotten to add the struct literal inside the block`
`12`	`12`	`--> src/macros.rs`
`13`	`13`	`\|`
`14`	`14`	`~ ::core::ptr::write($slot, $t { SomeStruct {`
`15`		`- \|9 $($acc)*`
	`15`	`+ \|8 $($acc)*`
`16`	`16`	`~ } });`
`17`	`17`	`\|`
`18`	`18`