Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove std::either #9157

Closed
brson opened this issue Sep 13, 2013 · 15 comments · Fixed by #11149
Closed

Remove std::either #9157

brson opened this issue Sep 13, 2013 · 15 comments · Fixed by #11149
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@brson
Copy link
Contributor

brson commented Sep 13, 2013

It's rarely used and usually it's better to define a custom enum type.

@catamorphism
Copy link
Contributor

I agree. I can't think of a situation where using Either is preferable to defining a more informative type.

@bluss
Copy link
Member

bluss commented Sep 14, 2013

I'm looking at what current uses of Either can be migrated/refined. Some of them are using it just like a Result.

@bluss
Copy link
Member

bluss commented Sep 14, 2013

What do you think about these two? They don't exactly follow the pattern of Result since instead of a value or an error description, they represent two kinds of values.

libstd/rt/kill.rs:    pub fn try_block(mut task: ~Task) -> Either<~Task, BlockedTask> {
libstd/unstable/sync.rs:    pub fn try_unwrap(self) -> Either<UnsafeArc<T>, T> {

bors added a commit that referenced this issue Sep 14, 2013
Work a bit towards #9157 "Remove Either". These instances don't need to use Either and are better expressed in other ways (removing allocations and simplifying types).
@wakandan
Copy link

Hi I'm new to Rust and want to contribute. My friend recommends I should take this as the first step so I decided to give it a try. Can I take this? Thanks.

@jdm
Copy link
Contributor

jdm commented Sep 16, 2013

@blake2-ppc Are you doing any further work here, or can @wakandan take over removing all traces of Either?

@wakandan
Copy link

@jdm still very noob here, but will try my best!

@bluss
Copy link
Member

bluss commented Sep 16, 2013

I won't do anything here. @wakandan can go ahead.

I have my concern though that maybe Either is the best thing to keep around for cases like the return value of the two pub functions I listed in my comment previous. Just like it's convenient to return a tuple instead of a custom type, Either is maybe the best choice(?).

@wakandan
Copy link

Thanks @blake2-ppc. I was fetching from the upstream of rust but couldn't see your #9180. What am I missing here?

@bluss
Copy link
Member

bluss commented Sep 16, 2013

It was merged, you should see it if you check out new enough master

commit 1c26513ef9a58fa3e6703320cc37427aa229bbbd
Merge: 5d905f1 15c9dc7
Author: bors <bors@rust-lang.org>
Date:   Sat Sep 14 08:50:50 2013 -0700

    auto merge of #9180 : blake2-ppc/rust/reduce-either, r=catamorphism

@Kimundi
Copy link
Member

Kimundi commented Sep 16, 2013

@blake2-ppc: I think @brson's intention was to replace such cases with an custom enum type.
@wakandan: For handling those cases (Either types that don't describe an 'success'/'failure' duality), do this for each distinct use case:

  • Find out for what specific reason it uses an Either.
  • Find a good name for a new enum that describes that duality, and create it.
  • If the code used methods of Either, duplicated them into the new enum type, and maybe specialize them for the specific use case (which mainly means replacing 'either', 'left' and 'right' terminology).

@bluss
Copy link
Member

bluss commented Sep 16, 2013

Yes kimundi, I just mean this

Just like it's convenient to return a tuple instead of a custom type, Either is maybe the best choice(?).

A public API might be easier to use if Either is used as the recurring alternative type, just like tuples are sometimes used instead of creating custom types for each time a pair of values returned.

@Kimundi
Copy link
Member

Kimundi commented Sep 16, 2013

@blake2-ppc Hm, I agree in principle, but we'd really need types for two, three, four, ... variants there.

Maybe we could define the Either operations as an trait, and add an deriving mode for custom enums?

Or just predefine a bunch like with tuples.

@glaebhoerl
Copy link
Contributor

If we want to

predefine a bunch like with tuples

and also give them syntax like with tuples, I had an idea for that which I recorded in #8277. Not sure it passes the cost-benefit test, just mentioning. The reason it might be nice is that while there's no trouble defining a bunch of enums like Any2, Any3, Any4 and so forth, the constructor names get hairy: Case1of2, Case2of2, Case1of3, Case2of3, Case3of3... and so on.

@wakandan
Copy link

@Kimundi ok thanks.
@blake2-ppc I got the updates, thanks.

@wakandan
Copy link

I tried to remove "either" in mod.rs and rt.rs first, hope it's ok. Got a lot of help from @huonw. Please let me know if there's something I need to pay attention to. Thanks.

@bors bors closed this as completed in 08321f1 Jan 3, 2014
flip1995 pushed a commit to flip1995/rust that referenced this issue Jul 18, 2022
update pull request template

Improved suggestion for formatting lint names in the PR template to use this format:
[`lint_name`]

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants