Skip to content

Rollup of 7 pull requests #141292

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

Merged
merged 29 commits into from
May 20, 2025
Merged

Rollup of 7 pull requests #141292

merged 29 commits into from
May 20, 2025

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

fuzzypixelz and others added 29 commits May 18, 2025 15:37
Exclude issues with an associated PR from the "What should I work on" GH query
Fix typos in "Libraries and Metadata"
Better, because then one knows how to run the examples.
Remove unused references and simplify one
They are all short and have a single call site.
When checking a pattern with guards in it, `GatherLocalsVisitor` will
visit both the pattern (when type-checking the let, arm, or param
containing it) and the guard expression (when checking the guard
itself). This keeps it from visiting the guard when visiting the
pattern, since otherwise it would gather locals from the guard twice,
which would lead to a delayed bug: "evaluated expression more than
once".
…r=lcnr

Error on recursive opaque ty in HIR typeck

"Non-trivially recursive" opaques are opaques whose hidden types are inferred to be equal to something other than themselves. For example, if we have a TAIT like `type TAIT = impl Sized`, if we infer the hidden type to be `TAIT := (TAIT,)`, that would be a non-trivial recursive definition. We don't want to support opaques that are non-trivially recursive, since they will (almost!! -- see caveat below) always result in borrowck errors, and are generally a pain to deal with.

On the contrary, trivially recursive opaques may occur today because the old solver overagerly uses `replace_opaque_types_with_inference_vars`. This infer var can then later be constrained to be equal to the opaque itself. These cases will not necessarily result in borrow-checker errors, since other uses of the opaque may properly constrain the opaque. If there are no other uses we may instead fall back to `()` today.

The only weird case that we have to unfortunately deal with was discovered in rust-lang#139406:

```rust
#![allow(unconditional_recursion)]

fn what1<T>(x: T) -> impl Sized {
    what1(x)
}

fn what2<T>(x: T) -> impl Sized {
    what2(what2(x))
}

fn print_return_type<T, U>(_: impl Fn(T) -> U) {
    println!("{}", std::any::type_name::<U>())
}

fn main() {
    print_return_type(what1::<i32>); // ()
    print_return_type(what2::<i32>); // i32
}
```

> HIR typeck eagerly replaces the return type with an infer var, ending up with `RPIT<T> = RPIT<RPIT<T>>` in the storage. While we return this in the `TypeckResults`, it's never actually used anywhere.
>
> MIR building then results in the following statement
> ```rust
> let _0: impl RPIT<T> /* the return place */ = build<RPIT<T>>(_some_local);
> ```
> Unlike HIR typeck MIR typeck now directly equates `RPIT<T>` with `RPIT<RPIT<T>>`. This does not try to define `RPIT` but instead relates its generic arguments https://github.com/rust-lang/rust/blob/b9856b6e400709392dd14599265b6fd52fc19f3e/compiler/rustc_infer/src/infer/relate/type_relating.rs#L185-L190
>
> This means we relate `T` with `RPIT<T>`, which results in a defining use `RPIT<T> = T`

I think it's pretty obvious that this is not desirable behavior, and according to the crater run there were no regressions, so let's break this so that we don't have any inference hazards in the new solver.

In the future `what2` may end up compiling again by also falling back to `()`. However, that is not yet guaranteed and the transition to this state is made significantly harder by not temporarily breaking it on the way. It is also concerning to change the inferred hidden type like this without any notification to the user, even if likely not an issue in this concrete case.
…errors

Resolved issue with mismatched types triggering ICE in certain scenarios

## Background

The function `annotate_mut_binding_to_immutable_binding` called in `emit_coerce_suggestions` performs a type comparison between the `expected` and `found` types from `ExpectedFound` in the `TypeError`. This can fail if the `found` type contains a region variable that's been rolled back.

## What is being changed?

This updates `annotate_mut_binding_to_immutable_binding` to use `expr_ty` and `expected` from the parent function instead of the types from the `TypeError`. This sidesteps the issue of using `found` from `TypeError` which may leak lingering inference region variables.

This does change the diagnostic behavior to _only_ support cases where the expected outermost type is `&T`, but that seems to be the intended functionality.

Also fixed the example in the `annotate_mut_binding_to_immutable_binding` rustdocs.

r? rust-lang/types

Fixes rust-lang#140823
…-inconsistency-warning, r=oli-obk

Warning added when dependency crate has async drop types, and the feature is disabled

In continue of rust-lang#141031.

When dependency crate has non-empty `adt_async_destructor` table in metadata, and `async_drop` feature is disabled for local crate, warning will be emitted.

Test `dependency-dropped` has two revisions - with and without feature enabled. With feature enabled, async drop for dropee is executed ("Async drop" printed). Without the feature enabled, sync drop is executed ("Sync drop" printed) and warning is emitted.

