Move `Error` trait into core #3

yaahc · 2020-09-18T17:36:56Z

Building upon these prior issues:

From this comment Backtrace stabilization is currently blocked on prototyping one or more of the suggested solutions for moving the std::error::Error trait into core

That comment suggests three possible solutions.

1. Remove `backtrace` from `Error` and instead provide it via some generic context mechanism (RFC 2895).

2. Define a `Backtrace` trait in core and have an implementation of that trait in libstd. `Error::backtrace` would then return a `&dyn Backtrace`.

3. Define `Backtrace` in core but have it dispatch via an internal vtable. This is effectively the same as 2 but the trait is hidden. The `capture` method will have to be moved to a free function in libstd.

Option one is already known to work, and so it does not need to be prototyped. In addition to these backtrace related issues a prototype of the error trait in core will need to find a way to work around pub fn downcast<T: Error + 'static>(self: Box<Self>) -> Result<Box<T>, Box<dyn Error>> since Box won't be available in core. The top level comment in @withoutboats's stabilization PR goes into more detail on this second issue and outlines how a new core::error::Error implementation could use hooks that are defined in std, similar to panic hooks, to work solve the issues around Box.

Boats has already raised issues about option two in this comment so I think the best way forward is to start working on a core::error::Error trait and core::backtrace::Backtrace type that both rely on hooks / manual vtables to maintain backwards compatibility with the existing error trait.

The text was updated successfully, but these errors were encountered:

nagisa · 2020-09-19T12:47:09Z

I attempted to prototype the move to libcore with backtrace removed in this branch.

The mentioned downcast is indeed not at all an issue (I applied the suggestion I made in this comment), but

