[PATCH] rust: alloc: allow coercion from `Box<T>` to `Box<dyn U>` if T implements U

[PATCH] rust: alloc: allow coercion from `Box<T>` to `Box<dyn U>` if T implements U

[Alexandre Courbot]

This enables the creation of trait objects backed by a Box, similarly to
what can be done with the standard library.

Suggested-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
From this discussion on Zulip [1].

Heavily inspired from the similar feature on `Arc`.

[1] https://rust-for-linux.zulipchat.com/#narrow/channel/291565-Help/topic/Trait.20objects.3F/with/510689662
---
 rust/kernel/alloc/kbox.rs | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
index b77d32f3a58bab5ec73c612bdaaba0d79bfdff65..969b9f9fd3149685e1d1ecdf1eed9c647c887397 100644
--- a/rust/kernel/alloc/kbox.rs
+++ b/rust/kernel/alloc/kbox.rs
@@ -32,6 +32,8 @@
 ///
 /// When dropping a [`Box`], the value is also dropped and the heap memory is automatically freed.
 ///
+/// [`Box`]es can also be used to store trait objects by coercing their type.
+///
 /// # Examples
 ///
 /// ```
@@ -62,7 +64,17 @@
 /// `self.0` is always properly aligned and either points to memory allocated with `A` or, for
 /// zero-sized types, is a dangling, well aligned pointer.
 #[repr(transparent)]
-pub struct Box<T: ?Sized, A: Allocator>(NonNull<T>, PhantomData<A>);
+#[cfg_attr(CONFIG_RUSTC_HAS_COERCE_POINTEE, derive(core::marker::CoercePointee))]
+pub struct Box<#[pointee] T: ?Sized, A: Allocator>(NonNull<T>, PhantomData<A>);
+
+// This is to allow coercion from `Box<T>` to `Box<U>` if `T` can be converted to the
+// dynamically-sized type (DST) `U`.
+#[cfg(not(CONFIG_RUSTC_HAS_COERCE_POINTEE))]
+impl<T: ?Sized + core::marker::Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Box<U>> for Box<T> {}
+
+// This is to allow `Box<U>` to be dispatched on when `Box<T>` can be coerced into `Box<U>`.
+#[cfg(not(CONFIG_RUSTC_HAS_COERCE_POINTEE))]
+impl<T: ?Sized + core::marker::Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<Box<U>> for Box<T> {}
 
 /// Type alias for [`Box`] with a [`Kmalloc`] allocator.
 ///

---
base-commit: a2cc6ff5ec8f91bc463fd3b0c26b61166a07eb11
change-id: 20250408-box_trait_objs-02a700401f0b

Best regards,
-- 
Alexandre Courbot <acourbot@nvidia.com>

Re: [PATCH] rust: alloc: allow coercion from `Box<T>` to `Box<dyn U>` if T implements U

[Benno Lossin]

On Tue Apr 8, 2025 at 7:18 AM CEST, Alexandre Courbot wrote:
> This enables the creation of trait objects backed by a Box, similarly to
> what can be done with the standard library.
>
> Suggested-by: Benno Lossin <benno.lossin@proton.me>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> From this discussion on Zulip [1].
>
> Heavily inspired from the similar feature on `Arc`.
>
> [1] https://rust-for-linux.zulipchat.com/#narrow/channel/291565-Help/topic/Trait.20objects.3F/with/510689662
> ---
>  rust/kernel/alloc/kbox.rs | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
> index b77d32f3a58bab5ec73c612bdaaba0d79bfdff65..969b9f9fd3149685e1d1ecdf1eed9c647c887397 100644
> --- a/rust/kernel/alloc/kbox.rs
> +++ b/rust/kernel/alloc/kbox.rs
> @@ -32,6 +32,8 @@
>  ///
>  /// When dropping a [`Box`], the value is also dropped and the heap memory is automatically freed.
>  ///
> +/// [`Box`]es can also be used to store trait objects by coercing their type.
> +///
>  /// # Examples
>  ///
>  /// ```
> @@ -62,7 +64,17 @@
>  /// `self.0` is always properly aligned and either points to memory allocated with `A` or, for
>  /// zero-sized types, is a dangling, well aligned pointer.
>  #[repr(transparent)]
> -pub struct Box<T: ?Sized, A: Allocator>(NonNull<T>, PhantomData<A>);
> +#[cfg_attr(CONFIG_RUSTC_HAS_COERCE_POINTEE, derive(core::marker::CoercePointee))]
> +pub struct Box<#[pointee] T: ?Sized, A: Allocator>(NonNull<T>, PhantomData<A>);
> +
> +// This is to allow coercion from `Box<T>` to `Box<U>` if `T` can be converted to the
> +// dynamically-sized type (DST) `U`.
> +#[cfg(not(CONFIG_RUSTC_HAS_COERCE_POINTEE))]
> +impl<T: ?Sized + core::marker::Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Box<U>> for Box<T> {}

You forgot to add the `A: Allocator` generic here and in the impl below.

---
Cheers,
Benno

> +
> +// This is to allow `Box<U>` to be dispatched on when `Box<T>` can be coerced into `Box<U>`.
> +#[cfg(not(CONFIG_RUSTC_HAS_COERCE_POINTEE))]
> +impl<T: ?Sized + core::marker::Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<Box<U>> for Box<T> {}
>  
>  /// Type alias for [`Box`] with a [`Kmalloc`] allocator.
>  ///
>
> ---
> base-commit: a2cc6ff5ec8f91bc463fd3b0c26b61166a07eb11
> change-id: 20250408-box_trait_objs-02a700401f0b
>
> Best regards,

Re: [PATCH] rust: alloc: allow coercion from `Box<T>` to `Box<dyn U>` if T implements U

[Miguel Ojeda]

On Tue, Apr 8, 2025 at 12:22 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> You forgot to add the `A: Allocator` generic here and in the impl below.

Yeah, for this sort of changes (ideally, all patches, really), please
test with both the minimum supported version and the latest.

For instance, there is another issue, `#[pointee]` is only applicable
when you have the feature:

    error: cannot find attribute `pointee` in this scope
     --> rust/kernel/alloc/kbox.rs:66:18
      |
    66 | pub struct Box<#[pointee] T: ?Sized, A:
Allocator>(NonNull<T>, PhantomData<A>);

By the way, I would suggest taking the chance to move the sentence in
the docs below to the "Examples section" and providing one.

Thanks!

Cheers,
Miguel

Re: [PATCH] rust: alloc: allow coercion from `Box<T>` to `Box<dyn U>` if T implements U

[Danilo Krummrich]

