Canonicalize free regions from inputs as placeholders in root univ rust-lang/rust#155487
@bors try @rust-timer queue
View on GitHub
Awaiting bors try build completion.
@rustbot label: +S-waiting-on-perf
View on GitHub
⌛ Trying commit e431633 with merge e39d9d1…
To cancel the try build, run the command @bors try cancel.
Workflow: https://github.com/rust-lang/rust/actions/runs/24607425420
View on GitHub
Queued e39d9d1 with parent 8da2d28, future comparison URL.
There are currently 0 preceding artifacts in the queue.
It will probably take at least ~1.0 hours until the benchmark run finishes.
Finished benchmarking commit (e39d9d1): comparison URL.
Overall result: ✅ improvements - no action needed
Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.
@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression
Instruction count
Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
- | - | 0 |
| Regressions ❌ (secondary) |
0.2% | [0.1%, 0.3%] | 4 |
| Improvements ✅ (primary) |
-1.1% | [-4.4%, -0.2%] | 40 |
| Improvements ✅ (secondary) |
-1.1% | [-3.2%, -0.1%] | 33 |
| All ❌✅ (primary) | -1.1% | [-4.4%, -0.2%] | 40 |
Max RSS (memory usage)
Results (primary 2.8%, secondary 3.2%)
A less reliable metric. May be of interest, but not used to determine the overall result above.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
2.8% | [1.0%, 5.5%] | 7 |
| Regressions ❌ (secondary) |
3.2% | [1.6%, 4.8%] | 2 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
- | - | 0 |
| All ❌✅ (primary) | 2.8% | [1.0%, 5.5%] | 7 |
Cycles
Results (primary -3.2%, secondary 0.2%)
A less reliable metric. May be of interest, but not used to determine the overall result above.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
- | - | 0 |
| Regressions ❌ (secondary) |
2.1% | [2.1%, 2.1%] | 1 |
| Improvements ✅ (primary) |
-3.2% | [-4.0%, -2.3%] | 7 |
| Improvements ✅ (secondary) |
-1.6% | [-1.6%, -1.6%] | 1 |
| All ❌✅ (primary) | -3.2% | [-4.0%, -2.3%] | 7 |
Binary size
Results (secondary -0.1%)
A less reliable metric. May be of interest, but not used to determine the overall result above.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
- | - | 0 |
| Regressions ❌ (secondary) |
- | - | 0 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
-0.1% | [-0.1%, -0.1%] | 4 |
| All ❌✅ (primary) | - | - | 0 |
Bootstrap: 493.313s -> 494.404s (0.22%)
Artifact size: 394.36 MiB -> 394.31 MiB (-0.01%)
r? lcnr
tests/ui/higher-ranked/trait-bounds/normalize-under-binder/issue-71955.stderr · outdated
| 7 | = note: closure with signature `for<'a> fn(&'a &'2 str) -> bool` must implement `FnOnce<(&&'1 str,)>`, for any lifetime `'1`... |
|
| 8 | = note: ...but it actually implements `FnOnce<(&&'2 str,)>`, for some specific lifetime `'2` |
|
| 7 | = note: closure with signature `for<'a> fn(&'a &'0 str) -> bool` must implement `FnOnce<(&&'1 str,)>`, for any two lifetimes `'0` and `'1`... |
|
| 8 | = note: ...but it actually implements `FnOnce<(&&str,)>` |
We have lots of those ...but it actually implements X for some specific lifetime '?x -> ...but it actually implements X for some specific lifetime diagnostic drifts due to the canonicalized region being placeholders rather than region vars.
I think this is somewhat regressive. Would it make sense to fix them somehow?
I'm not sure I should do something with diagnostics but marking this as ready 😅
compiler/rustc_next_trait_solver/src/canonical/canonicalizer.rs · outdated
| 469 | if !(placeholder.universe() == ty::UniverseIndex::ROOT |
|
| 470 | && placeholder.bound.kind == ty::BoundRegionKind::Anon) |
|
| 471 | && max_input_universe.can_name(placeholder.universe()) |
shouldn't just placeholder.universe() != ty::UniverseIndex::ROOT be enough here. We should never add new placeholders to an existing universe and max_input_universe is guaranteed to be able to the root universe, as can_name is equivalent to max_input_universe >= placeholder.universe()
Wouldn't there still be the cases that adding new placeholders to a new_universe > max_input_universe?
ah, yeah, it's the other way around
- either the placeholder is in the root universe
- or from a universe which can't be named by
max_input_universe
2 thoughts here:
It's somewhat scary that this requires literally no changes when we instantiate query responses. I guess what's happening is that this is (another) load bearing use of query_response_instantiation_guess (old solver) and the hack in compute_query_response_instantiation_values (next solver). We should definitely at least update the comments here.
Related to the above, it feels scary to me to change the old solver canonicalization here. I think your change also makes the different CanonicalizeMode be slightly outdated or what not 🤔 could you just limit this to the new solver for now?
CanonicalizeUserTypeAnnotations is interesting to me as that one is more a canonicalize_response where the var_values are just the early/late bound params in the parent function
View on GitHub
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.
Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.
@bors try @rust-timer queue
View on GitHub
Awaiting bors try build completion.
@rustbot label: +S-waiting-on-perf
View on GitHub
⌛ Trying commit 7f82ec1 with merge ea40b77…
To cancel the try build, run the command @bors try cancel.
Workflow: https://github.com/rust-lang/rust/actions/runs/25006330722
View on GitHub
Queued ea40b77 with parent 2f43fe4, future comparison URL.
There is currently 1 preceding artifact in the queue.
It will probably take at least ~1.0 hours until the benchmark run finishes.
Finished benchmarking commit (ea40b77): comparison URL.
Overall result: ✅ improvements - no action needed
Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.
@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression
Instruction count
Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
- | - | 0 |
| Regressions ❌ (secondary) |
- | - | 0 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
-1.2% | [-2.9%, -0.8%] | 6 |
| All ❌✅ (primary) | - | - | 0 |
Max RSS (memory usage)
Results (secondary -6.9%)
A less reliable metric. May be of interest, but not used to determine the overall result above.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
- | - | 0 |
| Regressions ❌ (secondary) |
- | - | 0 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
-6.9% | [-6.9%, -6.9%] | 1 |
| All ❌✅ (primary) | - | - | 0 |
Cycles
This perf run didn't have relevant results for this metric.
Binary size
This perf run didn't have relevant results for this metric.
Bootstrap: 487.764s -> 486.922s (-0.17%)
Artifact size: 393.48 MiB -> 393.47 MiB (-0.00%)
compiler/rustc_next_trait_solver/src/canonical/mod.rs · outdated
| 218 | // in the canonical form. However, we can still map these back to the original |
|
| 219 | // input regions, as we set their placeholder indices during canonicalization |
|
| 220 | // with there indices in the original values. We deliberately do so because |
|
| 221 | // we don't want to unify any free region into other region in nest contexts |
|
| 222 | // but want to propagate region constraints on them back to the caller. |
|
| 223 | // Therefore, we need to map them back to their original values. |
| // in the canonical form. However, we can still map these back to the original | |
| // input regions, as we set their placeholder indices during canonicalization | |
| // with there indices in the original values. We deliberately do so because | |
| // we don't want to unify any free region into other region in nest contexts | |
| // but want to propagate region constraints on them back to the caller. | |
| // Therefore, we need to map them back to their original values. | |
| // in the canonical form. | |
| // | |
| // We can still map these back to the original input regions, as we | |
| // just instantiate the canonical variable with its corresponding | |
| // `original_value`. | |
| // | |
| // For more information on why we canonicalize all input regions as | |
| // placeholders, see the comment in `Canonicalizer::fold_region`. |
the comment is already quite good, I mostly just started to rewrite it as I was trying to fix a typo. Alternatively just fix that typo and leave the comment otherwise as is.
"we don't want to unify any free region into other region in nest[ed] contexts"
View on GitHub
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.
Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.
@bors r=lcnr
View on GitHub
⌛ Testing commit 926a31b with merge 976cee2...
Workflow: https://github.com/rust-lang/rust/actions/runs/25085326303
View on GitHub
The job dist-x86_64-msvc failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
💔 Test for 976cee2 failed: CI. Failed job:
auto - dist-x86_64-msvc(web logs, enhanced plaintext logs)
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.
Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.
@bors r=lcnr
View on GitHub
⌛ Testing commit f2ae894 with merge c935696...
Workflow: https://github.com/rust-lang/rust/actions/runs/25111175404
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 0424cc1 (parent) -> c935696 (this PR)
Test differences
Show 9 test diffs
Stage 1
[ui] tests/ui/implied-bounds/normalization-preserve-equality.rs#borrowck: pass -> [missing] (J1)[ui] tests/ui/implied-bounds/normalization-preserve-equality.rs#borrowck_current: [missing] -> pass (J1)[ui] tests/ui/implied-bounds/normalization-preserve-equality.rs#borrowck_next: [missing] -> pass (J1)
Stage 2
[ui] tests/ui/implied-bounds/normalization-preserve-equality.rs#borrowck: pass -> [missing] (J0)[ui] tests/ui/implied-bounds/normalization-preserve-equality.rs#borrowck_current: [missing] -> pass (J0)[ui] tests/ui/implied-bounds/normalization-preserve-equality.rs#borrowck_next: [missing] -> pass (J0)
Additionally, 3 doctest diffs were found. These are ignored, as they are noisy.
Job group index
- J0: aarch64-apple, aarch64-gnu, aarch64-gnu-llvm-21-1, aarch64-msvc-1, arm-android, armhf-gnu, dist-i586-gnu-i586-i686-musl, i686-gnu-1, i686-gnu-nopt-1, i686-msvc-1, optional-x86_64-gnu-parallel-frontend, test-various, x86_64-gnu, x86_64-gnu-debug, x86_64-gnu-gcc, x86_64-gnu-llvm-21, x86_64-gnu-llvm-21-2, x86_64-gnu-llvm-22-2, x86_64-gnu-nopt, x86_64-gnu-stable, x86_64-mingw-1, x86_64-msvc-1
- J1: x86_64-gnu-llvm-21-3, x86_64-gnu-llvm-22-3
Test dashboard
Run
cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard c935696dd07ca51e6fba2f6579919eea2a50863b --output-dir test-dashboardAnd then open test-dashboard/index.html in your browser to see an overview of all executed tests.
Job duration changes
- dist-aarch64-linux: 1h 47m -> 2h 58m (+66.2%)
- dist-x86_64-illumos: 1h 37m -> 2h 39m (+63.7%)
- dist-x86_64-llvm-mingw: 1h 29m -> 2h 17m (+54.2%)
- dist-ohos-armv7: 1h 8m -> 1h 37m (+43.6%)
- dist-ohos-x86_64: 1h 14m -> 1h 47m (+43.3%)
- dist-x86_64-freebsd: 1h 24m -> 1h 59m (+41.0%)
- dist-ohos-aarch64: 1h 17m -> 1h 48m (+40.1%)
- dist-aarch64-llvm-mingw: 1h 39m -> 2h 3m (+24.2%)
- dist-x86_64-mingw: 2h 10m -> 2h 37m (+20.8%)
- dist-i686-linux: 1h 51m -> 2h 13m (+19.8%)
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.
Finished benchmarking commit (c935696): comparison URL.
Overall result: ✅ improvements - no action needed
@rustbot label: -perf-regression
Instruction count
Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
- | - | 0 |
| Regressions ❌ (secondary) |
- | - | 0 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
-1.2% | [-2.9%, -0.8%] | 6 |
| All ❌✅ (primary) | - | - | 0 |
Max RSS (memory usage)
Results (secondary 0.2%)
A less reliable metric. May be of interest, but not used to determine the overall result above.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
- | - | 0 |
| Regressions ❌ (secondary) |
1.6% | [1.6%, 1.6%] | 1 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
-1.3% | [-1.3%, -1.3%] | 1 |
| All ❌✅ (primary) | - | - | 0 |
Cycles
Results (secondary -0.3%)
A less reliable metric. May be of interest, but not used to determine the overall result above.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
- | - | 0 |
| Regressions ❌ (secondary) |
2.3% | [2.3%, 2.3%] | 1 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
-2.9% | [-2.9%, -2.9%] | 1 |
| All ❌✅ (primary) | - | - | 0 |
Binary size
This perf run didn't have relevant results for this metric.
Bootstrap: 481.379s -> 487.214s (1.21%)
Artifact size: 390.96 MiB -> 390.94 MiB (-0.01%)
View all comments
Context: The box named coroutine witness Send lifetime requirements now considered by leakcheck this roadmap
Fixes (only for the next-solver) #106569
Prerequisite of #155749