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 10 pull requests #121327

Merged
merged 23 commits into from
Feb 20, 2024
Merged

Rollup of 10 pull requests #121327

merged 23 commits into from
Feb 20, 2024

Conversation

Noratrieb
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

nnethercote and others added 23 commits February 17, 2024 09:40
`CompilerError` has `CompilationFailed` and `ICE` variants, which seems
reasonable at first. But the way it identifies them is flawed:
- If compilation errors out, i.e. `RunCompiler::run` returns an `Err`,
  it uses `CompilationFailed`, which is reasonable.
- If compilation panics with `FatalError`, it catches the panic and uses
  `ICE`. This is sometimes right, because ICEs do cause `FatalError`
  panics, but sometimes wrong, because certain compiler errors also
  cause `FatalError` panics. (The compiler/rustdoc/clippy/whatever just
  catches the `FatalError` with `catch_with_exit_code` in `main`.)

In other words, certain non-ICE compilation failures get miscategorized
as ICEs. It's not possible to reliably distinguish the two cases, so
this commit merges them. It also renames the combined variant as just
`Failed`, to better match the existing `Interrupted` and `Skipped`
variants.

Here is an example of a non-ICE failure that causes a `FatalError`
panic, from `tests/ui/recursion_limit/issue-105700.rs`:
```
 #![recursion_limit="4"]
 #![invalid_attribute]
 #![invalid_attribute]
 #![invalid_attribute]
 #![invalid_attribute]
 #![invalid_attribute]
 //~^ERROR recursion limit reached while expanding

 fn main() {{}}
```
When these extra directives were ported over as part of rust-lang#112300, it made sense
to introduce `iter_header_extra` and pass them in as an extra argument.

But now that rust-lang#120881 has added a `mode` parameter to `iter_header` for its own
purposes, it's slightly simpler to move the coverage special-case code directly
into `iter_header` as well. This lets us get rid of `iter_header_extra`.
Noticed these while doing something else. There's no practical change, but it's preferable to use `DUMMY_SP` as little as possible, particularly when we have perfectlly useful `Span`s available.
It showed up with 3% execution time in a compiler profile.
This helps iron out a difference between Sub and Equate
…r=lcnr

Change leak check and suspicious auto trait lint warning messages

The leak check lint message "this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!" is misleading as some cases may not be phased out and could end being accepted. This is under discussion still.

The suspicious auto trait lint the change in behavior already happened, so the new message is probably more accurate.

r? `@lcnr`

Closes rust-lang#93367
unstable-book: Separate testing and production sanitizers

This is a redo of [this PR](rust-lang#108942). Left the commit as before (except for reflowing to 80-width), since it already got approved.
…rError, r=oli-obk

Merge `CompilerError::CompilationFailed` and `CompilerError::ICE`.

`CompilerError` has `CompilationFailed` and `ICE` variants, which seems reasonable at first. But the way it identifies them is flawed:
- If compilation errors out, i.e. `RunCompiler::run` returns an `Err`, it uses `CompilationFailed`, which is reasonable.
- If compilation panics with `FatalError`, it catches the panic and uses `ICE`. This is sometimes right, because ICEs do cause `FatalError` panics, but sometimes wrong, because certain compiler errors also cause `FatalError` panics. (The compiler/rustdoc/clippy/whatever just catches the `FatalError` with `catch_with_exit_code` in `main`.)

In other words, certain non-ICE compilation failures get miscategorized as ICEs. It's not possible to reliably distinguish the two cases, so this commit merges them. It also renames the combined variant as just `Failed`, to better match the existing `Interrupted` and `Skipped` variants.

Here is an example of a non-ICE failure that causes a `FatalError` panic, from `tests/ui/recursion_limit/issue-105700.rs`:
```
 #![recursion_limit="4"]
 #![invalid_attribute]
 #![invalid_attribute]
 #![invalid_attribute]
 #![invalid_attribute]
 #![invalid_attribute]
 //~^ERROR recursion limit reached while expanding

 fn main() {{}}
```

r? ``@spastorino``
Move the extra directives for `Mode::CoverageRun` into `iter_header`

When these extra directives were ported over as part of rust-lang#112300, it made sense to introduce `iter_header_extra` and pass them in as an extra argument.

But now that rust-lang#120881 has added a `mode` parameter to `iter_header` for its own purposes, it's slightly simpler to move the coverage special-case code directly into `iter_header` as well. This lets us get rid of `iter_header_extra`.
Allow AST and HIR visitors to return `ControlFlow`

Alternative to rust-lang#108598.

Since rust-lang/libs-team#187 was rejected, this implements our own version of the `Try` trait (`VisitorResult`) and the `try` macro (`try_visit`). Since this change still allows visitors to return `()`, no changes have been made to the existing ones. They can be done in a separate PR.
Drive-by `DUMMY_SP` -> `Span` and fmt changes

Noticed these while doing something else. There's no practical change, but it's preferable to use `DUMMY_SP` as little as possible, particularly when we have perfectlly useful `Span`s available.
Add regression test for rust-lang#103369

The issue was fixed in 1.70.0.
Closes rust-lang#103369.
…rieb

Remove an old hack for rustdoc

Since rust-lang#78696 has been resolved
…thlin

Make `is_nonoverlapping` `#[inline]`

It showed up with 3% execution time in a compiler profile.

backlink to rust-lang#120848

r? ``@saethlin``
return `ty::Error` when equating `ty::Error`

This helps iron out a difference in diagnostics between `Sub` and `Equate` relations, which I'm currently trying to unify.

r? oli-obk
@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler 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. rollup A PR which is a rollup labels Feb 20, 2024
@Noratrieb
Copy link
Member Author

@bors r+ rollup=never p=10

@bors
Copy link
Contributor

bors commented Feb 20, 2024

📌 Commit 599768d has been approved by Nilstrieb

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 Feb 20, 2024
@bors
Copy link
Contributor

bors commented Feb 20, 2024

⌛ Testing commit 599768d with merge 5af2130...

@bors
Copy link
Contributor

bors commented Feb 20, 2024

☀️ Test successful - checks-actions
Approved by: Nilstrieb
Pushing 5af2130 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 20, 2024
@bors bors merged commit 5af2130 into rust-lang:master Feb 20, 2024
12 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 20, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#120716 Change leak check and suspicious auto trait lint warning me… cfbc193b0a1eec073f7662d4c433a0c36112e6d4 (link)
#121195 unstable-book: Separate testing and production sanitizers 0009fcf989a7700a1dc88f17a71e4531390b74dc (link)
#121205 Merge CompilerError::CompilationFailed and `CompilerError… 8341a7f60da556211c8ac6c079c882c2c48f6d7d (link)
#121233 Move the extra directives for Mode::CoverageRun into `ite… 3405cce3f2cfc3e01da2299afc4308f11f15f7dc (link)
#121256 Allow AST and HIR visitors to return ControlFlow e022ef0c7dfc9f6e72a082cbdb4c274168b7720f (link)
#121307 Drive-by DUMMY_SP -> Span and fmt changes 7fe440733a16dcf075912ca27fe583dee354198b (link)
#121308 Add regression test for #103369 6108ac3b0a280b0e024636475c9be1b84b08f5a8 (link)
#121310 Remove an old hack for rustdoc d751869150c276dd64445855a9732a785e2736fb (link)
#121311 Make is_nonoverlapping #[inline] 17c454c9519b66345d9c8cc787b2f08c695e2af8 (link)
#121319 return ty::Error when equating ty::Error 03164daaef5a68eda71aebfd3260050b44bdf6c5 (link)

previous master: bcea3cb748

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 (5af2130): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.2% [0.2%, 0.2%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results

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)
2.5% [2.5%, 2.6%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.4% [-3.4%, -3.4%] 1
All ❌✅ (primary) 2.5% [2.5%, 2.6%] 2

Cycles

Results

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)
1.8% [1.7%, 1.9%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 639.837s -> 641.541s (0.27%)
Artifact size: 308.63 MiB -> 308.58 MiB (-0.02%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc 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-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler 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.