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

Remove fn backtrace and replace with usages of provider API #99431

Merged
merged 1 commit into from
Aug 2, 2022

Conversation

yaahc
Copy link
Member

@yaahc yaahc commented Jul 18, 2022

This PR is part of moving error into core and has been split to facilitate review.

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jul 18, 2022
@rustbot
Copy link
Collaborator

rustbot commented Jul 18, 2022

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

@rust-highfive
Copy link
Collaborator

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 18, 2022
@joshtriplett
Copy link
Member

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Jul 18, 2022

📌 Commit 10bb380599e28e796bfad141ea555e9ecf418543 has been approved by joshtriplett

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 18, 2022
@joshtriplett
Copy link
Member

@bors r-

@bjorn3's suggestion seems like an improvement to me. r=me with that applied (whichever of the two works), assuming it compiles.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 19, 2022
@joshtriplett
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jul 19, 2022

📌 Commit 10bb380599e28e796bfad141ea555e9ecf418543 has been approved by joshtriplett

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 19, 2022
@bors
Copy link
Contributor

bors commented Jul 19, 2022

⌛ Testing commit 10bb380599e28e796bfad141ea555e9ecf418543 with merge b7dcd855e7a5f67c9b032f0688300e3227bb61f4...

@bors
Copy link
Contributor

bors commented Jul 20, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 20, 2022
@rust-log-analyzer

This comment has been minimized.

@bjorn3
Copy link
Member

bjorn3 commented Jul 20, 2022

error: could not compile anyhow

Probably caused by anyhow's autodetection of the backtrace feature this PR removed. The intention is that changes to the api would cause anyhow to not use backtrace, but there is a bug there I think.

https://github.com/dtolnay/anyhow/blob/master/build.rs

@yaahc
Copy link
Member Author

yaahc commented Jul 21, 2022

I don't understand what is going wrong here. The log output from this failure doesn't show anything other than that error: could not compile anyhow messsage and I'm not able to reproduce that when compiling anyhow directly with my custom rustc toolchain. I double checked against master and against the 1.0.55 tag which matches the version that failed in the log output. If I run tests I am able to get the test_backtrace.rs test to fail because it doesn't use the probe, and instead only checks that the current compiler version is a nightly compiler, but that shouldn't be involved here, right? The probe itself looks fine and seems to work fine, what the hell!? 😭

cc @dtolnay

I guess I'm gonna try to figure out exactly what step the CI failed on to see if that helps me with getting a repro...

edit: it looks like it only failed on windows, I'm going to assume that is relevant.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (ebe571cf3c33a5e18ad1a047c97f24c41f51bea8): comparison url.

Instruction count

  • Primary benchmarks: 🎉 relevant improvement found
  • Secondary benchmarks: 😿 relevant regressions found
mean1 max count2
Regressions 😿
(primary)
N/A N/A 0
Regressions 😿
(secondary)
1.1% 1.5% 3
Improvements 🎉
(primary)
-1.3% -1.3% 1
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) -1.3% -1.3% 1

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

This benchmark run did not return any relevant results for this metric.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

Footnotes

  1. the arithmetic mean of the percent change

  2. number of relevant changes

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 26, 2022
@yaahc
Copy link
Member Author

yaahc commented Jul 26, 2022

we likely want to split it into a separate PR if there's any impact.

@Mark-Simulacrum how do we feel about these results? Results are mixed and only seem to impact incremental builds. Do you think this should be split further or can we go ahead and merge the bump together with this PR?

@Mark-Simulacrum Mark-Simulacrum self-assigned this Jul 26, 2022
@Mark-Simulacrum
Copy link
Member

I think I would recommend splitting to a separate PR. The results are pretty likely to be noise, but a separate PR will also isolate from any unexpected problems (e.g., from non-Linux platforms) and doesn't seem to hard. I'm happy to r+ that PR quickly :)

If the results were completely neutral then we could go ahead here, but as-is there doesn't seem to be much motivation to bundle things.

@Mark-Simulacrum Mark-Simulacrum removed their assignment Jul 26, 2022
@yaahc yaahc mentioned this pull request Jul 28, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 30, 2022
@yaahc
Copy link
Member Author

yaahc commented Aug 1, 2022

re-approving now that the rustc-perf bug has been fixed and landed separately.

@bors r=joshtriplett

@bors
Copy link
Contributor

bors commented Aug 1, 2022

📌 Commit b2bbca3 has been approved by joshtriplett

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 1, 2022
@bors
Copy link
Contributor

bors commented Aug 2, 2022

⌛ Testing commit b2bbca3 with merge 9538d2d...

@bors
Copy link
Contributor

bors commented Aug 2, 2022

☀️ Test successful - checks-actions
Approved by: joshtriplett
Pushing 9538d2d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 2, 2022
@bors bors merged commit 9538d2d into rust-lang:master Aug 2, 2022
@rustbot rustbot added this to the 1.64.0 milestone Aug 2, 2022
@bors bors mentioned this pull request Aug 2, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9538d2d): comparison url.

Instruction count

  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: 😿 relevant regressions found
mean1 max count2
Regressions 😿
(primary)
N/A N/A 0
Regressions 😿
(secondary)
1.5% 1.5% 2
Improvements 🎉
(primary)
-0.9% -1.4% 2
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) -0.9% -1.4% 2

Max RSS (memory usage)

Results
  • Primary benchmarks: 😿 relevant regression found
  • Secondary benchmarks: no relevant changes found
mean1 max count2
Regressions 😿
(primary)
4.2% 4.2% 1
Regressions 😿
(secondary)
N/A N/A 0
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) 4.2% 4.2% 1

Cycles

Results
  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: no relevant changes found
mean1 max count2
Regressions 😿
(primary)
N/A N/A 0
Regressions 😿
(secondary)
N/A N/A 0
Improvements 🎉
(primary)
-4.3% -6.0% 11
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) -4.3% -6.0% 11

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change 2 3

  2. number of relevant changes 2 3

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 10, 2022
Stabilize backtrace

This PR stabilizes the std::backtrace module. As of rust-lang#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 rust-lang#72981, rust-lang#3156.

Stabilized API summary:
```rust
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`
facebook-github-bot pushed a commit to facebook/buck2 that referenced this pull request Sep 8, 2022
Summary:
It includes this fix which was backported to 1.62.1.
rust-lang/rust#98950

`2022-08-06` is what will become 1.64.0.

Changes are caused by:
- Split TypeVisitable from TypeFoldable: rust-lang/rust#98206
- Remove `fn backtrace` and replace with usages of provider API: rust-lang/rust#99431

Reviewed By: stepancheg

Differential Revision: D39277984

fbshipit-source-id: 5afc6c2b9e5ee3074ea0ede995868aef12cebf14
facebook-github-bot pushed a commit to facebookincubator/gazebo_lint that referenced this pull request Sep 8, 2022
Summary:
It includes this fix which was backported to 1.62.1.
rust-lang/rust#98950

`2022-08-06` is what will become 1.64.0.

Changes are caused by:
- Split TypeVisitable from TypeFoldable: rust-lang/rust#98206
- Remove `fn backtrace` and replace with usages of provider API: rust-lang/rust#99431

Reviewed By: stepancheg

Differential Revision: D39277984

fbshipit-source-id: 5afc6c2b9e5ee3074ea0ede995868aef12cebf14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants