Start working on proof of concept for exposing Backtrace in core

[yaahc]

Implementation of the proof of concept described in rust-lang/project-error-handling#3

[rust-highfive]

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

[ghost]

@rustbot assign @kw-fn

[ghost]

@rustbot assign @yaahc

[yaahc]

Another thing I've been thinking about. I'm worried that a proof of concept is an insufficient step in proving that backtrace can be moved to core, and that we might also end up wasting effort or never see this project finished. As such I feel like this PR should aim to be the prototype and then eventually the final implementation that we merge. Possibly with an RFC based on the first pass implementation to approve the full design.

[yaahc]

Update

I talked to the libs team today about this and have a plan now. More or less based on what I've said above I'm going to start with trying to aim straight for what I think the final implementation should look like, and not just for backtrace but also for the error trait in core. The plan is:

1. implement backtrace based on how #[global_allocator] and #[default_lib_allocator] work, with a new lang item
   1. where we have a global that owns the implementation of a trait for how to construct backtraces
   2. the global is declared in core and instantiated in std.
   3. the compiler will provide a default impl that always captures disabled backtraces, for when std is not linked in
   4. and used to implement all of the interfaces currently on Backtrace
2. Move Error to core, add a second lang item for its downcast impl, so that the downcast impl live in std while the error trait is defined in core.

The hope is that, given what we know now, we think this should be a relatively small amount of work. If everything goes according to plan this shouldn't delay the backtrace stabilization, and would get us Error in core much sooner. We are worried about possible unforeseen issues with using lang items though, so if issues pop up I will switch gears to focus on backtrace stabilization and focus on unblocking that rather than completing the whole project upfront.

[bjorn3]

nit: missing trailing newline

[bjorn3]

Can you add a Send and Sync bound?

[Aaron1011]

@yaahc - Are you still working on this PR? If not, I'd be interested in continuing work on it.

[yaahc]

@Aaron1011 Right now this PR is on pause because we figured the next step would be to write an RFC based on this design.

@JDuchniewicz has started working on that RFC here: https://github.com/JDuchniewicz/rfcs/blob/backtrace-in-core/0000-backtrace-in-core.md

That said, I'd love to have any help we can get pushing this PR into a more complete state, I'm just worried that it may be for naught if we end up rejecting this design.

[bjorn3]

From the RFC:

only that it is moved to core and if std is not linked, automatic backtrace handlers will be generated

How will these be generated? #86844 tries to remove the allocator shim allowing --emit obj objects that link to liballoc to be linked directly without having rustc generate the allocator shim. This is currently already working for libcore, but a backtrace handler shim would now start requiring a shim to even be able to link to libcore, which is an impossible to avoid dependency. There are #![no_std] projects that don't use liballoc and link the object files themself that would break because of this as rustc doesn't get the chance to generate any shim.

[yaahc]

From the RFC:

only that it is moved to core and if std is not linked, automatic backtrace handlers will be generated

How will these be generated?

Not sure. This is one of the areas I was worried about and figured might tank this design. I don't really have a solution and was hoping people with more familiarity with linking would be able to help me figure out if it was viable.

[bjorn3]

Weak linkage would be the standard answer to implement this in different languages I think. There are some targets supported by rustc that don't support weak linkage though I believe.

[yaahc]

Yea, that's the same thing I ran into when trying weak linkage early in the process.

I think I need to get a better idea of how this approach could cause issues for no_std projects. The current impl is closer to the #[panic_handler] attribute than the allocator shim as far as I can tell, but as is it still breaks no_std projects because they would need to define the hooks when previously they didn't, and I don't know if there's a panic=abort equivalent we could add for backtrace to indicate that the application never intends to capture backtraces. Either way @dtolnay is gonna look into getting an engineer working on this full time so we will have some help digging through the trade offs soon I hope.

[yaahc]

Closing this as no longer necessary. May re-open in the future if we end up deciding to keep fn backtrace on the Error trait.

[dvc94ch]

