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 Result::into_ok #61695

Open
phaylon opened this issue Jun 9, 2019 · 17 comments
Open

Tracking issue for Result::into_ok #61695

phaylon opened this issue Jun 9, 2019 · 17 comments
Labels
A-result-option B-unstable C-tracking-issue Libs-Small Libs-Tracked T-libs-api

Comments

@phaylon
Copy link
Contributor

@phaylon phaylon commented Jun 9, 2019

I would like to propose adding a unwrap_infallible associated function to core::result::Result. The purpose is to convert Result<T, core::convert::Infallible> to a T, as the error case is impossible.

The implementation would basically be:

impl<T> Result<T, Infallible> {
    #[inline]
    pub fn unwrap_infallible(self) -> T {
        match self {
            Ok(value) => value,
        }
    }
}

An example use-case I have is a wrapper type with generic value verification, like Handle<T:Verify>. Some verifications can fail, but some can not. When verification is infallible, a Result<Handle<T>, Infallible> will be returned.

Since some can fail, the Handle type implements TryFrom and not From. Because of the blanket implementation of TryFrom for all types implementing From, I can't additionally add a From conversion for the infallible cases. This blanket implementation makes sense, as it allows an API to take a T: TryFrom and handle all possible conversions, even infallible ones.

But for the API consumer it would be beneficial to be able to go from an infallible Result<T, Infallible> to a plain T without having to manually match or use expect. The latter is shorter and chainable, but has the disadvantage that it will still compile when the types are changed and the conversion is no longer infallible.

It might be that there is a different solution to infallible conversions via TryFrom in the future, for example via specialization. I believe that having an unwrap_infallible would still be beneficial in many other cases where generically fallible actions can be infallible in certain situations. One example is when working with a library that is based on fallible operations with a user supplied error type, but where the specific implementation from the user is infallible.

I'd be happy to work on a PR for this if it's acceptable to add, though I might require some guidance with regards to stability attributes, feature flags and so on. It's a bit hard to find information on that, and experimentation is costly as it takes me a while to complete a ./x.py test src/libcore :)

@jonas-schievink jonas-schievink added C-feature-request T-libs-api labels Jun 9, 2019
@Lonami
Copy link
Contributor

@Lonami Lonami commented Jun 9, 2019

How is this different from the never (!) type? In fact, that page mentions Infallible errors with the same wording.

@phaylon
Copy link
Contributor Author

@phaylon phaylon commented Jun 9, 2019

@Lonami I'm not sure I understand the question in relation to the proposal?

core::convert::Infallible is an existing type that acts as a stand-in for the never type, since the never type itself is still unstable. Core APIs like TryFrom already use Infallible.

I'm proposing an unwrapping fn on Result that is specific to dealing with core::convert::Infallible, and later (when it's stable) the never type.

@Lonami
Copy link
Contributor

@Lonami Lonami commented Jun 9, 2019

Oh, sorry, I should've checked first, I was unaware of that ^^

@phaylon
Copy link
Contributor Author

@phaylon phaylon commented Jun 9, 2019

No problem! Should've maybe added some background information, TryFrom and Infallible are rather recent additions :)

@scottmcm
Copy link
Member

@scottmcm scottmcm commented Jun 11, 2019

Looks like this might be rust-lang/rfcs#1723?

@phaylon
Copy link
Contributor Author

@phaylon phaylon commented Jun 11, 2019

@scottmcm Ah, yes. I didn't go back to 2016, and didn't search in the RFCs :)

@mzabaluev
Copy link
Contributor

@mzabaluev mzabaluev commented Nov 2, 2019

Hijacking this as a tracking issue for rust-lang/rfcs#2799
I'm amazed at: 1) myself for never finding this before writing the RFC; 2) anyone who has seen my proposal at internals.rust-lang.org not bringing up this issue either.

@SimonSapin SimonSapin changed the title Add Result::unwrap_infallible Tracking issue for Result::into_ok Nov 17, 2019
@SimonSapin SimonSapin added B-unstable C-tracking-issue and removed C-feature-request labels Nov 17, 2019
bors added a commit that referenced this issue Dec 10, 2019
Add method Result::into_ok

Implementation of rust-lang/rfcs#2799

Tracking issue #61695
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jan 10, 2020
@mexus
Copy link

@mexus mexus commented Apr 1, 2020

Is there anything else required to begin the process of stabilizing the feature?

@KodrAus KodrAus added A-result-option I-nominated Libs-Small Libs-Tracked labels Jul 29, 2020
@kotauskas
Copy link