Warning example:
```
warning: found async drop types in dependecy `async_drop_dep`, but async_drop feature is disabled for `dependency_dropped`
  --> $DIR/dependency-dropped.rs:7:1
   |
LL | #![cfg_attr(with_feature, feature(async_drop))]
   | ^
   |
   = help: if async drop type will be dropped in a crate without `feature(async_drop)`, sync Drop will be used
```
rustc-dev-guide subtree update

r? `@ghost`
…e, r=compiler-errors

`gather_locals`: only visit guard pattern guards when checking the guard

When checking a pattern with guards in it, `GatherLocalsVisitor` will visit both the pattern (when type-checking the let, arm, or param containing it) and local declarations in the guard expression (when checking the guard itself). This keeps it from visiting the guard when visiting the pattern, since otherwise it would gather locals from the guard twice, which would lead to a delayed bug: "evaluated expression more than once".

Tracking issue for guard patterns: rust-lang#129967
…er-errors

`lower_to_hir` cleanups

Some minor cleanups I made when reading this code.

r? `@Nadrieril`
Add tick to `RePlaceholder` debug output

Present when debug printing canonical queries

r? lcnr
@rustbot rustbot added the A-rustc-dev-guide Area: rustc-dev-guide label May 20, 2025
@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. rollup A PR which is a rollup labels May 20, 2025
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Collaborator

bors commented May 20, 2025

📌 Commit cc0ee34 has been approved by matthiaskrgr

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 May 20, 2025
@bors
Copy link
Collaborator

bors commented May 20, 2025

⌛ Testing commit cc0ee34 with merge 444a627...

@bors
Copy link
Collaborator

bors commented May 20, 2025

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 444a627 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 20, 2025
@bors bors merged commit 444a627 into rust-lang:master May 20, 2025
7 checks passed
@rustbot rustbot added this to the 1.89.0 milestone May 20, 2025
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#139419 Error on recursive opaque ty in HIR typeck 243b336d0d95e12dcfb7c71e238d05e3cea0e9a3 (link)
#141236 Resolved issue with mismatched types triggering ICE in cert… eeff7dd914b09a539bc676459e68dfa58bc9f20c (link)
#141253 Warning added when dependency crate has async drop types, a… 629a62b5a86a107d1e8062ce7a14e4bca5fec8a3 (link)
#141269 rustc-dev-guide subtree update 158a24d4a6748f98300cac7953fe5e06c9e137c9 (link)
#141275 gather_locals: only visit guard pattern guards when check… a29bdb1728f87cb02f44cbcbba69dd71247fb13d (link)
#141279 lower_to_hir cleanups 3ca709f7c518a71f78a3e1c57b201996a2d84f4e (link)
#141285 Add tick to RePlaceholder debug output c44d3f9b1abd9c4849b59c468d7b88fdd7eeaee7 (link)

previous master: 6cab15c1ae

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

Copy link

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 6cab15c (parent) -> 444a627 (this PR)

Test differences

Show 10 test diffs

Stage 1

  • [crashes] tests/crashes/139817.rs: pass -> [missing] (J2)
  • [crashes] tests/crashes/140823.rs: pass -> [missing] (J2)
  • [ui] tests/ui/fn/coerce-suggestion-infer-region.rs: [missing] -> pass (J2)
  • [ui] tests/ui/pattern/rfc-3637-guard-patterns/only-gather-locals-once.rs: [missing] -> pass (J2)
  • [ui] tests/ui/type-alias-impl-trait/match-upvar-discriminant-of-opaque.rs: [missing] -> pass (J2)

Stage 2

  • [crashes] tests/crashes/139817.rs: pass -> [missing] (J0)
  • [crashes] tests/crashes/140823.rs: pass -> [missing] (J0)
  • [ui] tests/ui/fn/coerce-suggestion-infer-region.rs: [missing] -> pass (J1)
  • [ui] tests/ui/pattern/rfc-3637-guard-patterns/only-gather-locals-once.rs: [missing] -> pass (J1)
  • [ui] tests/ui/type-alias-impl-trait/match-upvar-discriminant-of-opaque.rs: [missing] -> pass (J1)

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 444a62712a29e14d3b6147b51fd24e623496bf58 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. x86_64-apple-1: 8723.3s -> 12521.2s (43.5%)
  2. dist-armhf-linux: 5173.5s -> 6106.2s (18.0%)
  3. dist-arm-linux: 5534.4s -> 4596.1s (-17.0%)
  4. dist-x86_64-apple: 10114.8s -> 11324.6s (12.0%)
  5. dist-i686-msvc: 7679.4s -> 6854.5s (-10.7%)
  6. dist-apple-various: 7414.5s -> 8196.7s (10.5%)
  7. aarch64-apple: 4566.5s -> 5031.7s (10.2%)
  8. dist-i686-linux: 6023.4s -> 6626.8s (10.0%)
  9. x86_64-msvc-ext2: 5751.2s -> 6281.3s (9.2%)
  10. x86_64-mingw-2: 7962.1s -> 7247.9s (-9.0%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustc-dev-guide Area: rustc-dev-guide 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.