impl<'a> From<&str> for Box<dyn Error + Send + Sync + 'a> {

comes up as an issue. In particular it will trigger coherence's wrath with

error[E0119]: conflicting implementations of trait `core::convert::From<&str>` for type `boxed::Box<dyn core::error::Error + core::marker::Send + core::marker::Sync>`:
    --> library/alloc/src/errors.rs:66:1
     |
66   | impl<'a, E: Error + Send + Sync + 'a> From<E> for Box<dyn Error + Send + Sync + 'a> {
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `boxed::Box<dyn core::error::Error + core::marker::Send + core::marker::Sync>`
     |
    ::: library/alloc/src/boxed.rs:1155:1
     |
1155 | impl<'a> From<&str> for Box<dyn Error + Send + Sync + 'a> {
     | --------------------------------------------------------- first implementation here
     |
     = note: upstream crates may add a new impl of trait `core::error::Error` for type `&str` in future versions

yaahc · 2020-09-25T18:07:23Z

Here's a link to a related zulip discussion thread: https://rust-lang.zulipchat.com/#narrow/stream/257204-project-error-handling/topic/Stabilizing.20Backtrace.20.2F.20Core.20Error.20Proof.20of.20Concept/near/211291775

Once we've talked through potential solutions to the orphan rule issue more I'll come back and summarize the potential steps forward in the top level issue.

yaahc · 2020-09-26T17:42:05Z

I'm still unconvinced that hooks are the best solution as opposed to defining a Backtrace trait in core. Here is my analysis of the trade offs.

Comparison

Both traits and hooks are essentially ways to define a vtable
Traits carry the vtable as part of the object via a wide pointer
hooks store the vtable in a single static location
traits are dynamically dispatched via virtual calls
hooks are statically dispatched like any other non virtual function
With traits you can have many implementations of the vtable in the same binary
with hooks you can must have exactly one implementation, unless you never attempt to call the hooks
with traits have to pay the cost of virtual function calls
traits are likely simpler implementation wise
A trait based solution would require either a wide pointer on the stack or an extra layer of indirection

To my pattern matching brain traits feel like a universally quantified form of dynamic dispatch whereas these ad-hoc hooks feel like an existentially quantified form of const dynamic dispatch.

Previously @withoutboats raised concerns about stability when using traits. I don't think this is a fundamental issue though. We could twrap the trait in a newtype and make the trait itself unstable, which is essentially what we're doing with the hooks. Either way if the user is ever allowed to provide the vtable, whether via traits or hooks, adding new items to that vtable will require handling the default case.

The question is, which is more appropriate for backtraces?

What use cases are there for being able to work with multiple types of backtraces via a common interface?
- SpanTrace - doesn't seem useful here unless the generic member access RFC never merges
- Backtraces of frames of other languages via FFI? - Could this be handled by defining your own backtrace handler via the static hooks that is capable of understanding frames in of all the languages it will interact with?

eddyb · 2020-09-26T19:05:05Z

This is a full example of what @yaahc, @mystor and I have discussed (I don't plan to be actively involved, but felt like I needed to exemplify my suggestions): https://play.rust-lang.org/?version=nightly&mode=release&edition=2018&gist=63b4720d0347d05391643df9da08c72f

Also here's a more pseudocode version of it (and without most implementation details), to facilitate discussion:

mod core::panic {
    // perma(?)-unstable
    pub trait RawBacktraceImpl: fmt::Debug + fmt::Display + 'static {
        unsafe fn drop_and_free(self: *mut Self);
    }

    #[stable]
    pub struct Backtrace(*mut dyn RawBacktraceImpl);

    impl Backtrace {
        // perma(?)-unstable
        pub unsafe fn new(p: *mut dyn RawBacktraceImpl) -> Self {
            Self(p)
        }

        #[stable]
        pub fn capture() -> Option<Self> {
            extern "Rust" {
                #[lang_item = "backtrace_capture"]
                fn backtrace_capture() -> Option<Backtrace>;
            }
            unsafe { backtrace_capture() }
        }
    }

    impl !Send for Backtrace {}
    impl !Sync for Backtrace {}
    impl Drop for Backtrace {...}
    impl fmt::Debug for Backtrace {...}
    impl fmt::Display for Backtrace {...}
}

mod alloc::panic {
    #[stable]
    pub trait BacktraceImpl: fmt::Debug + fmt::Display + 'static {}

    impl<T: BacktraceImpl> From<Box<T>> for Backtrace {
        fn from(x: Box<T>) -> Self {
            struct Wrapper<T>(T);

            impl<T: BacktraceImpl> RawBacktraceImpl for Wrapper<T> {...}
            impl<T: fmt::Debug> fmt::Debug for Wrapper<T> {...}
            impl<T: fmt::Display> fmt::Display for Wrapper<T> {...}

            unsafe {
                Backtrace::new(Box::into_raw(x) as *mut Wrapper<T> as *mut dyn RawBacktraceImpl)
            }
        }
    }
}

mod std::panic {
    #[lang_item = "backtrace_capture"]
    fn backtrace_capture() -> Option<Backtrace> {
        struct StdBacktrace;

        impl fmt::Debug for StdBacktrace {...}
        impl fmt::Display for StdBacktrace {...}
        impl BacktraceImpl for StdBacktrace {}

        Some(Box::new(StdBacktrace).into())
    }
}

yaahc · 2021-10-08T22:44:52Z

Status Update

We're getting a clearer and clearer picture of what's needed to move the error trait into core so here is a list of the specific blockers that need to be resolved still:

Integrate negative trait impls with Coherence checks

Currently tracked in:

This feature is necessary to handle the following from impl on box.

impl From<&str> for Box<dyn Error> { ... }

Without having negative traits affect coherence moving the error trait into core and moving that From impl to alloc will cause the from impl to no longer compile because of a potential future incompatibility. The compiler indicates that &str could introduce an Error impl in the future, and thus prevents the From impl in alloc that would cause an overlap with From<E: Error> for Box<dyn Error>. Adding impl !Error for &str {} with the negative trait coherence feature will disable this error by encoding a stability guarantee that &str will never implement Error, making the From impl compile.

Resolve `fn backtrace` in core issue

After fixing the previous issue it will still not be possible to move Error straight into core because it has a method that includes the Backtrace type in it's signature, which is also confined to std rather than core. There are two potential solutions to this we're currently considering.

Move the Backtrace type to core using lang items to define it's functionality in std or provide a dummy impl / custom impl when in no_std contexts. Draft PR
Remove fn backtrace from the error trait in favor of generic member access. RFC

We are currently favoring the second option, removing fn backtrace entirely. However this approach is blocked by landing generic member access.

Generic Member Access

In order to remove the backtrace function we need to add an alternative API that maintains equivalent functionality. The Generic member access RFC proposes such a mechanism. This RFC is more or less ready to be implemented but is currently blocked on splitting out the type erasure mechanisms it introduces into its own RFC and landing those first, due to their complexity, and because they're partially justified by usecases unrelated to error handling.

@nrc has written a draft of this RFC: nrc/rfcs#1

Downcast

The last issue is the presence of downcast APIs associated with dyn Errors, some of these APIs reference the Box type, which cannot be moved to core. We currently believe that these methods can be moved to alloc via an impl Box<dyn Error> { ... } block, but this has not been tested yet. Once we resolve the previous three issues we will attempt to move the error trait to core and see if any additional unforeseen issues pop up.

…it, r=nikomatsakis Implement coherence checks for negative trait impls The main purpose of this PR is to be able to [move Error trait to core](rust-lang/project-error-handling#3). This feature is necessary to handle the following from impl on box. ```rust impl From<&str> for Box<dyn Error> { ... } ``` Without having negative traits affect coherence moving the error trait into `core` and moving that `From` impl to `alloc` will cause the from impl to no longer compiler because of a potential future incompatibility. The compiler indicates that `&str` _could_ introduce an `Error` impl in the future, and thus prevents the `From` impl in `alloc` that would cause overlap with `From<E: Error> for Box<dyn Error>`. Adding `impl !Error for &str {}` with the negative trait coherence feature will disable this error by encoding a stability guarantee that `&str` will never implement `Error`, making the `From` impl compile. We would have this in `alloc`: ```rust impl From<&str> for Box<dyn Error> {} // A impl<E> From<E> for Box<dyn Error> where E: Error {} // B ``` and this in `core`: ```rust trait Error {} impl !Error for &str {} ``` r? `@nikomatsakis` This PR was built on top of `@yaahc` PR rust-lang#85764. Language team proposal: to rust-lang/lang-team#96

spastorino · 2021-10-23T19:57:39Z

Implement coherence checks for negative trait impls was just merged rust-lang/rust#90104

yaahc · 2021-10-26T21:48:51Z

Now trialing moving Error into core by just ripping out fn backtrace

WIP PR: rust-lang/rust#90328

yaahc · 2021-10-27T20:03:24Z

Regarding this comment, rust-lang/rust#72981 (comment), the trial from rust-lang/rust#90328 seems to have shown that moving the impl to be on Box<dyn Error> is still a breaking change, since before you could call downcast as <dyn Error>::downcast(err).

error[E0599]: no function or associated item named `downcast` found for trait object `dyn core::error::Error` in the current scope
    --> library/alloc/src/boxed.rs:2101:22
     |
2101 |         <dyn Error>::downcast(err).map_err(|s| unsafe {
     |                      ^^^^^^^^ function or associated item not found in `dyn core::error::Error`

error[E0599]: no function or associated item named `downcast` found for trait object `dyn core::error::Error` in the current scope
    --> library/alloc/src/boxed.rs:2115:22
     |
2115 |         <dyn Error>::downcast(err).map_err(|s| unsafe {
     |                      ^^^^^^^^ function or associated item not found in `dyn core::error::Error`

For more information about this error, try `rustc --explain E0599`.

The original plan for this was to leverage a lang-item or something to allow the incoherent impl on dyn Error from the alloc crate despite Error being in core. Going to try that now to see if that can easily resolve the issue.

yaahc · 2021-12-09T00:17:17Z

Update

I was able to get rust-lang/rust#90328 compiling, now testing everything but hopefully that should all be sorted, which means the negative trait impls in coherence and downcast issues have both been resolved 🎉.

The only remaining blocker to moving the Error trait into core is fn backtrace and code review on the trial PR which will likely turn into the actual PR for moving it once I integrate the generic member access changes (assuming those are how we resolve the fn backtrace issue). The top priority now is getting rust-lang/rfcs#3192 and rust-lang/rfcs#2895 finalized, approved, merged, and implemented. And hopefully we can get the initial implementations done immediately and available on nightly while we hammer out the API details in the RFCs.

spastorino · 2021-12-09T13:34:32Z

Ohh I thought negative trait in coherence wasn't going to work for your use case because it doesn't work in some cases. Cool if the current solution does work for you :). Anyway, we are going to be making some modifications to negative traits in coherence. More info https://rust-lang.github.io/negative-impls-initiative/explainer/coherence-check.html

yaahc · 2022-07-15T21:22:59Z

Update

There's been a lot of progress in the last few months. Since the last update we've

Merged the provider RFC: Add provide_any module to core rfcs#3192
Implemented the provider API on nightly: Add the Provider api to core::any rust#91970
Fixed an issue with path element stability so the Error trait can be moved to core unstably: Support unstable moves via stable in unstable items rust#95956
Integrated the provider API with the error trait on nightly: Add provider API to error trait rust#98072

At this point there are no remaining blockers per-se, though there are due diligence steps that still need to be done.

Updating the RFC for generic member access: RFC: Generic member access for dyn Error trait objects rfcs#2895 to reflect changes in the API since it's original proposal
Setting up a tracking issue for the provider api integration with the error trait
Actually write the PR moving the error trait to core

With any luck and assuming I don't have too many distractions pop up I should be able to have this done asap, hopefully within the next few days.

ciphergoth · 2022-09-07T00:57:56Z

It looks like a relevant change has landed in Rust - does this mean that Rust 1.65 will have the Error trait in core? If so woohoo!

TimDiekmann · 2022-09-07T07:25:16Z

Amazing work @yaahc, thank you very much for your effort!

lygstate · 2022-09-07T07:34:42Z

Amazing work @yaahc, thank you very much for your effort!
Waiting for it for a long time

runiq · 2022-09-07T07:38:02Z

Wow, how did this fly under the radar? Amazing work, Jane! Thank you so much! <3

TheButlah · 2022-10-06T09:11:26Z

It looks like the error can be used in core but is marked as unstable - can someone confirm this is what people meant by

does this mean that Rust 1.65 will have the Error trait in core?

I read that and thought it was stable - happy to use it with unstable features if necessary :)
Sorry if I missed some context, I just found out about this issue from the unstable warning

bjorn3 · 2022-10-06T09:45:08Z

Beta has it behind the error_in_core feature gate.

dfreese · 2023-01-17T20:54:23Z

For completeness, the tracking issue for error_in_core is rust-lang/rust#103765

This comment has been minimized.

Sign in to view

yaahc mentioned this issue Sep 26, 2020

Move std::io::Error out of std. #11

Open

yaahc mentioned this issue Oct 1, 2020

Start working on proof of concept for exposing Backtrace in core rust-lang/rust#77384

Closed

KodrAus mentioned this issue Oct 12, 2020

Stabilize the backtrace feature. rust-lang/rust#72981

Closed

lygstate mentioned this issue Feb 16, 2021

[Wip]Tentative add alloc::io module. rust-lang/rust#77922

Closed

yaahc mentioned this issue May 27, 2021

Implement coherence checks for negative trait impls rust-lang/rust#85764

Closed

yaahc changed the title ~~Prototype core::error::Error to prove that stabilizing fn backtrace will not prevent a later move of the error trait to core~~ Move Error trait into core Oct 8, 2021

yaahc mentioned this issue Oct 19, 2021

Provide implementations of From<&str> and From<String> for Report eyre-rs/eyre#60

Closed

spastorino mentioned this issue Oct 20, 2021

Implement coherence checks for negative trait impls rust-lang/rust#90104

Merged

tmandry mentioned this issue Nov 10, 2021

AsyncRead, AsyncWrite traits rust-lang/wg-async#23

Closed

TimDiekmann mentioned this issue Jan 3, 2022

Add Provider API for context information on errors (and possibly other things) hashintel/hash#133

Merged

phil-opp mentioned this issue Jan 31, 2022

Building std without support for certain lang items rust-lang/wg-cargo-std-aware#79

Closed

kevinmehall mentioned this issue Apr 16, 2022

Add no_std support kevinmehall/rust-peg#294

Open

yaahc mentioned this issue Jul 15, 2022

Normalization support no-std eyre-rs/eyre#83

Merged

BastiDood mentioned this issue Aug 25, 2022

feat!: allow usage in no_std environments niklasf/shakmaty#61

Merged

TimDiekmann mentioned this issue Sep 7, 2022

Support core::error::Error hashintel/hash#1038

Merged

adsnaider mentioned this issue Oct 4, 2022

add #![no_std] support dtolnay/thiserror#196

Closed

AurevoirXavier mentioned this issue Oct 9, 2022

Use thiserror when it supports no_std hack-ink/array-bytes#18

Open

3 tasks

liquidev mentioned this issue Oct 31, 2022

Support #![no_std] mica-lang/mica#93

Open

claravanstaden mentioned this issue Nov 17, 2022

add no-std ralexstokes/ssz-rs#25

Merged

daviddrysdale mentioned this issue Dec 19, 2022

Implement Error for CoseError google/coset#60

Closed

plafer mentioned this issue Nov 8, 2023

Use thiserror consistently across the crate 0xPolygonMiden/miden-node#62

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move `Error` trait into core #3

Move `Error` trait into core #3

yaahc commented Sep 18, 2020 •

edited

nagisa commented Sep 19, 2020

yaahc commented Sep 25, 2020

This comment has been minimized.

yaahc commented Sep 26, 2020 •

edited

eddyb commented Sep 26, 2020

yaahc commented Oct 8, 2021 •

edited by JDuchniewicz

spastorino commented Oct 23, 2021

yaahc commented Oct 26, 2021 •

edited

yaahc commented Oct 27, 2021 •

edited

yaahc commented Dec 9, 2021 •

edited

spastorino commented Dec 9, 2021

yaahc commented Jul 15, 2022 •

edited

ciphergoth commented Sep 7, 2022

TimDiekmann commented Sep 7, 2022

lygstate commented Sep 7, 2022

runiq commented Sep 7, 2022

TheButlah commented Oct 6, 2022 •

edited

bjorn3 commented Oct 6, 2022

dfreese commented Jan 17, 2023

Move Error trait into core #3

Move Error trait into core #3

Comments

yaahc commented Sep 18, 2020 • edited

nagisa commented Sep 19, 2020

yaahc commented Sep 25, 2020

This comment has been minimized.

yaahc commented Sep 26, 2020 • edited

Comparison

eddyb commented Sep 26, 2020

yaahc commented Oct 8, 2021 • edited by JDuchniewicz

Status Update

Integrate negative trait impls with Coherence checks

Resolve fn backtrace in core issue

Generic Member Access

Downcast

spastorino commented Oct 23, 2021

yaahc commented Oct 26, 2021 • edited

yaahc commented Oct 27, 2021 • edited

yaahc commented Dec 9, 2021 • edited

spastorino commented Dec 9, 2021

yaahc commented Jul 15, 2022 • edited

ciphergoth commented Sep 7, 2022

TimDiekmann commented Sep 7, 2022

lygstate commented Sep 7, 2022

runiq commented Sep 7, 2022

TheButlah commented Oct 6, 2022 • edited

bjorn3 commented Oct 6, 2022

dfreese commented Jan 17, 2023

Move `Error` trait into core #3

Move `Error` trait into core #3

yaahc commented Sep 18, 2020 •

edited

yaahc commented Sep 26, 2020 •

edited

yaahc commented Oct 8, 2021 •

edited by JDuchniewicz

Resolve `fn backtrace` in core issue

yaahc commented Oct 26, 2021 •

edited

yaahc commented Oct 27, 2021 •

edited

yaahc commented Dec 9, 2021 •

edited

yaahc commented Jul 15, 2022 •

edited

TheButlah commented Oct 6, 2022 •

edited