Messages in this thread | | | Date | Thu, 30 Mar 2023 15:36:29 +0000 | From | Benno Lossin <> | Subject | Re: [PATCH v3 04/13] rust: add pin-init API core |
| |
On 30.03.23 15:33, Alice Ryhl wrote: > On 3/30/23 00:33, y86-dev@protonmail.com wrote: >> From: Benno Lossin <y86-dev@protonmail.com> >> >> This API is used to facilitate safe pinned initialization of structs. It >> replaces cumbersome `unsafe` manual initialization with elegant safe macro >> invocations. >> >> Due to the size of this change it has been split into six commits: >> 1. This commit introducing the basic public interface: traits and >> functions to represent and create initializers. >> 2. Adds the `#[pin_data]`, `pin_init!`, `try_pin_init!`, `init!` and >> `try_init!` macros along with their internal types. >> 3. Adds the `InPlaceInit` trait that allows using an initializer to create >> an object inside of a `Box<T>` and other smart pointers. >> 4. Adds the `PinnedDrop` trait and adds macro support for it in >> the `#[pin_data]` macro. >> 5. Adds the `stack_pin_init!` macro allowing to pin-initialize a struct on >> the stack. >> 6. Adds the `Zeroable` trait and `init::zeroed` function to initialize >> types that have `0x00` in all bytes as a valid bit pattern. >> >> Co-developed-by: Gary Guo <gary@garyguo.net> >> Signed-off-by: Gary Guo <gary@garyguo.net> >> Signed-off-by: Benno Lossin <y86-dev@protonmail.com> > > Reviewed-by: Alice Ryhl <aliceryhl@google.com> > >> +//! Aside from pinned initialization, this API also supports in-place construction without pinning, >> +//! the marcos/types/functions are generally named like the pinned variants without the `pin` >> +//! prefix. > > Typo: Should be "macros". > >> +type Invariant<T> = PhantomData<fn(*mut T) -> *mut T>; > > I think it would make sense to include a link to the nomicon on the > documentation for the Invariant type. > > E.g. this link: https://doc.rust-lang.org/nomicon/subtyping.html
Sure that is a good idea.
>> +// This is the module-internal type implementing `PinInit` and `Init`. It is unsafe to create this >> +// type, since the closure needs to fulfill the same safety requirement as the >> +// `__pinned_init`/`__init` functions. >> +struct InitClosure<F, T: ?Sized, E>(F, Invariant<(E, T)>); > > Documentation for a type should use /// rather than //. > > I think it would help to call out explicitly in the documentation on > this type that it is an implementation detail of the > pin_init_from_closure and init_from_closure methods.
Yeah, I will also move this into `__internal.rs` and add the comment.
-- Cheers, Benno
| |