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 9 pull requests #129936

Merged
merged 24 commits into from
Sep 5, 2024
Merged

Rollup of 9 pull requests #129936

merged 24 commits into from
Sep 5, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

veera-sivarajan and others added 24 commits July 13, 2024 11:49
Inspired by discussion on
rust-lang#129486 this is intended to at
least document the current state of the world in a more public location
than throughout a series of issues.
The actual implementation remains in `rustc_mir_dataflow`, but this
commit moves the `MirPass` impl to `rustc_mir_transform` and changes it
to a `MirLint` (fixing a `FIXME` comment).

(I originally tried moving the full implementation from
`rustc_mir_dataflow` but I had some trait problems with `HasMoveData`
and `RustcPeekAt` and `MaybeLiveLocals`. This commit was much smaller
and simpler, but still will allow some follow-up cleanups.)
Because that's now the only crate that uses it.

Moving stuff out of `rustc_middle` is always welcome.

I chose to use `impl crate::MirPass`/`impl crate::MirLint` (with
explicit `crate::`) everywhere because that's the only mention of
`MirPass`/`MirLint` used in all of these files. (Prior to this change,
`MirPass` was mostly imported via `use rustc_middle::mir::*` items.)
They're now all just used within this crate.
Forgot this during the release process
…stebank

Suggest `impl Trait` for References to Bare Trait in Function Header

Fixes rust-lang#125139

This PR suggests `impl Trait` when `&Trait` is found as a function parameter type or return type. This makes use of existing diagnostics by adding `peel_refs()` when checking for type equality.

Additionaly, it makes a few other improvements:
1. Checks if functions inside impl blocks have bare trait in their headers.
2. Introduces a trait `NextLifetimeParamName` similar to the existing `NextTypeParamName` for suggesting a lifetime name. Also, abstracts out the common logic between the two trait impls.

