rust: types: remove `Either<L, R>`
Benno Lossin
reviewerPATCH 1/1 rust: types: remove `Either<L, R>`
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.
--- rust/kernel/types.rs | 18 ------------------ 1 file changed, 18 deletions(-)
rust/kernel/types.rs +0 -18
@@ -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
Signature · 1 line
2.49.0
Quoted 10 lines · 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
Eitheralso 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 <[email protected]>
Alice Ryhl
reviewerQuoted 11 lines · 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
Eitheralso 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 <[email protected]>
I don't mind making a custom enum, but I do use this in Rust Binder.
Alice
Quoted 2 lines · Alice Ryhl
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
Alice Ryhl
reviewerQuoted 9 lines · Miguel Ojeda
Quoted 2 lines · Alice Ryhl
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?
Quoted 6 lines
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
Benno Lossin
reviewerQuoted 11 lines · Miguel Ojeda
Quoted 2 lines · Alice Ryhl
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...
Quoted 2 lines
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
Quoted 3 lines · Benno Lossin
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
Miguel Ojeda
AppliedQuoted 11 lines · 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
Eitheralso 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 <[email protected]>
Applied to rust-next -- thanks everyone!
Cheers,
Miguel