@yaahc can you summarize the decision for people not intimately familiar with all the discussions? Is rust not going to get support for backtraces outside of nightly?

[yaahc]

@yaahc can you summarize the decision for people not intimately familiar with all the discussions? Is rust not going to get support for backtraces outside of nightly?

Oh, sure. Apologies for not doing so here. I'd left an explanation in the associated RFC but didn't think to reproduce it here: rust-lang/rfcs#3156 (comment)

---

from the above link

What's the current status of this RFC?

The current status is that we don't intend to merge this RFC. It was written as one of the potential paths to moving error into core assuming generic member access didn't work out but at this point it seems extremely likely that we will merge the generic member access RFC once we finish the provider RFC. That RFC provides a superset of the functionality provided by fn backtrace() making it unnecessary. Once we have generic member access we will remove fn backtrace and immediately move Error into core.

For context:

• RFC: Generic member access for dyn Error trait objects rfcs#2895
• Add provide_any module to core rfcs#3192
• Move the Error trait into core (without fn backtrace) #90328
• PoC: implement core::panic::panic_error yaahc/rust#1

---

So to summarize the summary. We will have support for extracting backtraces from Error, but it will go through a different method as outlined in the generic member access RFC. As a quick example, instead of this:

fn extract_backtrace(error: &dyn Error) -> Option<&Backtrace> {
    error.backtrace()
}

We will use this:

fn extract_backtrace(error: &dyn Error) -> Option<&Backtrace> {
    error.context_ref::<Backtrace>()
}

[taqtiqa-mark]

Hmmm, the first example I believe I could/would guess. I don't believe I would ever guess the second.... Likely contributing to increasing the Rust-lang reputation as difficult or at least unintuitive?
My 2c.

[yaahc]

Hmmm, the first example I believe I could/would guess. I don't believe I would ever guess the second.... Likely contributing to increasing the Rust-lang reputation as difficult or at least unintuitive? My 2c.

I appreciate the feedback, and yea, this is something I've been worried about. I'll keep trying to think about what I can do to make it easier to learn / understand / intuit, but I'm not really sure what I can do other than focus on documentation and learning resources. I think there may also possibly be a more intuitive / descriptive name than context_ref/context_value but so far I've struggled to come up with anything that is significantly more compelling...

And even ignoring the Backtrace in core vs generic member access tradeoff, we're still going to want to add the latter API because it's independently motivated by wanting to be able to support more types than just Backtrace. We'd still have to grapple with this additional complexity even if we can find a way to implement Backtrace in core and keep fn backtrace on Error without requiring no_std binaries implement new useless lang items. I expect it's probably still better to just deduplicate the APIs and focus on teaching the one that is more powerful. It's a difficult balance :/

[Alxandr]

Not sure if this has been discussed (I didn't see it here) - but might it be a good idea to provide a backtrace method on &dyn Error that calls this context-method under the hood? As this is probably going to be the most used property on Error, and would provide some ease of discovery?

[bjorn3]

The problem is that if Error is moved to libcore, Backtrace would then need to be too. However backtraces require an unwinder and some way tk ask the os for the unwind tables, which isn't possible with libcore.

[tarcieri]

Perhaps Backtrace could have a constructor method which extracts from a dyn Error context, e.g. Backtrace::from_error?

[dvc94ch]

I don't think this has to be overanalyzed. The main goal is that people can finally debug their applications without compiling with nightly and then realizing that some dependency did not capture a backtrace, then having to clone/patch the crate and unwrap all the ?'s.

It doesn't have to be beginner friendly to access the Backtrace as all beginners want to do is {} or {:?}

[tarcieri]

I think the most important things are to have backtrace-aware exception loggers, and as an added bonus where possible error reporting service hooks (e.g. Airbrake, Datadog, LogRocket, Sentry, Rollbar) which can accept a structured backtrace as part of an error report.

Ideally one should not be handrolling code to spelunk through a dyn Error to extract the backtrace. There should be a simple and obvious way to print the error along with its associated context, which may or may not include a backtrace.

all beginners want to do is {} or {:?}

👍

[taqtiqa-mark]

I'll keep trying to think about what I can do to make it easier to learn / understand / intuit ... and focus on teaching the one that is more powerful. It's a difficult balance :/

Agreed. "There are no solutions, only trade-offs"

Here perhaps the doc's can motivate the novice by presenting an example exhibiting the power? Showing they have head room, and trust they'll be able to recognize the forthcoming "Productivity" or "Performance" the example demonstrates.

If a new user is looking at Rust - Performance. Reliability. Productivity. - they likely have performance on their priority list. Hence, they may be open to accepting the tradeoff if they are a little less productive to start with.

[dvc94ch]

I also don't think rust is that hard for beginners, even though it has that rap. You can clone your way out of lifetime issues, ownership is quite an intuitive concept, and the type system is built in a way that you can learn over time. Contrasting it with Haskell for example where everything is built with monads and transformers and applicative monoids. Either you know what all those things are or you'll have a hard time with reading Haskell code.

[yaahc]

Not sure if this has been discussed (I didn't see it here) - but might it be a good idea to provide a backtrace method on &dyn Error that calls this context-method under the hood? As this is probably going to be the most used property on Error, and would provide some ease of discovery?

This is a great idea! And no, I don't recall anyone who has proposed it, people have suggested adding an extension trait but this seems like the much more obviously correct answer IMO now that you mention it.

There are currently a couple of technical hurdles in the way. We already have something like this for downcast where we have to use a lang item to let the downcast methods live in alloc that include Box in their API. The only problem is Backtrace would not be able to live in alloc either, and I don't believe the compiler currently supports using lang items for splitting an inherent impl across three crates. The current interface only takes two impl IDs. As far as I know there's no reason that we couldn't extend this logic to allow three different impls, so this is probably something worth bringing up with the compiler team and opening an issue for. Alternatively we could do this as part of the proposed merger of std, alloc, and core under --build-std which would supersede the need for lang items, but I expect that approach to be much further away so I'd personally prefer to extend lang items to serve this usecase.

Edit: I just talked to @lcnr about this and he brought up that the compiler team has previously discussed making lang items apply to the crates themselves rather than the individual impls so you can put inherent impls across any of the three crates we have, and he's also indicated that he thinks this will be a relatively easy change and plans to do it this week, so we should absolutely be able to do this, thank you for the suggestion @Alxandr!

[bjorn3]

I don't believe the compiler currently supports using lang items for splitting an inherent impl across three crates.

AFAIK rustc does allow it. The float types are primitives and thus not defined in any existing crate. They have inherent impls in both libcore and liballoc.

[yaahc]

I don't believe the compiler currently supports using lang items for splitting an inherent impl across three crates.

AFAIK rustc does allow it. The float types are primitives and thus not defined in any existing crate. They have inherent impls in both libcore and liballoc.

That is consistent with my understanding but the issue is we need libcore, liballoc, and libstd, not just libcore and liballoc. I tried double checking the impl and it looks like it is still only two impls to me, though surprisingly the second one appears to be in libstd not liballoc, but either way I don't understand how rustc would allow this.

from compiler/rustc_hir/src/lang_items.rs:

    F32,                     sym::f32,                 f32_impl,                   Target::Impl,           GenericRequirement::None;
    F64,                     sym::f64,                 f64_impl,                   Target::Impl,           GenericRequirement::None;
    F32Runtime,              sym::f32_runtime,         f32_runtime_impl,           Target::Impl,           GenericRequirement::None;
    F64Runtime,              sym::f64_runtime,         f64_runtime_impl,           Target::Impl,           GenericRequirement::None;

from compiler/rustc_typeck/src/coherence/inherent_impls.rs:

            ty::Float(ty::FloatTy::F32) => {
                self.check_primitive_impl(
                    item.def_id,
                    lang_items.f32_impl(),
                    lang_items.f32_runtime_impl(),
                    "f32",
                    "f32",
                    item.span,
                    assoc_items,
                );
            }

from library/std/src/f32.rs:

#[cfg(not(test))]
#[lang = "f32_runtime"]
impl f32 {

from library/core/src/num/f32.rs

#[lang = "f32"]
#[cfg(not(test))]
impl f32 {

The limiting factor as I understand it is the signature of check_primative_id only taking two DefIDs, though I'm not positive I understand how the impl_def_id relates to the lang_def_ids. I think the impl_def_id refers to the type itself, in this case f32, and the lang_def_ids refer to the two impl blocks with lang item attributes attached, but I'm certainly no expert.

    fn check_primitive_impl(
        &self,
        impl_def_id: LocalDefId,
        lang_def_id: Option<DefId>,
        lang_def_id2: Option<DefId>,
        lang: &str,
        ty: &str,
        span: Span,
        assoc_items: &[hir::ImplItemRef],
    ) {

[bjorn3]

You can add an extra paramater to check_primitive_impl I think.

[yaahc]

You can add an extra paramater to check_primitive_impl I think.

Ah, yea that was my original plan until @lcnr said he and the compiler team had a better plan that would supercede these impl lang items entirely. If that ends up running into issues though adding a parameter to the method will definitely be the approach I end up taking here.

[lcnr]

opened rust-lang/compiler-team#487 and will work on implementing this over the next weeks

[taqtiqa-mark]

this is something I've been worried about ...
...
... I think there may also possibly be a more intuitive / descriptive name than context_ref/context_value but so far I've struggled to come up with anything that is significantly more compelling

I used to think the 'Only two things in CS are hard...' quip was a pointed jab. Until I had to do it :)

Caveat: GitHub doesn't show any code for functions context_* so the following is speculative.

To my mind context_* is going to be encompassing (back, forward, sideways etc), backtrace is unidirectional.

If the docs say that context_* can provide you with representation(s) of past-state and current-state (and/or future-state) then the idea is intuitive - backtrace is a special instance of that.

If not, and context_* only relates to past-state, then my expectations about context_* are out of alignment with what it does.
In case that is the situation: When I look at lists of synonyms (YMV) for "context" and "backtrace" the only synonym for "context" that intuitively (to my mind) connects to "backtrace" is background. Not sure if background_* makes sense in the the context (sorry) of what these context_* functions are intended to do, i.e. going forward:

fn extract_backtrace(error: &dyn Error) -> Option<&Backtrace> {
    error.background_ref::<Backtrace>()
}

[yaahc]

@taqtiqa-mark ah, yea, context isn't meant to be a synonym to backtrace or directly related to it. Instead it's supposed to be a generalization, where you can think of backtrace as a single kind of context on an error that you might care about. Context in this case meaning "information relevant to an error that is not itself part of the error message". Backtrace is context in so far as it acts like a map describing where in your codebase the error originated from, but the .context_* methods are meant to support more types than just backtrace. For example we want to be able to use it to grab things like ExitCodes from errors in std::process::Termination or things like suggestions and notes which could be displayed after the set of error messages in an error report, such as in this example from docs.rs/color-eyre:

color-eyre on  hooked [$!] is 📦 v0.5.0 via 🦀 v1.44.0
❯ cargo run --example custom_section
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/examples/custom_section`
Error:
   0: Unable to read config
   1: cmd exited with non-zero status code

Stderr:
   cat: fake_file: No such file or directory

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ SPANTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

   0: custom_section::output2 with self="cat" "fake_file"
      at examples/custom_section.rs:14
   1: custom_section::read_file with path="fake_file"
      at examples/custom_section.rs:58
   2: custom_section::read_config
      at examples/custom_section.rs:63

Suggestion: try using a file that exists next time

The Suggestion, stderr, and spantrace are all relevant to the error being reported here, but they are potentially too big to fit into the error messages themselves, so we're adding the .context_ methods to make it so we can grab that data separately from the error messages when reporting an error to a user, log file, or developer.