### Related Issues
I ran into a bunch of related diagnostic issues but couldn't fix them within the scope of this PR. So, I have created the following issues:
1. [Misleading Suggestion when Returning a Reference to a Bare Trait from a Function](rust-lang#127689)
2. [Verbose Error When a Function Takes a Bare Trait as Parameter](rust-lang#127690)
3. [Incorrect Suggestion when Returning a Bare Trait from a Function](rust-lang#127691)

r​? ```@estebank``` since you implemented  rust-lang#119148
…bank

Don't Suggest Labeling `const` and `unsafe` Blocks

Fixes rust-lang#128604

Previously, both anonymous constant blocks (E.g. The labeled block
inside `['_'; 'block: { break 'block 1 + 2; }]`) and inline const
blocks (E.g. `const { ... }`) were considered to be the same
kind of blocks. This caused the compiler to incorrectly suggest
labeling both the blocks when only anonymous constant blocks can be
labeled.

This PR adds an other enum variant to `Context` so that both the
blocks can be handled appropriately.

Also, adds some doc comments and removes unnecessary `&mut` in a
couple of places.
…, r=compiler-errors

Non-exhaustive structs may be empty

This is a follow-up to a discrepancy noticed in rust-lang#122792: today, the following struct is considered inhabited (non-empty) outside its defining crate:
```rust
#[non_exhaustive]
pub struct UninhabitedStruct {
    pub never: !,
    // other fields
}
```

`#[non_exhaustive]` on a struct should mean that adding fields to it isn't a breaking change. There is no way that adding fields to this struct could make it non-empty since the `never` field must stay and is inconstructible. I suspect this was implemented this way due to confusion with `#[non_exhaustive]` enums, which indeed should be considered non-empty outside their defining crate.

I propose that we consider such a struct uninhabited (empty), just like it would be without the `#[non_exhaustive]` annotation.

Code that doesn't pass today and will pass after this:
```rust
// In a different crate
fn empty_match_on_empty_struct<T>(x: UninhabitedStruct) -> T {
    match x {}
}
```

This is not a breaking change.

r? ``@compiler-errors``
…-on-wasm32-u-u, r=workingjubilee

Document the broken C ABI of `wasm32-unknown-unknown`

Inspired by discussion on
rust-lang#129486 this is intended to at least document the current state of the world in a more public location than throughout a series of issues.
…esleywiser

update comment regarding TargetOptions.features

The claim that `-Ctarget-features` cannot disable these features set in the target spec is definitely wrong -- I tried it for `x86_64-pc-windows-gnu`, which enables SSE3 that way. Building with `-Ctarget-feature=-sse3` works fine, and `cfg!(target_feature = "sse3")` is `false` in that build.

There are also some indications that these are actually intended to be overwritten:

https://github.com/rust-lang/rust/blob/3b14526cead4105f82c398d8d4c7954efa3bab6b/compiler/rustc_target/src/spec/targets/i686_unknown_uefi.rs#L22-L23

https://github.com/rust-lang/rust/blob/84ac80f1921afc243d71fd0caaa4f2838c294102/compiler/rustc_target/src/spec/targets/x86_64h_apple_darwin.rs#L18-L23

So... let's update the comment to match reality, I guess?

The claim that they overwrite `-Ctarget-cpu` is based on
- for `native`, the comment in the apple target spec quoted above
- for other CPU strings, the assumption that `LLVMRustCreateTargetMachine` will apply these features after doing whatever the base CPU model does. I am not sure how to check that, I hope some LLVM backend people can chime in. :)
do not attempt to prove unknowable goals

In case a goal is unknowable, we previously still checked all other possible ways to prove this goal, even though its final result is already guaranteed to be ambiguous. By ignoring all other candidates in that case we can avoid a lot of unnecessary work, fixing the performance regression in typenum found in rust-lang#121848.

This is already the behavior in the old solver. This could in theory cause future-compatability issues as considering fewer goals unknowable may end up causing performance regressions/hangs. I am quite confident that this will not be an issue.

r? ``@compiler-errors``
…Pass, r=cjgillot

Move `SanityCheck` and `MirPass`

They are currently in `rustc_middle`. This PR moves them to `rustc_mir_transform`, which makes more sense.

r? ``@cjgillot``
…tic, r=compiler-errors

rustc_driver_impl: remove some old dead logic

This got added in rust-lang@5013952, before `cfg(target_feature)` was stable. It should not be needed any more ever since `cfg(target_feature)` is stable.
…roalbini

include 1.80.1 release notes on master

Forgot this during the release process.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-release Relevant to the release subteam, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) rollup A PR which is a rollup labels Sep 3, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=9

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-msvc-ext failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
[RUSTC-TIMING] miri test:false 4.609
error: failed to remove file `C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage1-tools\x86_64-pc-windows-msvc\release\miri.exe`

Caused by:
  Access is denied. (os error 5)
Command has failed. Rerun with -v to see more details.
  local time: Wed, Sep  4, 2024 11:13:18 AM
  network time: Wed, 04 Sep 2024 11:13:18 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@bors
Copy link
Contributor

bors commented Sep 4, 2024

💔 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 Sep 4, 2024
@lqd
Copy link
Member

lqd commented Sep 4, 2024

@bors retry #127883

@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 Sep 4, 2024
@bors
Copy link
Contributor

bors commented Sep 4, 2024

⌛ Testing commit 2e00511 with merge e158abf...

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 4, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#127692 (Suggest `impl Trait` for References to Bare Trait in Function Header)
 - rust-lang#128701 (Don't Suggest Labeling `const` and `unsafe` Blocks )
 - rust-lang#128934 (Non-exhaustive structs may be empty)
 - rust-lang#129630 (Document the broken C ABI of `wasm32-unknown-unknown`)
 - rust-lang#129863 (update comment regarding TargetOptions.features)
 - rust-lang#129896 (do not attempt to prove unknowable goals)
 - rust-lang#129926 (Move `SanityCheck` and `MirPass`)
 - rust-lang#129928 (rustc_driver_impl: remove some old dead logic)
 - rust-lang#129930 (include 1.80.1 release notes on master)

r? `@ghost`
`@rustbot` modify labels: rollup
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-msvc-ext failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
[RUSTC-TIMING] miri test:false 4.572
error: failed to remove file `C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage1-tools\x86_64-pc-windows-msvc\release\miri.exe`

Caused by:
  Access is denied. (os error 5)
Command has failed. Rerun with -v to see more details.
  local time: Wed, Sep  4, 2024  1:55:51 PM
  network time: Wed, 04 Sep 2024 13:55:51 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@bors
Copy link
Contributor

bors commented Sep 4, 2024

💔 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 Sep 4, 2024
@lqd
Copy link
Member

lqd commented Sep 4, 2024

@bors retry #127883

@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 Sep 4, 2024
@tgross35
Copy link
Contributor

tgross35 commented Sep 4, 2024

6 in a row 🙃

@lqd
Copy link
Member

lqd commented Sep 4, 2024

how high can we go? what's the best streak so far? 😆

@tgross35
Copy link
Contributor

tgross35 commented Sep 4, 2024

I thought I was lucky when I got three in a row last week, Matthias blew my record out of the water here 😆

@bors
Copy link
Contributor

bors commented Sep 4, 2024

⌛ Testing commit 2e00511 with merge 009e738...

@tgross35
Copy link
Contributor

tgross35 commented Sep 4, 2024

image

@bors
Copy link
Contributor

bors commented Sep 5, 2024

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 009e738 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 5, 2024
@bors bors merged commit 009e738 into rust-lang:master Sep 5, 2024
7 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Sep 5, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#127692 Suggest impl Trait for References to Bare Trait in Functi… 325d32d7683784f91024ff0a5913a3d81d7a04d6 (link)
#128701 Don't Suggest Labeling const and unsafe Blocks ad75ab001d50bb7a3070b03f918dc39fac736bef (link)
#128934 Non-exhaustive structs may be empty 2e684c754636a1a0904d3c80c4bacc38dd875d38 (link)
#129630 Document the broken C ABI of wasm32-unknown-unknown 710c8ade512fa50c928e1195ca98de55749a22ff (link)
#129863 update comment regarding TargetOptions.features c28fa0bb8057320b32bed36874cc8f98e75e6ce4 (link)
#129896 do not attempt to prove unknowable goals 7543ae467a791d13dfef55d81a3562c10233d4fd (link)
#129926 Move SanityCheck and MirPass c1f52f268f1e37b086809149cd3f5fd78b1969d9 (link)
#129928 rustc_driver_impl: remove some old dead logic 6b31e4dbd77e3b60da1314e7c3369867e32b967a (link)
#129930 include 1.80.1 release notes on master d987f67e9601c265c82cc40af11fc60dc3f7a03b (link)

previous master: 4ac7bcbaad

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (009e738): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (primary -1.7%, secondary 0.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.6% [2.6%, 2.6%] 2
Improvements ✅
(primary)
-1.7% [-1.7%, -1.7%] 1
Improvements ✅
(secondary)
-1.7% [-2.1%, -1.2%] 2
All ❌✅ (primary) -1.7% [-1.7%, -1.7%] 1

Cycles

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

Binary size

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

Bootstrap: 751.188s -> 752.275s (0.14%)
Artifact size: 340.71 MiB -> 340.60 MiB (-0.03%)

@jieyouxu jieyouxu added the CI-spurious-fail-msvc CI spurious failure: target env msvc label Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-spurious-fail-msvc CI spurious failure: target env msvc merged-by-bors This PR was explicitly merged by bors. 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. T-release Relevant to the release subteam, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.