Skip to content

Commit 8c27f55

Browse files
committed
pin-init: enhance safety documentation for macro-generated code
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. Also updates test expectations to match the new macro-generated documentation. Signed-off-by: Liam Davis <davisliam123@gmail.com>
1 parent 781e4e2 commit 8c27f55

File tree

7 files changed

+112
-41
lines changed

7 files changed

+112
-41
lines changed

src/__internal.rs

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,19 +51,30 @@ where
5151
///
5252
/// # Safety
5353
///
54-
/// Only the `init` module is allowed to use this trait.
54+
/// This `unsafe trait` should only be implemented by the `#[pin_data]` macro. The implementer must ensure:
55+
/// - `Self::PinData` is the correct metadata type for `Self` and implements `PinData<Datee = Self>`
56+
/// - `__pin_data()` returns a valid instance that accurately reflects the structural pinning and field layout of `Self`
57+
/// - The metadata provides sound field accessors that uphold their safety contracts
58+
/// - Incorrect metadata can lead to undefined behavior when used by the pin-init system
5559
pub unsafe trait HasPinData {
5660
type PinData: PinData;
5761

58-
#[expect(clippy::missing_safety_doc)]
62+
/// # Safety
63+
///
64+
/// This method should only be called by macros in the pin-init crate.
5965
unsafe fn __pin_data() -> Self::PinData;
6066
}
6167

6268
/// Marker trait for pinning data of structs.
6369
///
6470
/// # Safety
6571
///
66-
/// Only the `init` module is allowed to use this trait.
72+
/// This `unsafe trait` should only be implemented by the `#[pin_data]` macro for its generated
73+
/// metadata struct. The implementer must ensure:
74+
/// - `Self` is `Copy`
75+
/// - `Self::Datee` is the correct struct type implementing `HasPinData<PinData = Self>`
76+
/// - Generated field accessors uphold their safety contracts and correctly use the appropriate
77+
/// initialization methods
6778
pub unsafe trait PinData: Copy {
6879
type Datee: ?Sized + HasPinData;
6980

@@ -81,19 +92,27 @@ pub unsafe trait PinData: Copy {
8192
///
8293
/// # Safety
8394
///
84-
/// Only the `init` module is allowed to use this trait.
95+
/// This `unsafe trait` should only be implemented by the `pin-init` macro system. The implementer must ensure:
96+
/// - `Self::InitData` accurately represents the initialization structure of `Self`
97+
/// - `__init_data()` returns a valid metadata instance for this type
98+
/// - The metadata correctly reflects the field layout and initialization requirements
8599
pub unsafe trait HasInitData {
86100
type InitData: InitData;
87101

88-
#[expect(clippy::missing_safety_doc)]
102+
/// # Safety
103+
///
104+
/// This method should only be called by macros in the pin-init crate.
89105
unsafe fn __init_data() -> Self::InitData;
90106
}
91107

92108
/// Same function as `PinData`, but for arbitrary data.
93109
///
94110
/// # Safety
95111
///
96-
/// Only the `init` module is allowed to use this trait.
112+
/// This `unsafe trait` should only be implemented by the `pin-init` macro system. The implementer must ensure:
113+
/// - `Self` is `Copy`
114+
/// - `Self::Datee` correctly corresponds to the type `Self` represents for initialization purposes
115+
/// - The trait is used consistently within the pin-init system
97116
pub unsafe trait InitData: Copy {
98117
type Datee: ?Sized + HasInitData;
99118

@@ -116,12 +135,16 @@ impl<T: ?Sized> Clone for AllData<T> {
116135

117136
impl<T: ?Sized> Copy for AllData<T> {}
118137

119-
// SAFETY: TODO.
138+
// SAFETY: This implementation upholds the `InitData` invariants that `AllData<T>` is `Copy`, and
139+
// `Self::Datee` is `T`, which correctly represents the type `AllData<T>` is concerned with
140+
// for initialization, as used by the `pin_init!` macros. The `make_closure` method is inherited
141+
// and is a safe identity function, fulfilling trait expectations.
120142
unsafe impl<T: ?Sized> InitData for AllData<T> {
121143
type Datee = T;
122144
}
123145

124-
// SAFETY: TODO.
146+
// SAFETY: `__init_data` returns `AllData<T>` which is a correct `InitData` implementation
147+
// for type `T`. The function itself performs no unsafe memory operations.
125148
unsafe impl<T: ?Sized> HasInitData for T {
126149
type InitData = AllData<T>;
127150

src/lib.rs

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -763,7 +763,10 @@ macro_rules! stack_try_pin_init {
763763
///
764764
/// let init = pin_init!(&this in Buf {
765765
/// buf: [0; 64],
766-
/// // SAFETY: TODO.
766+
/// // SAFETY: `this.as_ptr()` (within `pin_init!(&this in ...)` context) yields a `*mut Buf`
767+
/// // that the macro guarantees is valid for initialization (non-null, aligned, points to memory for `Buf`).
768+
/// // This makes it dereferenceable for `addr_of_mut!`, which safely creates a raw pointer to the `buf` field,
769+
/// // even if uninitialized, as it avoids creating a reference.
767770
/// ptr: unsafe { addr_of_mut!((*this.as_ptr()).buf).cast() },
768771
/// pin: PhantomPinned,
769772
/// });
@@ -1409,28 +1412,6 @@ unsafe impl<T> PinInit<T> for T {
14091412
}
14101413
}
14111414

1412-
// SAFETY: when the `__init` function returns with
1413-
// - `Ok(())`, `slot` was initialized and all pinned invariants of `T` are upheld.
1414-
// - `Err(err)`, slot was not written to.
1415-
unsafe impl<T, E> Init<T, E> for Result<T, E> {
1416-
unsafe fn __init(self, slot: *mut T) -> Result<(), E> {
1417-
// SAFETY: `slot` is valid for writes by the safety requirements of this function.
1418-
unsafe { slot.write(self?) };
1419-
Ok(())
1420-
}
1421-
}
1422-
1423-
// SAFETY: when the `__pinned_init` function returns with
1424-
// - `Ok(())`, `slot` was initialized and all pinned invariants of `T` are upheld.
1425-
// - `Err(err)`, slot was not written to.
1426-
unsafe impl<T, E> PinInit<T, E> for Result<T, E> {
1427-
unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E> {
1428-
// SAFETY: `slot` is valid for writes by the safety requirements of this function.
1429-
unsafe { slot.write(self?) };
1430-
Ok(())
1431-
}
1432-
}
1433-
14341415
/// Smart pointer containing uninitialized memory and that can write a value.
14351416
pub trait InPlaceWrite<T> {
14361417
/// The type `Self` turns into when the contents are initialized.

src/macros.rs

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,11 @@ macro_rules! __pinned_drop {
518518
}
519519
),
520520
) => {
521-
// SAFETY: TODO.
521+
// SAFETY: This macro generates the `unsafe impl PinnedDrop`. This is safe as it's
522+
// an internal part of the `#[pinned_drop]` proc-macro, which ensures correct
523+
// transformation of the user's `impl`. The added `OnlyCallFromDrop` token restricts
524+
// `PinnedDrop::drop` calls to the actual drop process (via the `Drop::drop` impl
525+
// generated by `#[pin_data(PinnedDrop)]`) where `self` is pinned and valid.
522526
unsafe $($impl_sig)* {
523527
// Inherit all attributes and the type/ident tokens for the signature.
524528
$(#[$($attr)*])*
@@ -878,7 +882,12 @@ macro_rules! __pin_data {
878882
}
879883
}
880884

881-
// SAFETY: TODO.
885+
// SAFETY: The `__pin_data` macro ensures this `impl PinData` for `__ThePinData` is
886+
// safe by guaranteeing that `__ThePinData` is `Copy`, `PinData::Datee` is correctly
887+
// set to `$name` (which itself implements `HasPinData<PinData = __ThePinData>`), and
888+
// that all generated field accessor methods on `__ThePinData` are sound, uphold their
889+
// individual safety contracts (such as ensuring valid `slot` pointers), and correctly
890+
// use either `$crate::PinInit::__pinned_init` or `$crate::Init::__init`.
882891
unsafe impl<$($impl_generics)*>
883892
$crate::__internal::PinData for __ThePinData<$($ty_generics)*>
884893
where $($whr)*
@@ -1000,23 +1009,34 @@ macro_rules! __pin_data {
10001009
{
10011010
$(
10021011
$(#[$($p_attr)*])*
1012+
/// # Safety
1013+
///
1014+
/// The caller must ensure that `slot` is a valid pointer to uninitialized memory
1015+
/// that is properly aligned and large enough to hold a value of type `$p_type`.
1016+
/// `slot` is a pointer valid for writes and any value written to it is pinned.
10031017
$pvis unsafe fn $p_field<E>(
10041018
self,
10051019
slot: *mut $p_type,
10061020
init: impl $crate::PinInit<$p_type, E>,
10071021
) -> ::core::result::Result<(), E> {
1008-
// SAFETY: TODO.
1022+
// SAFETY: `slot` points to valid, uninitialized memory for a `$p_type`.
10091023
unsafe { $crate::PinInit::__pinned_init(init, slot) }
10101024
}
10111025
)*
10121026
$(
10131027
$(#[$($attr)*])*
1028+
/// # Safety
1029+
///
1030+
/// The caller must ensure that `slot` is a valid pointer to uninitialized memory
1031+
/// that is properly aligned and large enough to hold a value of type `$type`.
1032+
/// The memory at `slot` must also be valid for writes.
1033+
/// `slot` is valid for writes.
10141034
$fvis unsafe fn $field<E>(
10151035
self,
10161036
slot: *mut $type,
10171037
init: impl $crate::Init<$type, E>,
10181038
) -> ::core::result::Result<(), E> {
1019-
// SAFETY: TODO.
1039+
// SAFETY: `slot` points to valid, uninitialized memory for a `$type`.
10201040
unsafe { $crate::Init::__init(init, slot) }
10211041
}
10221042
)*
@@ -1132,7 +1152,11 @@ macro_rules! __init_internal {
11321152
struct __InitOk;
11331153
// Get the data about fields from the supplied type.
11341154
//
1135-
// SAFETY: TODO.
1155+
// SAFETY: The `$get_data()` call (which resolves to either `HasPinData::__pin_data()`
1156+
// or `HasInitData::__init_data()`) is safe because this macro is part of the pin-init
1157+
// crate, satisfying the safety requirement that these methods should only be called by
1158+
// macros in the pin-init crate (as documented in their respective `# Safety` sections).
1159+
// The `paste!` macro is used for re-tokenization and does not affect this safety argument.
11361160
let data = unsafe {
11371161
use $crate::__internal::$has_data;
11381162
// Here we abuse `paste!` to retokenize `$t`. Declarative macros have some internal
@@ -1188,7 +1212,12 @@ macro_rules! __init_internal {
11881212
let init = move |slot| -> ::core::result::Result<(), $err> {
11891213
init(slot).map(|__InitOk| ())
11901214
};
1191-
// SAFETY: TODO.
1215+
// SAFETY: The `init` closure, generated by this macro, upholds the contract of
1216+
// `$crate::$construct_closure`. It ensures `slot` is fully initialized on `Ok(())`
1217+
// or properly cleaned (via `DropGuard`s) on `Err(err)`, leaving `slot` safe to
1218+
// deallocate. Pinning invariants are maintained using `PinData`/`InitData` methods
1219+
// for field initialization, and `slot` is not moved (for `!Unpin` types) as it's
1220+
// accessed via a pointer.
11921221
let init = unsafe { $crate::$construct_closure::<_, $err>(init) };
11931222
init
11941223
}};
@@ -1338,7 +1367,8 @@ macro_rules! __init_internal {
13381367
// Since we are in the closure that is never called, this will never get executed.
13391368
// We abuse `slot` to get the correct type inference here:
13401369
//
1341-
// SAFETY: TODO.
1370+
// SAFETY: This is unreachable code that is used solely for compile-time type checking,
1371+
// it is never executed.
13421372
unsafe {
13431373
// Here we abuse `paste!` to retokenize `$t`. Declarative macros have some internal
13441374
// information that is associated to already parsed fragments, so a path fragment

tests/ui/compile-fail/init/invalid_init.stderr

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,5 @@ error[E0277]: the trait bound `impl pin_init::PinInit<Bar>: Init<Bar>` is not sa
1010
| |______the trait `Init<Bar>` is not implemented for `impl pin_init::PinInit<Bar>`
1111
| required by a bound introduced by this call
1212
|
13-
= help: the following other types implement trait `Init<T, E>`:
14-
ChainInit<I, F, T, E>
15-
Result<T, E>
13+
= help: the trait `Init<T, E>` is implemented for `ChainInit<I, F, T, E>`
1614
= note: this error originates in the macro `$crate::__init_internal` which comes from the expansion of the macro `init` (in Nightly builds, run with -Z macro-backtrace for more info)

tests/ui/expand/many_generics.expanded.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,20 +46,37 @@ const _: () = {
4646
where
4747
T: Bar<'a, 1>,
4848
{
49+
/// # Safety
50+
///
51+
/// The caller must ensure that `slot` is a valid pointer to uninitialized memory
52+
/// that is properly aligned and large enough to hold a value of type `$p_type`.
53+
/// `slot` is a pointer valid for writes and any value written to it is pinned.
4954
unsafe fn _pin<E>(
5055
self,
5156
slot: *mut PhantomPinned,
5257
init: impl ::pin_init::PinInit<PhantomPinned, E>,
5358
) -> ::core::result::Result<(), E> {
5459
unsafe { ::pin_init::PinInit::__pinned_init(init, slot) }
5560
}
61+
/// # Safety
62+
///
63+
/// The caller must ensure that `slot` is a valid pointer to uninitialized memory
64+
/// that is properly aligned and large enough to hold a value of type `$type`.
65+
/// The memory at `slot` must also be valid for writes.
66+
/// `slot` is valid for writes.
5667
unsafe fn array<E>(
5768
self,
5869
slot: *mut [u8; 1024 * 1024],
5970
init: impl ::pin_init::Init<[u8; 1024 * 1024], E>,
6071
) -> ::core::result::Result<(), E> {
6172
unsafe { ::pin_init::Init::__init(init, slot) }
6273
}
74+
/// # Safety
75+
///
76+
/// The caller must ensure that `slot` is a valid pointer to uninitialized memory
77+
/// that is properly aligned and large enough to hold a value of type `$type`.
78+
/// The memory at `slot` must also be valid for writes.
79+
/// `slot` is valid for writes.
6380
unsafe fn r<E>(
6481
self,
6582
slot: *mut &'b mut [&'a mut T; SIZE],

tests/ui/expand/pin-data.expanded.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,24 @@ const _: () = {
1717
#[allow(dead_code)]
1818
#[expect(clippy::missing_safety_doc)]
1919
impl __ThePinData {
20+
/// # Safety
21+
///
22+
/// The caller must ensure that `slot` is a valid pointer to uninitialized memory
23+
/// that is properly aligned and large enough to hold a value of type `$p_type`.
24+
/// `slot` is a pointer valid for writes and any value written to it is pinned.
2025
unsafe fn _pin<E>(
2126
self,
2227
slot: *mut PhantomPinned,
2328
init: impl ::pin_init::PinInit<PhantomPinned, E>,
2429
) -> ::core::result::Result<(), E> {
2530
unsafe { ::pin_init::PinInit::__pinned_init(init, slot) }
2631
}
32+
/// # Safety
33+
///
34+
/// The caller must ensure that `slot` is a valid pointer to uninitialized memory
35+
/// that is properly aligned and large enough to hold a value of type `$type`.
36+
/// The memory at `slot` must also be valid for writes.
37+
/// `slot` is valid for writes.
2738
unsafe fn array<E>(
2839
self,
2940
slot: *mut [u8; 1024 * 1024],

tests/ui/expand/pinned_drop.expanded.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,24 @@ const _: () = {
1717
#[allow(dead_code)]
1818
#[expect(clippy::missing_safety_doc)]
1919
impl __ThePinData {
20+
/// # Safety
21+
///
22+
/// The caller must ensure that `slot` is a valid pointer to uninitialized memory
23+
/// that is properly aligned and large enough to hold a value of type `$p_type`.
24+
/// `slot` is a pointer valid for writes and any value written to it is pinned.
2025
unsafe fn _pin<E>(
2126
self,
2227
slot: *mut PhantomPinned,
2328
init: impl ::pin_init::PinInit<PhantomPinned, E>,
2429
) -> ::core::result::Result<(), E> {
2530
unsafe { ::pin_init::PinInit::__pinned_init(init, slot) }
2631
}
32+
/// # Safety
33+
///
34+
/// The caller must ensure that `slot` is a valid pointer to uninitialized memory
35+
/// that is properly aligned and large enough to hold a value of type `$type`.
36+
/// The memory at `slot` must also be valid for writes.
37+
/// `slot` is valid for writes.
2738
unsafe fn array<E>(
2839
self,
2940
slot: *mut [u8; 1024 * 1024],

0 commit comments

Comments
 (0)