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

Tracking issue for Option::xor #50512

Closed
andre-vm opened this issue May 7, 2018 · 27 comments · Fixed by #60376
Closed

Tracking issue for Option::xor #50512

andre-vm opened this issue May 7, 2018 · 27 comments · Fixed by #60376
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@andre-vm
Copy link
Contributor

andre-vm commented May 7, 2018

Today I got myself in a situation in which I had two Option<T> values, and I wanted to return the only one that was Some. If both of them were Some, then I'd rather return None. I think it would be perfect to have a xor method, very similar to the or method:

impl<T> Option<T> {

    //...

    fn xor(self, optb: Option<T>) -> Option<T> {
        match (&self, &optb) {
            (&Some(_), &None) => self,
            (&None, &Some(_)) => optb,
            _ => None,
        }
    }
}

I think that would solve my problem nicely, and could be useful for a lot of people as well.

@andre-vm andre-vm changed the title Add xor method to Option<T> Add xor method to Option<T> May 7, 2018
@cuviper cuviper added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. labels May 7, 2018
@clarfonthey
Copy link
Contributor

There should also be an xor_else method, similar to or_else.

@ollie27
Copy link
Member

ollie27 commented May 7, 2018

There would be no point in a xor_else because you always have to evaluate the second argument.

@clarfonthey
Copy link
Contributor

…good point. Never mind, then.

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 17, 2018
Add Option::xor method

Implements the method requested in rust-lang#50512.
@andre-vm
Copy link
Contributor Author

Implemented on #50553.

@clarfonthey
Copy link
Contributor

@Milack27 You actually need to keep this issue open as a tracking issue. It hasn't been stabilised yet, so, it can only be used with #![feature(option_xor)] at the moment.

If you could rename this issue to "Tracking issue for Option::xor" it would also help make that clear.

@kennytm kennytm reopened this May 18, 2018
@kennytm kennytm added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. and removed C-feature-request Category: A feature request, i.e: not implemented / a PR. labels May 18, 2018
@andre-vm andre-vm changed the title Add xor method to Option<T> Tracking issue for Option::xor May 18, 2018
@sfackler sfackler added the B-unstable Blocker: Implemented in the nightly compiler and unstable. label May 18, 2018
@andre-vm
Copy link
Contributor Author

andre-vm commented May 18, 2018

@clarcharr Oops, sorry! Just fixed it.

@pravic
Copy link
Contributor

pravic commented May 23, 2018

Aren't new methods supposed to be added via RFC?

@andre-vm
Copy link
Contributor Author

@pravic I don't know. The same concern was raised in #50553, but it seems the approval of the libs team is sufficient. Initially, I just posted this issue as a feature suggestion, expecting to get the opinions of the community members before going through the RFC process. Nevertheless, I can still write an RFC if it's needed.

@clarfonthey
Copy link
Contributor

@pravic depends on what kind of method. Simple ones like this are implemented and left unstable to be used by the community, and usually that + a final comment period is enough to accept a feature. Ones that are more iffy on the design need RFCs though.

@Centril
Copy link
Contributor

Centril commented Nov 25, 2018

@sfackler Quite a bit of time has passed since it was first implemented on the 18th of May; How do you feel about stabilizing this?

@jcotton42
Copy link
Contributor

Seconding @Centril's question, it's now February 2019

@sfackler
Copy link
Member

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Feb 16, 2019

Team member @sfackler has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Feb 16, 2019
@withoutboats
Copy link
Contributor

withoutboats commented Feb 16, 2019

Seems really niche, I'm not sure if adding another method to option is worth the externalities of that versus people writing match statements

@Centril
Copy link
Contributor

Centril commented Feb 16, 2019

Fwiw, I think opta.xor(optb) is useful in situations where you e.g. have some optional configuration a, and optional b, and then you only want one of them or it is an error otherwise.

@andre-vm
Copy link
Contributor Author

Just for the sake of curiosity, Option::xor would have been useful to me when I was solving the Advent of Code 2017. I needed to parse a text file like this:

     |          
     |  +--+    
     A  |  C    
 F---|----E|--+ 
     |  |  |  D 
     +B-+  +--+ 

Then, I had to follow the path and collect the letters. I could have used Option::xor to decide where to go next when I met a corner. Here's a simplified version of the code:

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=cda2bdf9c41964c8aeb01916bd817c2d

@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Feb 23, 2019
@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Feb 23, 2019
@rfcbot
Copy link

rfcbot commented Feb 23, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@Amanieu
Copy link
Member

Amanieu commented Feb 27, 2019

Note that the same thing can also be expressed as opta.is_some() != optb.is_some(). Do we really need a separate method just for this?