On Tue, Apr 08, 2025 at 10:22:29AM +0000, Benno Lossin wrote:
> On Tue Apr 8, 2025 at 7:18 AM CEST, Alexandre Courbot wrote:
> > This enables the creation of trait objects backed by a Box, similarly to
> > what can be done with the standard library.
> >
> > Suggested-by: Benno Lossin <benno.lossin@proton.me>
> > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> > ---
> > From this discussion on Zulip [1].
> >
> > Heavily inspired from the similar feature on `Arc`.
> >
> > [1] https://rust-for-linux.zulipchat.com/#narrow/channel/291565-Help/topic/Trait.20objects.3F/with/510689662
> > ---
> >  rust/kernel/alloc/kbox.rs | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
> > index b77d32f3a58bab5ec73c612bdaaba0d79bfdff65..969b9f9fd3149685e1d1ecdf1eed9c647c887397 100644
> > --- a/rust/kernel/alloc/kbox.rs
> > +++ b/rust/kernel/alloc/kbox.rs
> > @@ -32,6 +32,8 @@
> >  ///
> >  /// When dropping a [`Box`], the value is also dropped and the heap memory is automatically freed.
> >  ///
> > +/// [`Box`]es can also be used to store trait objects by coercing their type.
> > +///
> >  /// # Examples
> >  ///
> >  /// ```
> > @@ -62,7 +64,17 @@
> >  /// `self.0` is always properly aligned and either points to memory allocated with `A` or, for
> >  /// zero-sized types, is a dangling, well aligned pointer.
> >  #[repr(transparent)]
> > -pub struct Box<T: ?Sized, A: Allocator>(NonNull<T>, PhantomData<A>);
> > +#[cfg_attr(CONFIG_RUSTC_HAS_COERCE_POINTEE, derive(core::marker::CoercePointee))]
> > +pub struct Box<#[pointee] T: ?Sized, A: Allocator>(NonNull<T>, PhantomData<A>);
> > +
> > +// This is to allow coercion from `Box<T>` to `Box<U>` if `T` can be converted to the
> > +// dynamically-sized type (DST) `U`.
> > +#[cfg(not(CONFIG_RUSTC_HAS_COERCE_POINTEE))]
> > +impl<T: ?Sized + core::marker::Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Box<U>> for Box<T> {}
> 
> You forgot to add the `A: Allocator` generic here and in the impl below.

Nit: Please use a where clause instead; I think it reads better and conforms
with the style of the rest of the file.

Re: [PATCH] rust: alloc: allow coercion from `Box<T>` to `Box<dyn U>` if T implements U

[Alexandre Courbot]

On Tue Apr 8, 2025 at 7:35 PM JST, Miguel Ojeda wrote:
> On Tue, Apr 8, 2025 at 12:22 PM Benno Lossin <benno.lossin@proton.me> wrote:
>>
>> You forgot to add the `A: Allocator` generic here and in the impl below.
>
> Yeah, for this sort of changes (ideally, all patches, really), please
> test with both the minimum supported version and the latest.

Apologies for that, I had no idea how to build using a specific
toolchain and did the wrong thing, which is sending without proper
testing.

I had some trouble finding how to 1) find out the minimum supported Rust
version, and 2) how to build using a specific toolchain.

For 1) I eventually found a hardcoded version in
scripts/min-tool-version.sh ; and 2) is somehow documentated in the
Quick Start guide that mentions the `rustup override set stable`
command.

I can send a patch against the Coding Guidelines adding a section to
encourage testing against the minimum version and explain how to force a
specific Rust version if you can confirm this would be helpful (and that
min-tool-version.sh is the correct way to check the minimum supported
Rust version).

>
> For instance, there is another issue, `#[pointee]` is only applicable
> when you have the feature:
>
>     error: cannot find attribute `pointee` in this scope
>      --> rust/kernel/alloc/kbox.rs:66:18
>       |
>     66 | pub struct Box<#[pointee] T: ?Sized, A:
> Allocator>(NonNull<T>, PhantomData<A>);

Mmm that one is trickier to address - I don't know of a way to make this
`#[pointee]` attribute depedent on `CONFIG_RUSTC_HAS_COERCE_POINTEE`...
Only solution I see if having two separate declarations for `Box`, and
then we have the problem of bindings the rustdoc to the declaration that
is picked at build time... Any idea for how to best address this?

Thanks,
Alex.

Re: [PATCH] rust: alloc: allow coercion from `Box<T>` to `Box<dyn U>` if T implements U

[Alice Ryhl]

On Tue, Apr 8, 2025 at 3:19 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> On Tue Apr 8, 2025 at 7:35 PM JST, Miguel Ojeda wrote:
> > On Tue, Apr 8, 2025 at 12:22 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >>
> >> You forgot to add the `A: Allocator` generic here and in the impl below.
> >
> > Yeah, for this sort of changes (ideally, all patches, really), please
> > test with both the minimum supported version and the latest.
>
> Apologies for that, I had no idea how to build using a specific
> toolchain and did the wrong thing, which is sending without proper
> testing.
>
> I had some trouble finding how to 1) find out the minimum supported Rust
> version, and 2) how to build using a specific toolchain.
>
> For 1) I eventually found a hardcoded version in
> scripts/min-tool-version.sh ; and 2) is somehow documentated in the
> Quick Start guide that mentions the `rustup override set stable`
> command.
>
> I can send a patch against the Coding Guidelines adding a section to
> encourage testing against the minimum version and explain how to force a
> specific Rust version if you can confirm this would be helpful (and that
> min-tool-version.sh is the correct way to check the minimum supported
> Rust version).
>
> >
> > For instance, there is another issue, `#[pointee]` is only applicable
> > when you have the feature:
> >
> >     error: cannot find attribute `pointee` in this scope
> >      --> rust/kernel/alloc/kbox.rs:66:18
> >       |
> >     66 | pub struct Box<#[pointee] T: ?Sized, A:
> > Allocator>(NonNull<T>, PhantomData<A>);
>
> Mmm that one is trickier to address - I don't know of a way to make this
> `#[pointee]` attribute depedent on `CONFIG_RUSTC_HAS_COERCE_POINTEE`...
> Only solution I see if having two separate declarations for `Box`, and
> then we have the problem of bindings the rustdoc to the declaration that
> is picked at build time... Any idea for how to best address this?

This might work:
struct Box<#[cfg_attr(CONFIG_RUSTC_HAS_COERCE_POINTEE, pointee)] T>

Re: [PATCH] rust: alloc: allow coercion from `Box<T>` to `Box<dyn U>` if T implements U

[Alexandre Courbot]

On Tue Apr 8, 2025 at 10:40 PM JST, Alice Ryhl wrote:
> On Tue, Apr 8, 2025 at 3:19 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>>
>> On Tue Apr 8, 2025 at 7:35 PM JST, Miguel Ojeda wrote:
>> > On Tue, Apr 8, 2025 at 12:22 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >>
>> >> You forgot to add the `A: Allocator` generic here and in the impl below.
>> >
>> > Yeah, for this sort of changes (ideally, all patches, really), please
>> > test with both the minimum supported version and the latest.
>>
>> Apologies for that, I had no idea how to build using a specific
>> toolchain and did the wrong thing, which is sending without proper
>> testing.
>>
>> I had some trouble finding how to 1) find out the minimum supported Rust
>> version, and 2) how to build using a specific toolchain.
>>
>> For 1) I eventually found a hardcoded version in
>> scripts/min-tool-version.sh ; and 2) is somehow documentated in the
>> Quick Start guide that mentions the `rustup override set stable`
>> command.
>>
>> I can send a patch against the Coding Guidelines adding a section to
>> encourage testing against the minimum version and explain how to force a
>> specific Rust version if you can confirm this would be helpful (and that
>> min-tool-version.sh is the correct way to check the minimum supported
>> Rust version).
>>
>> >
>> > For instance, there is another issue, `#[pointee]` is only applicable
>> > when you have the feature:
>> >
>> >     error: cannot find attribute `pointee` in this scope
>> >      --> rust/kernel/alloc/kbox.rs:66:18
>> >       |
>> >     66 | pub struct Box<#[pointee] T: ?Sized, A:
>> > Allocator>(NonNull<T>, PhantomData<A>);
>>
>> Mmm that one is trickier to address - I don't know of a way to make this
>> `#[pointee]` attribute depedent on `CONFIG_RUSTC_HAS_COERCE_POINTEE`...
>> Only solution I see if having two separate declarations for `Box`, and
>> then we have the problem of bindings the rustdoc to the declaration that
>> is picked at build time... Any idea for how to best address this?
>
> This might work:
> struct Box<#[cfg_attr(CONFIG_RUSTC_HAS_COERCE_POINTEE, pointee)] T>

