Skip to content

Conversation

@jhpratt
Copy link
Member

@jhpratt jhpratt commented Jan 31, 2025

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

ZuseZ4 and others added 30 commits January 29, 2025 21:31
It's either "profile-guided" or "profiled-guided" and I think it'sw the former. :)
this allows compare-mode to share the same subdirectory and removes
differences due to that
…e/expression

```
error[E0271]: expected `{[email protected]:18:40}` to be a closure that returns `()`, but it returns `!`
  --> $DIR/fallback-closure-wrap.rs:19:9
   |
LL |     let error = Closure::wrap(Box::new(move || {
   |                                        -------
LL |         panic!("Can't connect to server.");
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found `!`
   |
   = note: expected unit type `()`
                   found type `!`
   = note: required for the cast from `Box<{closure@$DIR/fallback-closure-wrap.rs:18:40: 18:47}>` to `Box<dyn FnMut()>`
```

```
error[E0271]: expected `{[email protected]:6:10}` to be a closure that returns `bool`, but it returns `Option<()>`
  --> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:6:16
   |
LL |     call(|| -> Option<()> {
   |     ---- ------^^^^^^^^^^
   |     |          |
   |     |          expected `bool`, found `Option<()>`
   |     required by a bound introduced by this call
   |
   = note: expected type `bool`
              found enum `Option<()>`
note: required by a bound in `call`
  --> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:3:25
   |
LL | fn call(_: impl Fn() -> bool) {}
   |                         ^^^^ required by this bound in `call`
```

```
error[E0271]: expected `{[email protected]:28:13}` to be a closure that returns `Result<(), _>`, but it returns `!`
    --> f670.rs:28:20
     |
28   |     let c = |e| -> ! {
     |             -------^
     |                    |
     |                    expected `Result<(), _>`, found `!`
...
32   |     f().or_else(c);
     |         ------- required by a bound introduced by this call
-Ztrack-diagnostics: created at compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs:1433:28
     |
     = note: expected enum `Result<(), _>`
                found type `!`
note: required by a bound in `Result::<T, E>::or_else`
    --> /home/gh-estebank/rust/library/core/src/result.rs:1406:39
     |
1406 |     pub fn or_else<F, O: FnOnce(E) -> Result<T, F>>(self, op: O) -> Result<T, F> {
     |                                       ^^^^^^^^^^^^ required by this bound in `Result::<T, E>::or_else`
```
```
error[E0271]: expected `{[email protected]:18:13}` to be a closure that returns `Result<(), _>`, but it returns `!`
    --> tests/ui/closures/return-type-doesnt-match-bound.rs:18:20
     |
18   |     let c = |e| -> ! { //~ ERROR to be a closure that returns
     |             -------^
     |                    |
     |                    expected `Result<(), _>`, found `!`
...
22   |     f().or_else(c);
     |         ------- -
     |         |
     |         required by a bound introduced by this call
     |
     = note: expected enum `Result<(), _>`
                found type `!`
note: required by a bound in `Result::<T, E>::or_else`
    --> /home/gh-estebank/rust/library/core/src/result.rs:1406:39
     |
1406 |     pub fn or_else<F, O: FnOnce(E) -> Result<T, F>>(self, op: O) -> Result<T, F> {
     |                                       ^^^^^^^^^^^^ required by this bound in `Result::<T, E>::or_else`
```
I was confused here for a bit.
Also rewrite the merged arm slightly to more closely match the arm above
it.
This comment made sense when this crate was called `rustc_typeck`, but
makes less sense now that it's called `rustc_hir_analysis`. Especially
given that `check_drop_impl` is only called within the crate.
Note: `inherit_predicates_for_delegation_item` already has these cases
merged.
There is a comment `Delegation to inherent methods is not yet
supported.` that appears three times mid-pattern and somehow inhibits
rustfmt from formatting the enclosing `match` statement. This commit
moves them to the top of the pattern, which enables more formatting.
`delegation.rs` has three builders: `GenericsBuilder`,
`PredicatesBuilder`, and `GenericArgsBuilder`. The first two builders
have just two optional parameters, and the third one has zero. Each
builder is used within a single function. The code is over-engineered.

This commit removes the builders, replacing each with with a single
`build_*` function. This makes the code shorter and simpler.
It used to be bigger, with an `Xform` trait, but now it has just a
single function.
Instead re-export `rustc_hir_analysis::collect::suggest_impl_trait`,
which is the only thing from the module used in another crate. This
fixes a `FIXME` comment. Also adjust some visibilities to satisfy the
`unreachable_pub` lint.

This changes requires downgrading a link in a comment on `FnCtxt`
because `collect` is no longer public and rustdoc complains otherwise.
This is annoying but I can't see how to avoid it.
…il,compiler-errors

When encountering unexpected closure return type, point at return type/expression

```
error[E0271]: expected `{[email protected]:18:40}` to be a closure that returns `()`, but it returns `!`
  --> $DIR/fallback-closure-wrap.rs:19:9
   |
LL |     let error = Closure::wrap(Box::new(move || {
   |                                        -------
LL |         panic!("Can't connect to server.");
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found `!`
   |
   = note: expected unit type `()`
                   found type `!`
   = note: required for the cast from `Box<{closure@$DIR/fallback-closure-wrap.rs:18:40: 18:47}>` to `Box<dyn FnMut()>`
```

```
error[E0271]: expected `{[email protected]:6:10}` to be a closure that returns `bool`, but it returns `Option<()>`
  --> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:6:16
   |
LL |     call(|| -> Option<()> {
   |     ---- ------^^^^^^^^^^
   |     |          |
   |     |          expected `bool`, found `Option<()>`
   |     required by a bound introduced by this call
   |
   = note: expected type `bool`
              found enum `Option<()>`
note: required by a bound in `call`
  --> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:3:25
   |
LL | fn call(_: impl Fn() -> bool) {}
   |                         ^^^^ required by this bound in `call`
```

```
error[E0271]: expected `{[email protected]:28:13}` to be a closure that returns `Result<(), _>`, but it returns `!`
    --> f670.rs:28:20
     |
28   |     let c = |e| -> ! {
     |             -------^
     |                    |
     |                    expected `Result<(), _>`, found `!`
...
32   |     f().or_else(c);
     |         ------- required by a bound introduced by this call
-Ztrack-diagnostics: created at compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs:1433:28
     |
     = note: expected enum `Result<(), _>`
                found type `!`
note: required by a bound in `Result::<T, E>::or_else`
    --> /home/gh-estebank/rust/library/core/src/result.rs:1406:39
     |
1406 |     pub fn or_else<F, O: FnOnce(E) -> Result<T, F>>(self, op: O) -> Result<T, F> {
     |                                       ^^^^^^^^^^^^ required by this bound in `Result::<T, E>::or_else`
```

CC rust-lang#111539.
Autodiff Upstreaming - rustc_codegen_ssa, rustc_middle

This PR should not be merged until the rustc_codegen_llvm part is merged.
I will also alter it a little based on what get's shaved off from the cg_llvm PR,
and address some of the feedback I received in the other PR (including cleanups).

I am putting it already up to
1) Discuss with `@jieyouxu` if there is more work needed to add tests to this and
2) Pray that there is someone reviewing who can tell me why some of my autodiff invocations get lost.

Re 1: My test require fat-lto. I also modify the compilation pipeline. So if there are any other llvm-ir tests in the same compilation unit then I will likely break them. Luckily there are two groups who currently have the same fat-lto requirement for their GPU code which I have for my autodiff code and both groups have some plans to enable support for thin-lto. Once either that work pans out, I'll copy it over for this feature. I will also work on not changing the optimization pipeline for functions not differentiated, but that will require some thoughts and engineering, so I think it would be good to be able to run the autodiff tests isolated from the rest for now. Can you guide me here please?
For context, here are some of my tests in the samples folder: https://github.com/EnzymeAD/rustbook

Re 2: This is a pretty serious issue, since it effectively prevents publishing libraries making use of autodiff: EnzymeAD#173. For some reason my dummy code persists till the end, so the code which calls autodiff, deletes the dummy, and inserts the code to compute the derivative never gets executed. To me it looks like the rustc_autodiff attribute just get's dropped, but I don't know WHY? Any help would be super appreciated, as rustc queries look a bit voodoo to me.

Tracking:

- rust-lang#124509

r? `@jieyouxu`
…lcnr

`rustc_hir_analysis` cleanups

Just some improvements I found while looking through this code.

r? `@lcnr`
@rustbot rustbot added the rollup A PR which is a rollup label Jan 31, 2025
@jhpratt
Copy link
Member Author

jhpratt commented Jan 31, 2025

@bors r+ rollup=never p=5

@bors
Copy link
Collaborator

bors commented Jan 31, 2025

📌 Commit e429ada has been approved by jhpratt

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 Jan 31, 2025
@bors
Copy link
Collaborator

bors commented Jan 31, 2025

⌛ Testing commit e429ada with merge 7f36543...

@bors
Copy link
Collaborator

bors commented Jan 31, 2025

☀️ Test successful - checks-actions
Approved by: jhpratt
Pushing 7f36543 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 31, 2025
@bors bors merged commit 7f36543 into rust-lang:master Jan 31, 2025
7 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Jan 31, 2025
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#132156 When encountering unexpected closure return type, point at … 60624dbc6670dd87771b51c91d9713758f14964b (link)
#133429 Autodiff Upstreaming - rustc_codegen_ssa, rustc_middle cc1a53b5443b367b1b402a9002571bbaa67474d5 (link)
#136281 rustc_hir_analysis cleanups 4be9561d87346e3a4846672afe9133e13313c535 (link)
#136297 Fix a typo in profile-guided-optimization.md ab59a318f4b40f33d7c91cee4d1e8fd537ab46d6 (link)
#136300 atomic: extend compare_and_swap migration docs a118ccfc18c9ca3e2921fc8bda94c1ccb5e863e6 (link)
#136310 normalize *.long-type.txt paths for compare-mode tests ab66f6b481dfa87bbf14161284c304ecc67d2574 (link)
#136312 Disable overflow_delimited_expr in edition 2024 c6f1d3d238395540ba49fa35cb57d8da92a8d1cc (link)
#136313 Filter out RPITITs when suggesting unconstrained assoc type… 2df0836be214ee7f7b9b6ff9eecdb44744b41326 (link)
#136323 Fix a typo in conventions.md 5af8b1a053372703c8c884111d695c605bce30f7 (link)

previous master: 25a16572a3

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

Overall result: ❌✅ regressions and improvements - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
43.3% [42.1%, 44.0%] 4
Regressions ❌
(secondary)
42.8% [39.0%, 46.6%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.3% [-0.3%, -0.2%] 4
All ❌✅ (primary) 43.3% [42.1%, 44.0%] 4

Max RSS (memory usage)

Results (secondary -0.1%)

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

Cycles

Results (primary 21.3%, secondary 8.7%)

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)
44.8% [43.2%, 45.7%] 4
Regressions ❌
(secondary)
45.4% [41.9%, 48.9%] 2
Improvements ✅
(primary)
-2.1% [-2.3%, -2.0%] 4
Improvements ✅
(secondary)
-3.5% [-4.5%, -2.5%] 6
All ❌✅ (primary) 21.3% [-2.3%, 45.7%] 8

Binary size

Results (primary 0.0%, secondary 0.1%)

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.0% [0.0%, 0.4%] 51
Regressions ❌
(secondary)
0.2% [0.0%, 0.4%] 21
Improvements ✅
(primary)
-0.0% [-0.0%, -0.0%] 3
Improvements ✅
(secondary)
-0.6% [-0.7%, -0.5%] 2
All ❌✅ (primary) 0.0% [-0.0%, 0.4%] 54

Bootstrap: 777.178s -> 777.68s (0.06%)
Artifact size: 328.82 MiB -> 328.84 MiB (0.01%)

@rustbot rustbot added the perf-regression Performance regression. label Feb 1, 2025
@lqd
Copy link
Member

lqd commented Feb 1, 2025

@rust-timer build cc1a53b

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (cc1a53b): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
43.3% [42.1%, 44.0%] 4
Regressions ❌
(secondary)
42.7% [39.0%, 46.4%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.3% [-0.3%, -0.2%] 4
All ❌✅ (primary) 43.3% [42.1%, 44.0%] 4

Max RSS (memory usage)

Results (primary 0.9%, secondary -0.1%)

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.6% [2.6%, 2.6%] 1
Regressions ❌
(secondary)
2.4% [1.8%, 3.0%] 2
Improvements ✅
(primary)
-0.9% [-0.9%, -0.9%] 1
Improvements ✅
(secondary)
-1.8% [-2.4%, -1.3%] 3
All ❌✅ (primary) 0.9% [-0.9%, 2.6%] 2

Cycles

Results (primary 21.2%, secondary 8.6%)

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)
45.0% [43.4%, 45.7%] 4
Regressions ❌
(secondary)
45.5% [42.1%, 48.9%] 2
Improvements ✅
(primary)
-2.6% [-2.7%, -2.4%] 4
Improvements ✅
(secondary)
-3.7% [-5.0%, -2.8%] 6
All ❌✅ (primary) 21.2% [-2.7%, 45.7%] 8

Binary size

Results (primary 0.0%, secondary 0.1%)

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.0% [0.0%, 0.4%] 53
Regressions ❌
(secondary)
0.2% [0.0%, 0.4%] 21
Improvements ✅
(primary)
-0.0% [-0.0%, -0.0%] 3
Improvements ✅
(secondary)
-0.6% [-0.7%, -0.5%] 2
All ❌✅ (primary) 0.0% [-0.0%, 0.4%] 56

Bootstrap: 777.178s -> 779.235s (0.26%)
Artifact size: 328.82 MiB -> 328.82 MiB (-0.00%)

@ZuseZ4
Copy link
Member

ZuseZ4 commented Feb 2, 2025

I'll take the liberty of declaring the perf regression as triaged, fixed with #136413

@oli-obk oli-obk added the perf-regression-triaged The performance regression has been triaged. label Feb 2, 2025
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this pull request Mar 11, 2025