Stabilize backtrace

[tbodt]

This PR stabilizes the std::backtrace module. As of #99431, the std::Error::backtrace item has been removed, and so the rest of the backtrace feature is set to be stabilized.

Previous discussion can be found in #72981, #3156.

Stabilized API summary:

pub mod std {
    pub mod backtrace {
        pub struct Backtrace { }
        pub enum BacktraceStatus {
            Unsupported,
            Disabled,
            Captured,
        }
        impl fmt::Debug for Backtrace {}
        impl Backtrace {
            pub fn capture() -> Backtrace;
            pub fn force_capture() -> Backtrace;
            pub const fn disabled() -> Backtrace;
            pub fn status(&self) -> BacktraceStatus;
        }
        impl fmt::Display for Backtrace {}
    }
}

@yaahc

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @m-ou-se (or someone else) soon.

Please see the contribution instructions for more information.

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[tbodt]

r? @yaahc

[tbodt]

Stabilization report

Implementation history

• Early history is summarized at https://github.com/rust-lang/rust/pull/72981#issue-630865543
• The libs team decided to defer to the error handling team on the above stabilization PR https://github.com/rust-lang/rust/pull/72981#issuecomment-880144242
• The error handling team ultimately decided to take another approach to exposing backtraces on errors, replacing std::Error::backtrace() with std::Error::provider https://github.com/rust-lang/rust/pull/99431
• With this out of the way, we are free to stabilize the rest of the backtrace feature, i.e. the API for actually capturing backtraces

API summary

Moved to PR description: https://github.com/rust-lang/rust/pull/99573#issue-1313682767

Experience report

Used in anyhow: https://github.com/dtolnay/anyhow/blob/ffb25df68acb80ddf195f5e3a91eeb0b66e2b014/src/error.rs#L828. If the backtrace feature isn't available, anyhow has an option to use an implementation of the exact same API on top of the backtrace crate: https://github.com/dtolnay/anyhow/blob/ffb25df68acb80ddf195f5e3a91eeb0b66e2b014/src/backtrace.rs#L63.

[yaahc]

@rfcbot merge

[rfcbot]

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

• [x] @Amanieu
• [x] @BurntSushi
• [x] @dtolnay
• [ ] @joshtriplett
• [ ] @m-ou-se
• [x] @yaahc

No concerns currently listed.

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.

[tbodt]

The implementation is made much more difficult by https://github.com/rust-lang/rust/pull/99212 not being available in the bootstrap compiler. If one simply does a partial stabilization, the result won't compile with the bootstrap compile. So I've had to make the entire stabilization conditional on cfg(not(bootstrap)). Is this reasonable? I think this means that after #99212 makes into bootstrap, there will be another cleanup step where we make it unconditionally stable, then wait for another bootstrap compiler update, then finally remove the cfg_attr(bootstrap, feature(backtrace)).

[tbodt]

This is also blocked by anyhow's deny(warnings). Stabilizing a feature results in a warning in any crate that uses it, recommending that the #[feature()] be removed. But anyhow has #[deny(warnings)], and it apparently blocks CI.

Edit: that's a doctest, not anyhow :facepalm:

[yaahc]

The implementation is made much more difficult by #99212 not being available in the bootstrap compiler. If one simply does a partial stabilization, the result won't compile with the bootstrap compile. So I've had to make the entire stabilization conditional on cfg(not(bootstrap)). Is this reasonable? I think this means that after #99212 makes into bootstrap, there will be another cleanup step where we make it unconditionally stable, then wait for another bootstrap compiler update, then finally remove the cfg_attr(bootstrap, feature(backtrace)).

Everything being under bootstrap cfgs seems reasonable to me. Additionally, I wouldn't stress over the ensuing bootstrap updates that would be required. The process^1 for updating the bootstrap cfgs is a separate one that doesn't really depend on the features that temporarily required bootstrap cfgs, so that should be handled by whoever is responsible for the next bootstrap update without you having to be involved any further.

[tbodt]

It might not actually be necessary to put it all in bootstrap - I got CI to pass. But that's not the full bors CI...

[tbodt]

@bors try

[bors]

@tbodt: :key: Insufficient privileges: not in try users

[yaahc]

@bors try

[bors]

:hourglass: Trying commit d9f2031279f9d33c61a315343467f0d8ea04dab3 with merge caecccbf2290905c3eb158421367c8817ab08bed...

[bors]

:sunny: Try build successful - checks-actions Build commit: caecccbf2290905c3eb158421367c8817ab08bed (caecccbf2290905c3eb158421367c8817ab08bed)

[tbodt]

So I've had to make the entire stabilization conditional on cfg(not(bootstrap)).

Guess I didn't have to! Only had to fix the doctest, it seems.

[BurntSushi]

For anyone else who is confused by the "partial" description here, it looks like it refers to the fact that the std::error::Error::backtrace() method is not included here (it is being removed, as planned). And in particular, this PR appears to stabilize the full std::backtrace module.

[yaahc]

For anyone else who is confused by the "partial" description here, it looks like it refers to the fact that the std::error::Error::backtrace() method is not included here (it is being removed, as planned). And in particular, this PR appears to stabilize the full std::backtrace module.

It also leaves out the BacktraceFrame related apis.

[CryZe]

I may miss something but does it make sense for disabled to be a function as opposed to an associated constant BackTrace::DISABLED?

[tbodt]

I may miss something but does it make sense for disabled to be a function as opposed to an associated constant BackTrace::DISABLED?

Backtrace::disabled() is for constructing a backtrace whose status() is Backtrace::DISABLED.

[CryZe]

I may miss something but does it make sense for disabled to be a function as opposed to an associated constant BackTrace::DISABLED?

Backtrace::disabled() is for constructing a backtrace whose status() is Backtrace::DISABLED.

You mean BacktraceStatus::Disabled. I'm proposing Backtrace::DISABLED, instead of Backtrace::disabled(). A const fn with no parameters is equivalent to a constant, so might as well make it one.

[tbodt]

Oh, my bad.

[rfcbot]

:bell: This is now entering its final comment period, as per the review above. :bell:

[yaahc]

I may miss something but does it make sense for disabled to be a function as opposed to an associated constant BackTrace::DISABLED?

This was discussed^1 when we originally introduced the function but we decided to go with const fn for consistency. I'd prefer to keep it as a const fn unless there's a specific advantage to making it a const that I'm not aware of.

[tbodt]

Now that Error::backtrace has been removed, this is effectively a full stabilization!

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[loyd]

Thanks to the error handling team and, especially, to @yaahc for not stabilizing Error::backtrace and the decision to use the provide API. My case is a special backtrace implementation with caching by (ip,sp) to avoid latencies at high quantiles in low-latency environments. Stabilization of Error::backtrace means crates will be focused on the default implementation instead. I believe the eyre's approach is much more flexible.

[yaahc]

@bors r+

[bors]

:pushpin: Commit 21396828e4743cd5ff50488b98ee3fbfa150dee4 has been approved by yaahc

It is now in the queue for this repository.