Improve bound const handling #144677

nnethercote · 2025-07-30T10:15:24Z

A few changes to make const handling more similar to type handling.

r? @compiler-errors -errors

rustbot · 2025-07-30T10:15:29Z

compiler-errors is not on the review rotation at the moment.
They may take a while to respond.

rustbot · 2025-07-30T10:15:30Z

changes to the core type system

cc @compiler-errors, @lcnr

HIR ty lowering was modified

cc @fmease

nnethercote · 2025-07-30T10:16:08Z

The third commit in particular is total cargo cult programming, me looking at the type handling code and assuming that doing the same thing for consts is reasonable. Let me know if that's wrong!

lcnr · 2025-07-30T11:59:20Z

compiler/rustc_type_ir/src/binder.rs

 }

 impl<I: Interner> ValidateBoundVars<I> {
    pub fn new(bound_vars: I::BoundVarKinds) -> Self {
        ValidateBoundVars {
            bound_vars,
            binder_index: ty::INNERMOST,
-            visited: SsoHashSet::default(),
+            visited_tys: SsoHashSet::default(),


this cache is only for perf. I think we generally only cache tys as visiting consts has to go through a ty at some point.

We have a bunch of Ty based caches in folders regardless of whether they treat tys and consts the same

So should I just remove visited_consts and the || !self.visited_consts.insert((self.binder_index, c)) part of the condition?

yeah, remove the caching in visit_const

lcnr · 2025-07-30T11:59:34Z

r=me after nit

nnethercote · 2025-07-31T01:45:33Z

@bors try @rust-timer queue

rust-bors · 2025-07-31T01:45:37Z

⌛ Trying commit 0a606b9 with merge 301f9b5…

To cancel the try build, run the command @bors try cancel.

Improve bound const handling

rust-bors · 2025-07-31T04:00:37Z

☀️ Try build successful (CI)
Build commit: 301f9b5 (301f9b5c0f279f8e2ad23856c4e4bd678eb001d9, parent: 606dcc0d2e54d260f67d8a91f8adaf797a4ed38a)

rust-timer · 2025-07-31T07:27:59Z

Finished benchmarking commit (301f9b5): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

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

Max RSS (memory usage)

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

Cycles

Results (primary 2.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

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

Binary size

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

Bootstrap: 468.729s -> 467.982s (-0.16%)
Artifact size: 376.81 MiB -> 376.78 MiB (-0.01%)

lcnr · 2025-07-31T08:08:10Z

compiler/rustc_type_ir/src/binder.rs

+    // A cache for types. We may encounter the same variable at different levels of binding, so
+    // this can't just be `Ty`.
+    visited_tys: SsoHashSet<(ty::DebruijnIndex, I::Ty)>,


hmm, I feel like this comment/name is now more confusing. I think keeping it as visited is better, but feel free to add a comment for "we only cache types as any complex const will have to step through a type at some point anyways".

lcnr

final nit

Type folders can only modify a few "types of interest": `Binder`, `Ty`, `Predicate`, `Clauses`, `Region`, `Const`. Likewise for type visitors, but they can also visit errors (via `ErrorGuaranteed`). Currently the impls of `try_super_fold_with`, `super_fold_with`, and `super_visit_with` do more than they need to -- they fold/visit values that cannot contain any types of interest. This commit removes those unnecessary fold/visit operations, which makes these impls more similar to the impls for `Ty`. It also removes the now-unnecessary derived impls for the no-longer-visited types.

Currently there is `Ty` and `BoundTy`, and `Region` and `BoundRegion`, and `Const` and... `BoundVar`. An annoying inconsistency. This commit repurposes the existing `BoundConst`, which was barely used, so it's the partner to `Const`. Unlike `BoundTy`/`BoundRegion` it lacks a `kind` field but it's still nice to have because it makes the const code more similar to the ty/region code everywhere. The commit also removes `impl From<BoundVar> for BoundTy`, which has a single use and doesn't seem worth it. These changes fix the "FIXME: We really should have a separate `BoundConst` for consts".

Alongside the existing type and region checking.

nnethercote · 2025-07-31T09:55:34Z

@bors r=lcnr

bors · 2025-07-31T09:55:36Z

📌 Commit 64be8bb has been approved by lcnr

It is now in the queue for this repository.

bors · 2025-08-03T05:26:47Z

⌛ Testing commit 64be8bb with merge da19b9d...

bors · 2025-08-03T08:35:50Z

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing da19b9d to master...

github-actions · 2025-08-03T08:38:55Z

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 5b9564a (parent) -> da19b9d (this PR)

Test differences

Show 15 test diffs

15 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard da19b9d24c4ed900b91b45ec7f7795ec43613b1e --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

dist-x86_64-apple: 12401.4s -> 8556.4s (-31.0%)
x86_64-gnu-aux: 7277.1s -> 6355.8s (-12.7%)
aarch64-apple: 5811.2s -> 5140.4s (-11.5%)
dist-armhf-linux: 5147.4s -> 4709.1s (-8.5%)
x86_64-apple-1: 8106.3s -> 7608.2s (-6.1%)
aarch64-gnu: 6984.8s -> 7404.6s (6.0%)
test-various: 5117.2s -> 4812.2s (-6.0%)
dist-android: 2644.5s -> 2501.0s (-5.4%)
x86_64-gnu-nopt: 7913.4s -> 7488.4s (-5.4%)
dist-i686-msvc: 7552.5s -> 7160.1s (-5.2%)

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.

rust-timer · 2025-08-03T09:47:10Z

Finished benchmarking commit (da19b9d): comparison URL.

Overall result: ❌ regressions - 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)	1.4%	[1.4%, 1.4%]	1
Regressions ❌ (secondary)	1.4%	[1.4%, 1.4%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.4%	[1.4%, 1.4%]	1

Max RSS (memory usage)

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

Cycles

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

Binary size

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

Bootstrap: 468.772s -> 467.648s (-0.24%)
Artifact size: 377.01 MiB -> 376.99 MiB (-0.00%)

rustbot assigned compiler-errors Jul 30, 2025

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. labels Jul 30, 2025

This comment has been minimized.

Sign in to view

nnethercote force-pushed the bound-const-handling branch from 8e58035 to c43a0de Compare July 30, 2025 11:17

lcnr reviewed Jul 30, 2025

View reviewed changes

compiler-errors assigned lcnr and unassigned compiler-errors Jul 30, 2025

nnethercote force-pushed the bound-const-handling branch from c43a0de to 0a606b9 Compare July 30, 2025 22:29

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Jul 31, 2025

Auto merge of #144677 - nnethercote:bound-const-handling, r=<try>

301f9b5

Improve bound const handling

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 31, 2025

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 31, 2025

lcnr reviewed Jul 31, 2025

View reviewed changes

nnethercote added 3 commits July 31, 2025 19:29

Check consts in ValidateBoundVars.

64be8bb

Alongside the existing type and region checking.

nnethercote force-pushed the bound-const-handling branch from 0a606b9 to 64be8bb Compare July 31, 2025 09:55

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 Jul 31, 2025

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 3, 2025

bors merged commit da19b9d into rust-lang:master Aug 3, 2025
11 checks passed

rustbot added this to the 1.91.0 milestone Aug 3, 2025

nnethercote deleted the bound-const-handling branch August 3, 2025 09:56

Improve bound const handling #144677

Improve bound const handling #144677

Uh oh!

Conversation

nnethercote commented Jul 30, 2025

Uh oh!

rustbot commented Jul 30, 2025

Uh oh!

rustbot commented Jul 30, 2025

Uh oh!

nnethercote commented Jul 30, 2025

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lcnr commented Jul 30, 2025

Uh oh!

nnethercote commented Jul 31, 2025

Uh oh!

This comment has been minimized.

rust-bors bot commented Jul 31, 2025

Uh oh!

rust-bors bot commented Jul 31, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Jul 31, 2025

Overall result: no relevant changes - no action needed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lcnr left a comment

Choose a reason for hiding this comment

Uh oh!

nnethercote commented Jul 31, 2025

Uh oh!

bors commented Jul 31, 2025

Uh oh!

bors commented Aug 3, 2025

Uh oh!

bors commented Aug 3, 2025

Uh oh!

Uh oh!

github-actions bot commented Aug 3, 2025

Test differences

Job duration changes

Uh oh!

rust-timer commented Aug 3, 2025

Overall result: ❌ regressions - no action needed

Uh oh!

Uh oh!