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.

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

Improve bound const handling #144677

Are you sure you want to change the base?

Improve bound const handling #144677

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!

Uh oh!