@andre-vm
Copy link
Contributor Author

andre-vm commented Feb 27, 2019

@Amanieu opta.is_some() != optb.is_some() returns a boolean, while opta.xor(optb) returns an Option.

@lambda-fairy
Copy link
Contributor

lambda-fairy commented Mar 1, 2019

Does this method exist in other languages? I've never seen such a method in OCaml or Haskell, both languages which make extensive use of option types and have been around much longer than Rust has. That should make us doubt whether this is useful enough to belong in the standard library.

@andre-vm
Copy link
Contributor Author

andre-vm commented Mar 1, 2019

@lfairy Your point is reasonable. I'm not familiar with either OCaml or Haskell, but I'd like to point out some possible arguments in favor of Option::xor that could be taken into account when comparing Rust to those languages:

  1. Rust already has a stable Option::or method, which works pretty much the same way. When users find it in the documentation, they may expect an Option::xor as well. If OCaml and Haskell also have an equivalent method to Option::or, but not a xor counterpart, it would be interesting to know why.

  2. Option::xor is a zero-cost abstraction, which is a valued concept in Rust. It's a tool that allows programmers to do more with less code. But if you don't need to use it, you have no penalties. The footprint of Option::xor in the standard library is also very small. Add Option::xor method #50553 adds only 36 lines, most of which are documentation comments.

  3. Users cannot implement this method in their own crates, as the Option type doesn't belong to them. The alternatives I can think of are a bit cumbersome, or at least not as convenient:

    • Use a match directly
    • Create a function similar to fn option_xor<T>(opta: Option<T>, optb: Option<T>) -> Option<T>
    • Create a new trait and implement Option::xor inside it

@Centril
Copy link
Contributor

Centril commented Mar 1, 2019

  1. Rust already has a stable Option::or method, which works pretty much the same way. When users find it in the documentation, they may expect an Option::xor as well. If OCaml and Haskell also have an equivalent method to Option::or, but not a xor counterpart, it would be interesting to know why.

Haskell's standard library has the more general operation <|>:

class Applicative f => Alternative f where 
  empty :: f a
  (<|>) :: f a -> f a -> f a

and Maybe is an instance of Alternative.

@boomshroom
Copy link

boomshroom commented Mar 1, 2019

Haskell's standard library has the more general operation <|>:

class Applicative f => Alternative f where
empty :: f a
(<|>) :: f a -> f a -> f a

and Maybe is an instance of Alternative

Looking at the implementation, it seems that Haskell's implementation is an inclusive or rather than exclusive and takes the first argument if it's present regardless of the second.

@rfcbot
Copy link

rfcbot commented Mar 5, 2019

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Mar 5, 2019
@Centril
Copy link
Contributor

Centril commented Mar 5, 2019

@Amanieu & @withoutboats: Since you have expressed doubts, can you confirm that you are OK with moving forward with this?

@Centril Centril added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Mar 26, 2019
Centril added a commit to Centril/rust that referenced this issue Jun 12, 2019
Centril added a commit to Centril/rust that referenced this issue Jun 12, 2019
@aaditmshah
Copy link

@andre-vm I just wanted to point out that Option::xor is not associative.

  lhs
= Some(1).xor(Some(2)).xor(Some(3))
= None.xor(Some(3))
= Some(3)

  rhs
= Some(1).xor(Some(2).xor(Some(3)))
= Some(1).xor(None)
= Some(1)

However, boolean xor is associative.

  lhs
= (true != true) != true
= false != true
= true

  rhs
= true != (true != true)
= true != false
= true

Thus, it seems to me that Option::xor is not an exclusive or operation (over a functor) in the strictest sense. Nevertheless, it still seems to be a useful operation. Hence, I would suggest adding a remark to the documentation, elucidating that Option::xor is not associative.

@aaditmshah
Copy link

aaditmshah commented May 15, 2023

I would also like to point out a simpler operation than Option::xor, which can be used to implement Option::xor:

impl<T> Option<T> {
    fn and_not<U>(self, other: Option<U>) -> Option<T> {
        match &other {
            &None => self,
            _ => None,
        }
    }
}

The Option::xor operation can be implemented using a combination of Option::and_not and Option::or.

option_a.xor(option_b) = option_a.and_not(option_b).or(option_b.and_not(option_a))

Conversely, we can implement Option::and_not using a combination of Option::xor and Option::and.

option_a.and_not(option_b) = option_a.map(Ok).xor(option_b.map(Err)).and(option_a)

It would be nice to have Option::and_not in the core library. However, even if we don't have it, it would be nice to add the previous example to the documentation of Option::xor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.