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

ACP: impl From<Infallible> for io::Error #86

Closed
Kixunil opened this issue Aug 11, 2022 · 7 comments
Closed

ACP: impl From<Infallible> for io::Error #86

Kixunil opened this issue Aug 11, 2022 · 7 comments
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@Kixunil
Copy link

Kixunil commented Aug 11, 2022

Proposal

Problem statement

Generic code sometimes needs to unify error types. This may happen in combinators or specialization-like wrappers. While Infallible should be obviously, trivially convertible to io::Error, the impl is not available.

Motivation, use-cases

My current use case (simplified)

pub struct HexWriter<W: HexWrite>(W);

/// Abstracts over fmt::Write and io::Write
pub trait HexWrite {
    type Error;
    fn write_byte(&mut self, byte: u8) -> Result<(), Self::Error>;
}

pub struct IoWriter<W: io::Write>(W);

impl<W: io::Write> HexWrite for IoWriter<W> { type Error = io::Error; /* ... */ }

/// Direct impl avoids dynamic dispatch of formatting and provides provably non-failing errors
impl HexWrite for String { type Error = Infallible; /* ... */ }


// Ideally, this would work for both HexWriter<String> and HexWriter<IoWriter<W>>:
impl<W> io::Write for HexWriter<W> where W: HexWrite, W::Error: Into<io::Error> { /* ... */ }

Potential other use cases

I vaguely remember doing something like this in the past, I think it was related to futures or some IO:

pub enum Error<A, B> {
   ErrorA(A),
   ErrorB(B),
}

impl<A, B> Error<A, B> {
    pub fn unify<E>(self) -> E where A: Into<E>, B: Into<E> {
        match self {
            ErrorA(a) => a.into(),
            ErrorB(b) => b.into(),
        }
    }
}

This allows unifying error types of different operations. If one of the operations is infallible and the other may return io::Error unify currently can not be used even though it looks like it should.

Solution sketches

Proposed solution: Impl conversion by matching on self, I don't see any downsides besides few added lines of code
Alternative solution 1: impl<T> From<Infallible> for T - this is probably far away due to overlap with impl<T> From<T> for T; the proposed solution already provides value and can be replaced with this alternative later without breaking anything
Alternative solution 2: do nothing, users can have their own IntoIoError trait (my current workaround) - this needlessly duplicates and complicates users code, adding a few lines to std seems to be a better trade-off.

Links and related work

PR implementing this

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.

@Kixunil Kixunil added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Aug 11, 2022
@Kixunil
Copy link
Author

Kixunil commented Aug 11, 2022

Actually, same applies to fmt::Error, so I think that should be included.

@shepmaster
Copy link
Member

While Infallible should be obviously, trivially convertible to io::Error,

it is not obvious to me, can you spell it out?

@Kixunil
Copy link
Author

Kixunil commented Aug 11, 2022

Infallible can never exist in run time, so this is the only reasonable implementation:

fn from(infallible: Infallible) -> Error {
    match infallible {}
}

@shepmaster
Copy link
Member

If I follow your logic, then Infallible -> () is also a reasonable implementation. Potentially also to any type.

This makes me think that I am not understanding your logic, which is why I’d appreciate if you could clarify the rationale.

@cuviper
Copy link
Member

cuviper commented Aug 11, 2022

Note that we do have From<Infallible> for TryFromSliceError and TryFromIntError.

There is also From<!> for Infallible, AIUI so that they might become the same type when ! is stabilized, and there's a "reserved" impl<T> From<!> for T (rust-lang/rust#64715).

@Kixunil
Copy link
Author

Kixunil commented Aug 11, 2022

@shepmaster actually, yes! Infallible should be convertible to any type but

  • writing blanket impl is currently impossible
  • writing impl for every single type in std is probably too much since Infallible is mainly used in error types.

@joshtriplett joshtriplett added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label May 17, 2023
@m-ou-se m-ou-se removed the ACP-accepted API Change Proposal is accepted (seconded with no objections) label May 17, 2023
@joshtriplett
Copy link
Member

We talked about this in the libs meetup. Initially there was some support for this, but we were hesitant about the effect this would have on the ecosystem in pushing people to add lots of these impls. We continue to hope that rust-lang/rust#64715 (and the stabilization of ! itself) might happen eventually (and we've poked for an update on the status of that). Meanwhile, we're going to close this..

@dtolnay dtolnay closed this as not planned Won't fix, can't repro, duplicate, stale Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

6 participants