Tracking Issue for `io::Error::other`

[ibraheemdev]

Feature gate: #![feature(io_error_other)]

This is a tracking issue for io::Error::other(err), a shortcut for io::Error::new(io::ErrorKind::Other, err)

Public API

// std::io

impl Error {
    pub fn other<E>(error: E) -> Error
    where
        E: Into<Box<dyn error::Error + Send + Sync>>;
}

Steps / History

• Implementation: Add io::Error::other #91947
• Final comment period (FCP)
• Stabilization PR: Stabilize io_error_other feature #115453

Unresolved Questions

• None yet.

[joshtriplett]

I'm not aware of any blockers on this one, and I know it'd be useful in real code.

@rust-lang/libs-api, shall we stabilize io::Error::other?

@rfcbot merge

[rfcbot]

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

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se
• @yaahc

Concerns:

• into-box resolved by Tracking Issue for io::Error::other #91946 (comment)

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.

[joshtriplett]

Unchecking my box, because I just realized this takes an Into, rather than just an Error, which precludes us from storing an error more efficiently (e.g. alternate boxes / thin boxes). I think this method should just take E: Error + ....

[joshtriplett]

@rfcbot concern into-box

[ibraheemdev]

Hm, one of the most common use cases for this is io::Error::other("something .... went wrong"), and strings don't implement Error.

[ibraheemdev]

@joshtriplett io::Error::new already takes E: Into<Box<dyn error::Error + Send + Sync>>

[yaahc]

@joshtriplett io::Error::new already takes E: Into<Box<dyn error::Error + Send + Sync>>

I don't think that necessarily means we should copy the existing API, we have a history of breaking patterns that we consider to have been mistakes, such as Arc<dyn Error> implementing Error whereas Box<dyn Error> instead impls From<E: Error> which precludes the possibility of it also implementing Error.

While the ship has certainly sailed for new and we will have to deal with explicit dependencies upon Box regardless, we don't need to add additional dependencies on Box if we think they were a mistake.

@joshtriplett do you have any specific ideas in mind for how we could store errors more efficiently here? cc @thomcc since I think you're the person who has most recently worked on the implementation. For my part I don't really see what we can gain here. I know that replacing E: Error + ... with E: Into<Box<dyn Error + ...>> will be backwards compatible, so I'm happy to vote to approve the more restrictive bound immediately and work on answering the Box dependency and optimization questions as a followup, but I suspect we will probably want to just go with the less restrictive bound in the end.

[thomcc]

So, yeah we could do some clever stuff in the impl to make it faster for E: Error + Send + Sync + 'static. Rather than Box<(ErrorKind, Box<dyn Error + ...>)> we could do ThinBox<(ErrorKind, dyn Error + ...)> (tuples for exposition only, currently we use a struct and would use one for this too).

I would be happy to implement this if we decided to go this route, but I don't think this is worth the hassle, honestly. We'd still have std::io::Error::into_inner, FWIW. So getting your data out on the other side requires boxing anyway (although we'd be able to use it for get_ref and such).

Overall, I do not feel strongly, but my vote would be to stick with Into<Box<dyn Error ...>> because of improved ergonomics.

[m-ou-se]

Hm, one of the most common use cases for this is io::Error::other("something .... went wrong"), and strings don't implement Error.

The problem is that such an api taking a string literal will be relatively inefficient, because we cannot specialize on the 'static lifetime, meaning that this will have to copy the entire string into a new allocation.

I believe this is exactly what @joshtriplett wanted to avoid with his proposal: to make sure it's only used for other(some_underlying_error) and not for other("some literal message").

There's no space inside an io::Error to store a full wide pointer (&'static str), so our internal hack for that is currently a macro (const_io_error) that puts a (ErrorKind, &'static str) into a static and then stores a reference to that in the io::Error. We can't do that through an api like io::Error::new_const_str(&'static str), because then there's no static ErrorKind+&str to refer to. io::Error::new_const_str<const S: &str>() could potentially work in the near future, though.

[joshtriplett]

I believe this is exactly what @joshtriplett wanted to avoid with his proposal: to make sure it's only used for other(some_underlying_error) and not for other("some literal message").

I do think it's perfectly fine to support a string or similar; that's not my concern here at all. I wanted to raise the question, before we commit to this API, of whether this API makes it harder for us to optimize io::Error further in the future.

I don't feel strongly about this. If folks feel either that this doesn't preclude such future optimizations or that other aspects of io::Error already preclude those optimizations, I don't want to block this on that basis.

@rfcbot resolved into-box