@kotauskas kotauskas commented Aug 20, 2020

into_err (analogous to unwrap_err for Result<!, E>) should also be added, I think

@scottmcm
Copy link
Member

@scottmcm scottmcm commented Sep 25, 2020

@mexus You can always make a stabilization PR and see what libs thinks 🙃

Unfortunately ! still isn't stable, so it's possible that the current bound of E: Into<!> might make it impractical to take advantage of on stable. I don't know whether a change to use the usual placeholder (E: Into<core::convert::Infallible>) would be acceptable.

@faern
Copy link
Contributor

@faern faern commented Mar 23, 2021

I published a PR for adding the corresponding Result::into_err: #83421. I hope we can use the same feature gate and tracking issue?

I think the into_err version is at least as justified as into_ok. I'm currently in a very real situation where I'm starting to use Result<Infallible, Error>. But I had to introduce my own trait for now to safely extract the Error in a non panicking way. I know the type Result<Infallible, Error> looks a bit strange. I have not studied if there is a reason the enum is called Infallible instead of Never. But the documentation for Infallible states that it carries the same role as ! does. And the documentation for ! clearly states that a valid use case is Result<!, E> for operations that run indefinitely or until they hit an error.

The standard library simply has the return type -> Error for this type of run-until-error operations. But I don't think that's because it's more idiomatic than Result<!, E>. I think it's just because exec was introduced long before Infallible was a thing. A downside with -> Error is that you can't use ? inside the function for aborting with the error. I think Result<!, E> is more idiomatic and show the intent better.

@faern
Copy link
Contributor

@faern faern commented Mar 23, 2021

Is there anything blocking this being stabilized? This can already be used without ! being stable thanks to the nice workaround invented in bad::Never 🎉. (All credit to @nvzqz 👏)

If this feature is stabilized. The following would work on stable:

fn infallible_op() -> Result<String, bad::Never> {
    Ok("hello".into())
}

fn main() {
    let x: String = infallible_op().into_ok();
    println!("Great results: {}", x);
}

JohnTitor added a commit to JohnTitor/rust that referenced this issue Mar 25, 2021
Add Result::into_err where the Ok variant is the never type

Equivalent of rust-lang#66045 but for the inverse situation where `T: Into<!>` rather than `E: Into<!>`.

I'm using the same feature gate name. I can't see why one of these methods would be OK to stabilize but not the other.

Tracking issue: rust-lang#61695
@faern
Copy link
Contributor

@faern faern commented Mar 25, 2021

I posted PR #83493 in order to implement impl Into<!> for Infallible. If that PR is not accepted, I think we should change the trait bound on into_ok and into_err to be Into<Infallible> instead. That's the only way to make these methods usable on stable with the current standard never type stand-in.

@Cyborus04
Copy link
Contributor

@Cyborus04 Cyborus04 commented Oct 12, 2021

What about the more general trait bound, Into<T>? i.e.

impl<T, E: Into<T>> Result<T, E> {
    pub fn into_ok(self) -> T {
        match self {
            Ok(x) => x,
            Err(e) => e.into(),
        }
    }
}
/// and a corresponding `into_err`

This could make using stuff like <[T]>::binary_search easier, like slice.binary_search(&x).into_ok()

@mzabaluev
Copy link
Contributor

@mzabaluev mzabaluev commented Oct 12, 2021

@Cyborus04

What about the more general trait bound, Into<T>?

This bound does not generalize Into<!>, so the current usage and purpose of into_ok (which started off as unwrap_infallible) would no longer apply.

This could make using stuff like <[T]>::binary_search easier, like slice.binary_search(&x).into_ok()

I feel that this case is too niche to justify adding a special method on Result.

@Cyborus04
Copy link
Contributor

@Cyborus04 Cyborus04 commented Oct 12, 2021

This bound does not generalize Into<!>

Oh, right. Can't implement !: Into<T> cause that would conflict with T: Into<T>. Don't know why I didn't see that.

I feel that this case is too niche to justify adding a special method on Result.

Fair enough

Edit: what about Result::into_ok_or_err (#82223)? That serves the same purpose as what I suggested

@Kixunil
Copy link
Contributor

@Kixunil Kixunil commented Feb 10, 2022

What's needed for stabilization of this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-result-option B-unstable C-tracking-issue Libs-Small Libs-Tracked T-libs-api
Projects
None yet
Development

No branches or pull requests