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

Rollup of 4 pull requests #95448

Merged
merged 13 commits into from
Mar 29, 2022
Merged

Rollup of 4 pull requests #95448

merged 13 commits into from
Mar 29, 2022

Conversation

Dylan-DPC
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

yaahc and others added 13 commits February 22, 2022 12:40
Co-authored-by: Mara Bos <m-ou.se@m-ou.se>
…ebration-station, r=joshtriplett

Stabilize Termination and ExitCode

From rust-lang#43301

This PR stabilizes the Termination trait and associated ExitCode type. It also adjusts the ExitCode feature flag to replace the placeholder flag with a more permanent name, as well as splitting off the `to_i32` method behind its own permanently unstable feature flag.

This PR stabilizes the termination trait with the following signature:

```rust
pub trait Termination {
    fn report(self) -> ExitCode;
}
```

The existing impls of `Termination` are effectively already stable due to the prior stabilization of `?` in main.

This PR also stabilizes the following APIs on exit code

```rust
#[derive(Clone, Copy, Debug)]
pub struct ExitCode(_);

impl ExitCode {
    pub const SUCCESS: ExitCode;
    pub const FAILURE: ExitCode;
}

impl From<u8> for ExitCode { /* ... */ }
```

---

All of the previous blockers have been resolved. The main ones that were resolved recently are:

* The trait's name: We decided against changing this since none of the alternatives seemed particularly compelling. Instead we decided to end the bikeshedding and stick with the current name. ([link to the discussion](https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Termination.2FExit.20Status.20Stabilization/near/269793887))
* Issues around platform specific representations: We resolved this issue by changing the return type of `report` from `i32` to the opaque type `ExitCode`. That way we can change the underlying representation without affecting the API, letting us offer full support for platform specific exit code APIs in the future.
* Custom exit codes: We resolved this by adding `From<u8> for ExitCode`. We choose to only support u8 initially because it is the least common denominator between the sets of exit codes supported by our current platforms. In the future we anticipate adding platform specific extension traits to ExitCode for constructors from larger or negative numbers, as needed.
Ensure io::Error's bitpacked repr doesn't accidentally impl UnwindSafe

Sadly, I'm not sure how to easily test that we don't impl a trait, though (or can libstd use `where io::Error: !UnwindSafe` or something).

Fixes rust-lang#95203
…-obk

Suggest wrapping patterns in enum variants

Structured suggestion to wrap a pattern in a single-field enum or struct:

```diff
 struct A;

 enum B {
   A(A),
 }

 fn main(b: B) {
   match b {
-    A => {}
+    B::A(A) => {}
   }
 }
```

Half of rust-lang#94942, the other half I'm not exactly sure how to fix.

Also includes two drive-by changes (that I am open to splitting out into another PR, but thought they could be rolled up into this one):
- 07776c1: Makes sure not to suggest wrapping if it doesn't have tuple field constructor (i.e. has named fields)
- 8f2bbb18fd53e5008bb488302dbd354577698ede: Also suggest wrapping expressions in a tuple struct (not just enum variants)
…compiler-errors

diagnostics: regression test for derive bounds

Closes rust-lang#79076
@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Mar 29, 2022
@Dylan-DPC
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Mar 29, 2022

📌 Commit 2471502 has been approved by Dylan-DPC

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 29, 2022
@bors
Copy link
Contributor

bors commented Mar 29, 2022

⌛ Testing commit 2471502 with merge 9c06e1b...

@bors
Copy link
Contributor

bors commented Mar 29, 2022

☀️ Test successful - checks-actions
Approved by: Dylan-DPC
Pushing 9c06e1b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 29, 2022
@bors bors merged commit 9c06e1b into rust-lang:master Mar 29, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 29, 2022
@bors bors mentioned this pull request Mar 29, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9c06e1b): comparison url.

Summary: This benchmark run shows 5 relevant regressions 😿 to instruction counts.

  • Arithmetic mean of relevant regressions: 1.5%
  • Largest regression in instruction counts: 2.5% on incr-patched: io error 6144 builds of issue-46449 debug

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

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression

@rustbot rustbot added the perf-regression Performance regression. label Mar 30, 2022
@pnkfelix
Copy link
Member

Visiting for weekly performance triage.

The only primary benchmark impacted by this was unicode-normalization-0.1.19, and that only saw a 0.50% regression on incr-patched: println and a 0.36% regression on full build. I think we can ignore that.

The only secondary benchmark that was impacted was issue-46449, which saw a 2% to 2.5% regression on three scenarios.

Looking over the four PR's in this rollup, I do not think its worth trying to tease out the cause of the regression to that secondary benchmark. Specifically, issue-46449 is tied to #46449 ; the problem it is trying to catch was tied to quadratic slowdown, not a minor change of a few percentage points.

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Mar 30, 2022
@Dylan-DPC Dylan-DPC deleted the rollup-wpj5yto branch March 30, 2022 23:56
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. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants