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

libstd: Add unwrap_or and unwrap_or_handle to Result #13475

Merged
merged 1 commit into from
Apr 14, 2014

Conversation

Ryman
Copy link
Contributor

@Ryman Ryman commented Apr 12, 2014

It might make more sense to mirror Option's unwrap_or_else but I've left it as handle as it feels more explicit about the signature difference.

@brson
Copy link
Contributor

brson commented Apr 12, 2014

I believe the current idea with the Option/Result interfaces is that for things like unwrap_or you can just convert to an Option with .ok(). unwrap_or_handle though looks Result-specific. Perhaps we can leave out unwrap_or and just have unwrap_or_handle.

@brson
Copy link
Contributor

brson commented Apr 12, 2014

Oh, hm. Result does define unwrap which could trivially be done with ok().unwrap() so I guess it's not so clear cut.

Does anybody remember why we kept unwrap? Is it the potential for more specific error messages?

@brson
Copy link
Contributor

brson commented Apr 12, 2014

Well, I r+'d based on existing standards.

@sfackler
Copy link
Member

@brson I think Result::unwrap is around from when we used to require that the error type be Show. It's superfluous now. I'd very much like to have Result::unwrap only defined for E: Show and have it actually add the error to the failure message. ok().unwrap() is an easy fallback for non-showable error types.

@brson
Copy link
Contributor

brson commented Apr 12, 2014

@sfackler Yeah that sounds reasonable. I don't recall why we removed the Show bound.

@sfackler
Copy link
Member

In the old implementation, all methods required a Show bound, which was kind of excessive. I'm writing up a PR right now.

@Ryman
Copy link
Contributor Author

Ryman commented Apr 12, 2014

@sfackler : That should fix #13379 👍

@Ryman Ryman closed this Apr 12, 2014
@sfackler sfackler reopened this Apr 12, 2014
bors added a commit that referenced this pull request Apr 14, 2014
It might make more sense to mirror `Option`'s `unwrap_or_else` but I've left it as `handle` as it feels more explicit about the signature difference.
@bors bors closed this Apr 14, 2014
@bors bors merged commit a16eae6 into rust-lang:master Apr 14, 2014
Dylan-DPC pushed a commit to Dylan-DPC/rust that referenced this pull request Nov 1, 2022
…, r=flodiebold

fix: Test all generic args for trait when finding matching impl

Addresses rust-lang/rust-analyzer#13463 (comment)

When finding matching impl for a trait method, we've been testing the unifiability of self type. However, there can be multiple impl of a trait for the same type with different generic arguments for the trait. This patch takes it into account and tests the unifiability of all type arguments for the trait (the first being the self type) thus enables rust-analyzer to find the correct impl even in such cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants