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

Add impl Into<!> for Infallible #83493

Closed
wants to merge 2 commits into from

Conversation

faern
Copy link
Contributor

@faern faern commented Mar 25, 2021

It's currently possible to convert a never (!) type instance into Infallible (impl From<!> for Infallible). But not the other way around. That means that the (unstable) methods Result::into_ok and Result::into_err (tracking issue: #61695) that use the trait bounds Into<!> will currently not work when you have a Result<Infallible, _> or Result<_, Infallible>.

I tried to implement impl From<Infallible> for ! and ended up with a number of compilation errors like these:

error[E0277]: `?` couldn't convert the error to `!`
   --> compiler/rustc_serialize/src/opaque.rs:151:33
    |
151 |         self.emit_usize(v.len())?;
    |                                 ^ the trait `From<()>` is not implemented for `!`

I found in the git history that this was attempted previously, in commit b2cf9a0 in PR #58302 by @SimonSapin. And the problem is brought up in comment #58302 (comment) but no solution or discussion followed. I also don't understand the error. Is this the same inference problem that is currently preventing the never type from becoming stable? Clearly, custom never tokens can implement conversion into !, just not the standard Infallible one?

I then tried impl Into<!> for Infallible instead. And it seems to work. In the future when ! is (hopefully) stabilized, this impl can just be removed, as it will be covered by by the blanket implementation impl<T> From<T> for T. I know that manually implementing Into is not recommended. But as far as I know it's also not forbidden? This might be a legitimate case where we need it, since the corresponding From does not work?

I'm not sure if this could complicate the stabilization of the never type maybe? If so, we should likely not merge this. As I'm really hoping to get the proper never type on stable sometime. Ping @nikomatsakis, since I think you are still looking into that?

I tagged it as #[stable] directly. My understanding is that trait impls can't be unstable? Please advice on how to proceed here if this is wrong. Is an RFC needed?

Alternative solution (for Result::{into_ok, into_err} specifically)

For the into_ok and into_err methods specifically there is another way to make them work on stable before ! is stabilized. The trait bounds can be changed from Into<!> to Into<Infallible. Since the conversion between the types work that way it will continue to be usable on nightly for the real never type, and it will allow stable to use the methods with Infallible. But unless there are reasons we don't want Infallible to be convertible to the real never type, I think this is cleaner and more useful for Infallible in general.

@rust-highfive
Copy link
Collaborator

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 25, 2021
@kennytm kennytm added needs-fcp This change is insta-stable, so needs a completed FCP to proceed. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Mar 26, 2021
@nikomatsakis nikomatsakis added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Mar 26, 2021
@nikomatsakis
Copy link
Contributor

I'm tagging this as T-lang too because of the interaction with the never type fallback story (which btw we plan to try and advance soon...).

@crlf0710
Copy link
Member

Triage: It seems further work on never type work will make this PR obsolete. I'll mark this as blocked for now.

@crlf0710 crlf0710 added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 17, 2021
@jonas-schievink jonas-schievink added the relnotes Marks issues that should be documented in the release notes of the next release. label Jul 17, 2021
@scottmcm
Copy link
Member

But unless there are reasons we don't want Infallible to be convertible to the real never type

The goal is that as part of the stabilization, Infallible will become just type Infallible = !; so that all the crates working with Infallible today (such as implementing their traits on it) will just naturally support ! immediately.

@JohnCSimon
Copy link
Member

@faern
Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you do please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive

@JohnCSimon JohnCSimon closed this Nov 27, 2022
@rustbot rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-fcp This change is insta-stable, so needs a completed FCP to proceed. relnotes Marks issues that should be documented in the release notes of the next release. S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. T-lang Relevant to the language team, which will review and decide on the 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 this pull request may close these issues.

9 participants