Re: [PATCH v5 09/26] rust: alloc: implement kernel `Box`

Alice Ryhl
2024-08-14T12:29:37Z

On Wed, Aug 14, 2024 at 2:22 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Wed, Aug 14, 2024 at 10:26:10AM +0200, Alice Ryhl wrote:
> > On Mon, Aug 12, 2024 at 8:25 PM Danilo Krummrich <dakr@kernel.org> wrote:
> > > +impl<T, A> From<Box<T, A>> for Pin<Box<T, A>>
> > > +where
> > > + T: ?Sized,
> > > + A: Allocator,
> > > +{
> > > + /// Converts a `Box<T, A>` into a `Pin<Box<T, A>>`. If `T` does not implement [`Unpin`], then
> > > + /// `*b` will be pinned in memory and can't be moved.
> > > + ///
> > > + /// See [`Box::into_pin`] for more details.
> > > + fn from(b: Box<T, A>) -> Self {
> > > + Box::into_pin(b)
> >
> > I still think it makes more sense to match std and only provide From
> > and not an into_pin, but it's not a blocker.
>
> Yeah, I just kept it since I'm not (yet) entirely sure what to think of the
> `From` and `Into` stuff in some cases.
>
> I don't really like that, depending on the context, it may hide relevant
> details.
>
> In the kernel, no matter how well documented an API is, I think it's rather
> common to look at the code for some implementation details before using it.
>
> Sometimes it might not be super trivial for the "occasional" reader to figure
> out what's the type of some variable. Calling `into_pin` vs. just `into`
> immediately tells the reader that things need to be pinned from there on.
>
> However, I had no specific example in my mind and I'm also not overly concerned
> to remove `into_pin`, but I want to at least share the reason why I kept it in
> the first place.

You can write `Pin::from` to convert the box. I think that reads
reasonably well.

But like I said, not a blocker for me.

Alice