Get rid of `StructurallyRelateAliases` by adwinwhite · Pull Request #158731 · rust-lang/rust

adwinwhite · 2026-07-03T12:58:42Z

View all comments

Part of #155345

Finally we can get rid of the last use of StructurallyRelateAliases::Yes.

r? lcnr

rustbot · 2026-07-03T12:58:45Z

changes to the core type system

cc @lcnr

lcnr · 2026-07-03T13:54:57Z

+            }
+
+            (ty::Infer(ty::TyVar(a_vid)), _) => {
+                infcx.equate_ty_var_raw(a_vid, b);


can you add a debug_assert to instantiate_ty_var_raw tht the type we instantiate it with does not refer to anything from a higher universe?

View changes since the review

same for regions and consts :>

The debug_assert in instantiate_ty_var_eq_raw would make tests/ui/async-await/async-closures/post-mono-higher-ranked-hang.rs hang again, probably due to max_universe being slow on deeply nested types.
And these asserts are likely unnecessary as we need to replace the universes anyway? We do meet higher universes in response. Generalization handles this by creating infer vars in current universe and I copied that.

We can't assert for regions either as we encounter higher universes in response var_values.
In proof tree visitor we have newly created original_values in lower universe.
Related #154329 (comment)

The asserts I added: 09388a3

hmm, having this assert right after we have a type folder whicih should do it correctly feels 🤷 to me

I worry about type_variables().instantiate more generally and would love to have that assert in there.

The debug_assert in instantiate_ty_var_eq_raw would make tests/ui/async-await/async-closures/post-mono-higher-ranked-hang.rs hang again, probably due to max_universe being slow on deeply nested types.

does the max_universe visitor use a cache?

lcnr · 2026-07-03T13:56:16Z

+        let a = infcx.shallow_resolve_const(a);
+        let b = infcx.shallow_resolve_const(b);


we should be able to not do that but debug_assert that this is a noop?

View changes since the review

did that work out?

partially, see the fixme.

lcnr · 2026-07-03T13:58:03Z

+        if let Some(a_inner) = a.no_bound_vars()
+            && let Some(b_inner) = b.no_bound_vars()


a and b have exactly the same bound vars here. we don't have to bother with the rebinding 🤔 actually, we can just recurse without instantiating bound vars at all

we can just assert that the bound vars are equal, and then recurse :>

View changes since the review

lcnr

beautiful ❤️ i love rigid aliases

View changes since this review

adwinwhite · 2026-07-08T07:51:40Z

@bors try @rust-timer queue

…=<try> Get rid of `StructurallyRelateAliases`

rust-bors · 2026-07-08T10:00:25Z

☀️ Try build successful (CI)
Build commit: a8102e7 (a8102e767108a4da38b2788dcc91946783fc743e)
Base parent: 99576cf (99576cf2ebc870c305225e9856b9423228872c56)

rust-timer · 2026-07-08T11:00:39Z

Finished benchmarking commit (a8102e7): 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)	-2.8%	[-16.8%, -0.2%]	25
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary 2.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.2%	[0.8%, 3.7%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary -11.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)	-11.1%	[-17.7%, -1.6%]	5
All ❌✅ (primary)	-	-	0

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 488.508s -> 490.377s (0.38%)
Artifact size: 388.95 MiB -> 388.58 MiB (-0.10%)

lcnr · 2026-07-13T09:10:36Z

        self.inner.borrow_mut().const_unification_table().union(a, b);
    }

+    fn instantiate_ty_var_eq_raw(&self, vid: ty::TyVid, ty: Ty<'tcx>) {


rename instantiate_ty_var_raw to generalize_and_instantiate_ty_var_raw maybe? Not a big fan of the eq in instantiate_ty_var_eq_raw

actually, just rename it to instantiate_ty_var. I don't think the _raw is useful and its impl is called instantiate_ty_var already

View changes since the review

lcnr · 2026-07-13T09:14:47Z

+            return answer;
+        }
+
+        let folded = if let ty::Infer(ty::TyVar(vid)) = t.kind() {


can you change this to a match and also add an assert that the universe of placeholders is "correct"

View changes since the review

or I guess don't handle placeholders but do keep the debug_assert 🤔

lcnr · 2026-07-13T09:17:31Z

+// We always set the universes in generalized term to the infer var's
+// in generalization, if those universes are higher than the infer var's.
+// We replicate the behavior here.
+struct UniverseFolder<'a, 'tcx> {


Suggested change

// We always set the universes in generalized term to the infer var's

// in generalization, if those universes are higher than the infer var's.

// We replicate the behavior here.

struct UniverseFolder<'a, 'tcx> {

/// Canonicalizing inputs puts all inference variables and placeholders

/// into the root universe.

///

/// This means when instantiating the query response we need to pull

/// down the universe of returned `var_values` to the universe of

/// the inference variable in `orig_values`.

///

/// This folder is similar to the `Generalizer`, except that it simply

/// structurally folds non-rigid aliases as these should have already

/// been generalized in the query so we shouldn't try to do it again.

struct LowerUniverseFolder<'a, 'tcx> {

View changes since the review

lcnr · 2026-07-13T09:19:42Z

+    }
+}
+
+impl<Infcx, I> PredicateEmittingRelation<Infcx> for ResponseRelating<'_, Infcx, I>


given that we don't use super_combine, do we even need to implement this?

View changes since the review

lcnr · 2026-07-13T09:25:23Z

+            return answer;
+        }
+
+        let folded = if let ty::Infer(ty::TyVar(vid)) = t.kind() {


or I guess don't handle placeholders but do keep the debug_assert 🤔

rustbot · 2026-07-17T06:14:39Z

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.

rust-log-analyzer · 2026-07-17T06:20:45Z

The job x86_64-gnu-gcc-core-tests failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)


error[E0405]: cannot find trait `PredicateEmittingRelation` in this scope
   --> compiler/rustc_next_trait_solver/src/canonical/mod.rs:444:16
    |
444 | impl<Infcx, I> PredicateEmittingRelation<Infcx> for ResponseRelating<'_, Infcx, I>
    |                ^^^^^^^^^^^^^^^^^^^^^^^^^ not found in this scope
    |
help: consider importing one of these traits
    |
 12 + use crate::canonical::relate::combine::PredicateEmittingRelation;

Important

For more information how to resolve CI failures of this job, visit this link.

lcnr · 2026-07-17T09:25:43Z

+                debug_assert!(
+                    self.for_universe.can_name(p.universe),
+                    "variable in universe {:?} can't name type in universe {:?}",
+                    self.for_universe,
+                    p.universe
+                );


can you instead do a separate max_universe check after this folder has returned. Given that we have a fast path for

if !(t.has_free_regions() || t.has_infer()) { return t; }

this is unreachable

View changes since the review

lcnr · 2026-07-17T09:29:28Z

@@ -1,3 +1,4 @@
+use rustc_data_structures::fx::FxHashSet;


we should lint on using rustc_data_structures in rustc_type_ir as it breaks rustanalyzer.

there's rustc_type_ir::data_structures::HashMap instead

View changes since the review

lcnr · 2026-07-17T09:31:58Z

+        // FIXME: make this a debug_assert.
+        // Currently proof tree evaluation can unify infer vars in original
+        // vars while not resolving them.
+        // See `tests/ui/traits/next-solver/transmute-from-async-closure.rs`


I feel like proof tree evaluation really shouldn't use this at all 😁

proof trees are just generally deeply annoying 😅 :x I guess this is fine for now

View changes since the review

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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Jul 3, 2026

rustbot assigned lcnr Jul 3, 2026