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

Box error alias #2820

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

richard-uk1
Copy link

@richard-uk1 richard-uk1 commented Nov 20, 2019

Rendered

Following on from a conversation on internals.rust-lang.org, I have written an RFC for adding BoxError to std::error. Although it is a small change, it may have a larger impact on the user experience if it becomes ubiquitous, and so I thought it was worth writing up the motivations and drawbacks in an RFC.

It's the first time I've done anything like this, so be kind to me! 😳

Copy link
Member

@shepmaster shepmaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other, similar changes throughout are not marked to reduce clutter, but they should all be addressed.

text/0000-box-error-alias.md Outdated Show resolved Hide resolved
text/0000-box-error-alias.md Outdated Show resolved Hide resolved
text/0000-box-error-alias.md Outdated Show resolved Hide resolved
text/0000-box-error-alias.md Outdated Show resolved Hide resolved
text/0000-box-error-alias.md Outdated Show resolved Hide resolved
text/0000-box-error-alias.md Outdated Show resolved Hide resolved
@RustyYato
Copy link

Can we also get?

type Result<T> = std::result::Result<T, BoxError>

@Centril Centril added T-libs-api Relevant to the library API team, which will review and decide on the RFC. A-type-alias Type alias related proposals & ideas labels Nov 20, 2019
@shepmaster
Copy link
Member

Can we also get?

type Result<T> = std::result::Result<T, BoxError>

My recent preference (and that of SNAFU) has been

type Result<T, E = BoxError> = std::result::Result<T, E>;

This way, you don't have to go all the way back to std::result::Result when you want a slightly different error type.

@richard-uk1
Copy link
Author

richard-uk1 commented Nov 22, 2019

@shepmaster

So this would look like

type BoxError = Box<dyn Error + Send + Sync>;

type Result<T, E = BoxError> = std::result::Result<T, E>;

in the module std::error?

edit typo

@shepmaster
Copy link
Member

Yep. Note that I'm not voting one way or other on the actual inclusion of this alias, just pointing out what I've found to be more ergonomic for other type aliases.

@dtolnay
Copy link
Member

dtolnay commented Nov 23, 2019

+@yoshuawuyts who was considering writing an RFC for a better type erased error type in the near future.

I think the rationale in this RFC against a dedicated erased error type of "it is not clear what the best design would be" and "it is not clear if one is necessary" are definitely not sufficient to rule out that idea. With that in mind, I would be opposed to accepting this RFC before making more progress on a type erased error type design, because I believe we would not want the standard library to expose both that and this type alias. We should expose one and it should be the one that we feel comfortable recommending for general use.

@epage
Copy link
Contributor

epage commented Nov 27, 2019

With that in mind, I would be opposed to accepting this RFC before making more progress on a type erased error type design, because I believe we would not want the standard library to expose both that and this type alias. We should expose one and it should be the one that we feel comfortable recommending for general use.

Especially if we provide a type Result<T, E = BoxError> = std::result::Result<T, E>;

How far can we get if this RFC instead provided a newtype? What might we want to change in the future that would be prevented?

@epage
Copy link
Contributor

epage commented Nov 27, 2019

Until we have specialization, my "vote" would be for a newtype that impl Error rather than impl From<E: Error> until specialization is stable is able to allow both or to defer this until specialization is stable if that isn't acceptable.

Personally, I feel providing shortcuts or simplifications in the stdlib for a type-erased error type that doesn't impl Error is harmful to the ecosystem. The benefit and curse of adding a type-erased error type to the stdlib is that its usage will spread because (1) we are endorsing its usage to the point people might not ask the questions they need to and (2) it'll be easy to naturally fall into using it (pit of success). One curse we'd run into without impl Error is that people will use it in [lib]s without knowing the ramifications, causing crate interoperability issues in the ecosystem. We can add a warning but that won't help people that don't land on the right page, are using stackoverflow, or are copying patterns from other crates they see.

@richard-uk1
Copy link
Author

richard-uk1 commented Nov 28, 2019

Not being able to implement From<Error> and Error is a pain, for sure.

I see the argument about it being confusing if you have both a BoxError and some opaque AnyError. Supposing hypothetically that we have specialization, what other benefits could the opaque AnyError have over the BoxError version? The advantage of BoxError is that it's simpler, being just a relabelling of a boxed error. As I see it they are

  1. You can make it smaller (a thin pointer rather than a fat pointer)
  2. You can overwrite stuff, like using the Display impl for your Debug to give better error messages in the fn main() -> Result<(), AnyError> case.

Am I missing any?

Edit: typos

@epage
Copy link
Contributor

epage commented Nov 29, 2019

Am I missing any?

There is the open question of whether to implicitly carry a Backtrace in an AnyError if the wrapper error doesn't have one.

@kornelski
Copy link
Contributor

kornelski commented Dec 1, 2019

