[PATCH] rust: types: remove `Either<L, R>`

[PATCH] rust: types: remove `Either<L, R>`

[Benno Lossin]

This enum is not used. Additionally, using it would result in poor
ergonomics, because in order to do any operation on a value it has to be
matched first. Our version of `Either` also doesn't provide any helper
methods making it even more difficult to use.

The alternative of creating a custom enum for the concrete use-case also
is much better for ergonomics. As one can provide functions on the type
directly and users don't need to match the value manually.

Signed-off-by: Benno Lossin <lossin@kernel.org>
---
 rust/kernel/types.rs | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 86562e738eac..8830f8c2d12e 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -556,24 +556,6 @@ fn drop(&mut self) {
     }
 }
 
-/// A sum type that always holds either a value of type `L` or `R`.
-///
-/// # Examples
-///
-/// ```
-/// use kernel::types::Either;
-///
-/// let left_value: Either<i32, &str> = Either::Left(7);
-/// let right_value: Either<i32, &str> = Either::Right("right value");
-/// ```
-pub enum Either<L, R> {
-    /// Constructs an instance of [`Either`] containing a value of type `L`.
-    Left(L),
-
-    /// Constructs an instance of [`Either`] containing a value of type `R`.
-    Right(R),
-}
-
 /// Zero-sized type to mark types not [`Send`].
 ///
 /// Add this type as a field to your struct if your type should not be sent to a different task.

base-commit: 22c3335c5dcd33063fe1894676a3a6ff1008d506
-- 
2.49.0

Re: [PATCH] rust: types: remove `Either<L, R>`

[Danilo Krummrich]

On 5/19/25 2:43 PM, Benno Lossin wrote:
> This enum is not used. Additionally, using it would result in poor
> ergonomics, because in order to do any operation on a value it has to be
> matched first. Our version of `Either` also doesn't provide any helper
> methods making it even more difficult to use.
> 
> The alternative of creating a custom enum for the concrete use-case also
> is much better for ergonomics. As one can provide functions on the type
> directly and users don't need to match the value manually.
> 
> Signed-off-by: Benno Lossin <lossin@kernel.org>

Reviewed-by: Danilo Krummrich <dakr@kernel.org>

Re: [PATCH] rust: types: remove `Either<L, R>`

[Alice Ryhl]

On Mon, May 19, 2025 at 5:43 AM Benno Lossin <lossin@kernel.org> wrote:
>
> This enum is not used. Additionally, using it would result in poor
> ergonomics, because in order to do any operation on a value it has to be
> matched first. Our version of `Either` also doesn't provide any helper
> methods making it even more difficult to use.
>
> The alternative of creating a custom enum for the concrete use-case also
> is much better for ergonomics. As one can provide functions on the type
> directly and users don't need to match the value manually.
>
> Signed-off-by: Benno Lossin <lossin@kernel.org>

I don't mind making a custom enum, but I do use this in Rust Binder.

Alice

Re: [PATCH] rust: types: remove `Either<L, R>`

[Miguel Ojeda]

On Mon, May 19, 2025 at 7:30 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> I don't mind making a custom enum, but I do use this in Rust Binder.

Yeah, Wedson added the type back then for Binder and kasync from a
quick look -- in those times, I see it in a few places only, e.g. in
`get_work_or_register`. Do you have many nowadays?

i.e. I don't want to add extra work for upstreaming Rust Binder, so if
that would make it harder downstream, we can live with it for the
moment.

We may want to add a line in the docs to ask the potential user to
consider whether a custom enum would be better nevertheless.

Cheers,
Miguel

Re: [PATCH] rust: types: remove `Either<L, R>`

[Alice Ryhl]

On Mon, May 19, 2025 at 11:08 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Mon, May 19, 2025 at 7:30 PM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > I don't mind making a custom enum, but I do use this in Rust Binder.
>
> Yeah, Wedson added the type back then for Binder and kasync from a
> quick look -- in those times, I see it in a few places only, e.g. in
> `get_work_or_register`. Do you have many nowadays?
>
> i.e. I don't want to add extra work for upstreaming Rust Binder, so if
> that would make it harder downstream, we can live with it for the
> moment.
>
> We may want to add a line in the docs to ask the potential user to
> consider whether a custom enum would be better nevertheless.

It's used get_work_or_register to return either work, or a
registration waiting for work if the work list is empty. I believe
there is only that one use.

Alice

Re: [PATCH] rust: types: remove `Either<L, R>`

[Benno Lossin]

On Mon May 19, 2025 at 8:08 PM CEST, Miguel Ojeda wrote:
> On Mon, May 19, 2025 at 7:30 PM Alice Ryhl <aliceryhl@google.com> wrote:
>>
>> I don't mind making a custom enum, but I do use this in Rust Binder.
>
> Yeah, Wedson added the type back then for Binder and kasync from a
> quick look -- in those times, I see it in a few places only, e.g. in
> `get_work_or_register`. Do you have many nowadays?
>
> i.e. I don't want to add extra work for upstreaming Rust Binder, so if
> that would make it harder downstream, we can live with it for the
> moment.

I can also take a look and make a patch that uses a custom enum instead
if you don't want to do that, Alice.

Another option would be to deprecate it. Do I remember correctly that
Linus doesn't like that? If yes, then that maybe isn't a good idea...

> We may want to add a line in the docs to ask the potential user to
> consider whether a custom enum would be better nevertheless.

Yeah that too.

---
Cheers,
Benno

Re: [PATCH] rust: types: remove `Either<L, R>`

[Miguel Ojeda]

On Mon, May 19, 2025 at 8:23 PM Benno Lossin <lossin@kernel.org> wrote:
>
> Another option would be to deprecate it. Do I remember correctly that
> Linus doesn't like that? If yes, then that maybe isn't a good idea...

Yeah, though for Rust we may need it sometimes during the "phased"
moves of items between Rust modules.

In this case, it is just not used upstream, and the only user
out-of-tree is easy to replace, so there is nothing stopping us from
just removing it.

Cheers,
Miguel

Re: [PATCH] rust: types: remove `Either<L, R>`

[Miguel Ojeda]

On Mon, May 19, 2025 at 2:43 PM Benno Lossin <lossin@kernel.org> wrote:
>
> This enum is not used. Additionally, using it would result in poor
> ergonomics, because in order to do any operation on a value it has to be
> matched first. Our version of `Either` also doesn't provide any helper
> methods making it even more difficult to use.
>
> The alternative of creating a custom enum for the concrete use-case also
> is much better for ergonomics. As one can provide functions on the type
> directly and users don't need to match the value manually.
>
> Signed-off-by: Benno Lossin <lossin@kernel.org>

Applied to `rust-next` -- thanks everyone!

Cheers,
Miguel