I thought I'd tried that already and it failed, but giving it another go
now, it indeed *does* work. I must have been thrown off by a different
build error at that time. This solves everything then - thanks a lot!

Re: [PATCH] rust: alloc: allow coercion from `Box<T>` to `Box<dyn U>` if T implements U

[Miguel Ojeda]

On Tue, Apr 8, 2025 at 3:19 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> Apologies for that, I had no idea how to build using a specific
> toolchain and did the wrong thing, which is sending without proper
> testing.

No worries at all! It is completely normal :)

> I had some trouble finding how to 1) find out the minimum supported Rust
> version, and 2) how to build using a specific toolchain.

For 1), the minimum versions for tools the Linux kernel uses is documented at:

    https://docs.kernel.org/process/changes.html#current-minimal-requirements

`min-tool-version.sh` is also fine, of course.

For 2), similarly, it works like for other tools: either you override
your binary in the `$PATH` (that is easy with Rust with the command
you mention or simply with `rustup default X`), or you provide other
binaries via e.g. `make RUSTC=...` like you would do with `make
CC=...`.

> I can send a patch against the Coding Guidelines adding a section to
> encourage testing against the minimum version and explain how to force a
> specific Rust version if you can confirm this would be helpful (and that
> min-tool-version.sh is the correct way to check the minimum supported
> Rust version).

So that sort of thing is supposed to be documented in the "Submit
Checklist Addendum" section of the "Maintainer Entry Profile" document
(`P:` field in `MAINTAINERS`) of a given subsystem, and in particular
for Rust is at:

    https://rust-for-linux.com/contributing#submit-checklist-addendum

I agree that linking from the Coding Guidelines or perhaps the root of
the Rust docs would probably help others to find it, since different
people go to different places when starting, so please feel free to
send a patch!

Cheers,
Miguel

Re: [PATCH] rust: alloc: allow coercion from `Box<T>` to `Box<dyn U>` if T implements U

[Alexandre Courbot]

On Tue Apr 8, 2025 at 10:58 PM JST, Miguel Ojeda wrote:
> On Tue, Apr 8, 2025 at 3:19 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>>
>> Apologies for that, I had no idea how to build using a specific
>> toolchain and did the wrong thing, which is sending without proper
>> testing.
>
> No worries at all! It is completely normal :)
>
>> I had some trouble finding how to 1) find out the minimum supported Rust
>> version, and 2) how to build using a specific toolchain.
>
> For 1), the minimum versions for tools the Linux kernel uses is documented at:
>
>     https://docs.kernel.org/process/changes.html#current-minimal-requirements
>
> `min-tool-version.sh` is also fine, of course.
>
> For 2), similarly, it works like for other tools: either you override
> your binary in the `$PATH` (that is easy with Rust with the command
> you mention or simply with `rustup default X`), or you provide other
> binaries via e.g. `make RUSTC=...` like you would do with `make
> CC=...`.
>
>> I can send a patch against the Coding Guidelines adding a section to
>> encourage testing against the minimum version and explain how to force a
>> specific Rust version if you can confirm this would be helpful (and that
>> min-tool-version.sh is the correct way to check the minimum supported
>> Rust version).
>
> So that sort of thing is supposed to be documented in the "Submit
> Checklist Addendum" section of the "Maintainer Entry Profile" document
> (`P:` field in `MAINTAINERS`) of a given subsystem, and in particular
> for Rust is at:
>
>     https://rust-for-linux.com/contributing#submit-checklist-addendum
>
> I agree that linking from the Coding Guidelines or perhaps the root of
> the Rust docs would probably help others to find it, since different
> people go to different places when starting, so please feel free to
> send a patch!

Mmm upon reading this section again I agree it contains the needed
information (even mentions `min-tool-version.sh`!) and can be found at
the expected place, so I indeed don't see what more could be added to
it. Thanks for the pointer and sorry again for overlooking!