Support for Result in fn main has been added specifically to make code examples clearer. Having this alias in std helps that goal further, since fn main() -> Result<(), BoxError> is drawing less unnecessary attention than the full type.

@mehcode
Copy link

mehcode commented Dec 8, 2019

I'd love to see something like this.

My preference would be to make the type a "primitive" and name it std::error ( and adding this to the prelude in the next epoch ). I don't think we should define how this type is implemented in the RFC, just that it "exists" and anything that is impl Error can ? to it. I can see two very different ways to do this: a type-erased Box error like in anyhow or an anonymous sum type ( if we ever get those ). The latter can be used to make std::error work for no_std. If the RFC and the exposed API doesn't make any assumptions on Box vs. anonymous sum type we can come back to that later.

I agree that at the same time, Result<T, E> should become Result<T, E=error>.

Furthermore I vote for taking anyhow::Error and just moving a core subset of it directly as std::error ( see https://github.com/dtolnay/anyhow ).


I'd like to take a second and note that I'm not capable of expressing just how significant and game changing this RFC would be. Daily, new Rust users run into issues with error types and not really understanding the intricacies with error enumerations and ?. I've seen many just use io::Result until it doesn't work and being confused.

@benkay86
Copy link

@shepmaster

So this would look like

type BoxError = Box<dyn Error + Send + Sync>;

I second making this type alias Send + Sync. Lots of examples in the Rust book just use Box<dyn Error> but that makes it really difficult to pass errors between threads, which is actually a very common use case. The vast majority of error types already satisfy this constraint anyway.

@Lokathor
Copy link
Contributor

Lokathor commented Dec 19, 2019

Future Possibility

In the future it may be possible to move the Error trait into alloc so that they're available to a wider portion of the ecosystem. In this case, the BoxError alias would also be moved.

@burdges
Copy link

burdges commented Dec 19, 2019

We cannot make std::error::Error fit into core because of std::backtrace::Backtrace, right?

If we had core::error::Error that avoided Backtrace then could we simply privileged error types that fit within three pointers like this?

use core::mem;
use core::raw::TraitObject;

const SMALL_ERROR_SIZE : usize = 3 *  mem::size_of::<usize>();

#[derive(Clone,Copy)]
pub struct SmallError {
    vtable: *mut (),
    data: [u8; SMALL_ERROR_SIZE],
}

impl Send for SmallError {}
impl Sync for SmallError {}

impl SmallError {
    pub fn new<E: Error+Clone+Copy+Send+Sync+Any+'static>(err: E) -> SmallError 
    // where mem::size_of::<E>() <= mem::size_of::<E>()  // sigh
    {
        assert!(mem::size_of::<E>() <= mem::size_of::<E>());
        let data = [0u8; SMALL_ERROR_SIZE],
        core::intrinsics::copy(&data as *const E, &mut data as *mut [u8; SMALL_ERROR_SIZE] as *mut E , 1);
        let vtable = unsafe { mem::transmute::<&mut dyn Error,TraitObject>(&mut dyn err).vtable };
        // mem::forget(err);
        SmallError { vtable, data }
    }
}

impl Deref for SmallError {
    type Target = dyn Error+Send+Sync+'static;  // Add +Clone+Copy+Any when mutli-trait objects exist
    fn deref(&self) -> &Self::Target {
        let SmallError { vtable, ref data } = self;
        mem::transmute::<TraitObject,&mut dyn Error>(TraitObject { vtable, data })
    }
}

impl DerefMut for SmallError {
    fn deref(&mut self) -> &mut Self::Target {
        let SmallError { vtable, ref data } = self;
        mem::transmute::<TraitObject,&mut dyn Error>(TraitObject { vtable, data })
    }
}

I suppose impl Error for SmallError works too of course.

@richard-uk1
Copy link
Author

I've been using anyhow::Error now for a while, and it does make working with errors really convenient, and it has a similar feel to BoxError, so the fact that its implementation is not specified and therefore changeable seems a clear advantage.

@dtolnay what do you think a MVP for an opaque type-erased error type in std would look like? Would it be anyhow::Error or a subset of its functionality?

@dtolnay
Copy link
Member

dtolnay commented Feb 9, 2020

I think it's early to push to stabilize anyhow::Error or a subset of its functionality in std. Every previous iteration that anyhow builds on has taken some time to understand the nuances and improve. I think if we still think anyhow is the best approach ~12 months after it was released, we could consider how to move forward.

For now I think the most important work toward a standard type-erased error type would be helping to get specialization mature enough that we could do a From<E> and Error impl on the same type.

@KodrAus KodrAus added the Libs-Tracked Libs issues that are tracked on the team's project board. label Jul 29, 2020
@KodrAus KodrAus added the A-error-handling Proposals relating to error handling. label Jul 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-error-handling Proposals relating to error handling. A-type-alias Type alias related proposals & ideas Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.