move `TypeOutlives` normalization out of region checking by JohnTitor · Pull Request #152270 · rust-lang/rust

JohnTitor · 2026-02-07T07:30:28Z

This adds two more normalization steps for outlives before region checking, as mentioned in the issue.

r? @lcnr
Fixes #151461 and fixes rust-lang/trait-system-refactor-initiative#260

lcnr · 2026-02-11T14:41:45Z

compiler/rustc_trait_selection/src/regions.rs

+    for obligation in infcx.take_registered_region_obligations() {
+        if seen_outputs.insert((obligation.sup_type, obligation.sub_region)) {
+            normalized_obligations.push(obligation);
+        }
+    }


we can't guarantee that the newly registered obligations are normalized.

What I would like to do instead is to normalize in fn compute_type_outlives_goal, rn we register the goal right away, but we could deeply normalize the type first. This means that the trait solver handles overflow for us then.

We don't have a folder to deeply normalize in the solver, so either duplicate the existing impl used by deeply_normalize (which can only be used outside of the solver) ,and change it to use an ecx, or make it generic over the normalization procedure

we should entirely remove the fn resolve_regions_with_normalize and instead expect the obligations to already be normalized

I see removed the normalization steps (I couldn't remove function entirely as it's used in rustc_trait_selection) in region checking and tried to implement it on next solver: 3f9571c

I don't think I've implemented normalizer on next solver completely but would like to receive your review.

(Seems ICE'd but I'd like to confirm I'm going to the right way)

rustbot · 2026-02-14T09:54:33Z

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

lcnr · 2026-02-14T17:58:57Z

compiler/rustc_infer/src/infer/outlives/obligations.rs

        assert!(!self.in_snapshot(), "cannot process registered region obligations in a snapshot");

-        // Must loop since the process of normalizing may itself register region obligations.
+        // We loop to handle the case that processing one obligation ends up registering another.


this should now never happen. Afaik we only added the loop in the PR doing normalization. Probably useful to look at the git blame here to revert pretty much all the unnecessary changes here

I see, addressed: be997ff

lcnr · 2026-02-14T18:01:13Z

compiler/rustc_trait_selection/src/regions.rs

+    /// With the next solver, `TypeOutlives` goals are normalized while they are
+    /// evaluated (and before being registered as region obligations), so region
+    /// resolution does not need to normalize them.


I think the framing of this comment only makes sense while doing the change in this PR and is otherwise confusing.

I think if we want to talk about normalization at all here, we should mention that type outlives obligations are expected to already be normalized at this point.

It was almost for myself, removed: 794d33d (this PR)

lcnr · 2026-02-14T18:01:32Z

tests/ui/traits/next-solver/undeclared-lifetime-no-ice-issue-151461.rs

@@ -0,0 +1,15 @@
+//@ compile-flags: -Znext-solver=globally
+


please add comment explaining what we're testing here and why it ICE'd

Added with link: c7b2252

lcnr · 2026-02-14T18:06:08Z

compiler/rustc_trait_selection/src/solve/delegate.rs

+                        &ObligationCause::dummy_with_span(span),
+                    );
+                    Some(Certainty::Yes)
+                }


try removing the fast path. I think it's not necessary for performance and easy to get wrong. We currently incorrectly register things if they have inference variables in them. These inference variables could then get instantiated with something that references an (higher-ranked) alias we have to normalize

Removed: 2f8c93e

lcnr · 2026-02-14T18:07:49Z

compiler/rustc_next_trait_solver/src/solve/mod.rs

+        // obligations so that later processing does not have to structurally process aliases.
+        let ty = self.resolve_vars_if_possible(ty);
+        let ty = if ty.has_aliases() {
+            self.deeply_normalize_for_outlives(goal.param_env, ty)


do the has_aliases check in the function instead of before calling it 🤔 while moving these sorts of checks out functions is generally good, here this check is an optimization of this function, so it should stay inside of it

Ok, addressed: 471f298

lcnr · 2026-02-14T18:10:08Z

compiler/rustc_next_trait_solver/src/solve/mod.rs

+                    return Ok(ct);
+                }
+
+                let ty::ConstKind::Unevaluated(..) = ct.kind() else {


style: please use an if let instead of an early return

Addresed in 471f298

lcnr · 2026-02-14T18:10:14Z

compiler/rustc_next_trait_solver/src/solve/mod.rs

+                    return Ok(ty);
+                }
+
+                let ty::Alias(..) = ty.kind() else { return ty.try_super_fold_with(self) };


Addresed in 471f298

lcnr · 2026-02-14T18:21:53Z

compiler/rustc_next_trait_solver/src/solve/mod.rs

+                    ),
+                );
+
+                let result = match self.ecx.try_evaluate_goal(GoalSource::TypeRelating, goal) {


Why try_evaluate_goal?

Thinking about how to implement this, I'd take a look at the FindParamInClause visitor

I expect this function to do:

call structurally_normalize_term instead of manually evaluating

if you encounter an infer var, return Certainty::Maybe (change the error ty of the type folder to be Result<Certainty, NoSolution>)

if it errors, return an error

if you hit the recursion limit, return overflow (i think FindParamInClause also needs to check the recursion limit and can currently cause stack overflows)

With this the returned type should be fully normalized

when encountering binders, you need to map the bound vars to placeholders, and then map placeholders back to bound vars on success (same as the deeply_normalize folder

Addressed in 471f298 but I'm still not sure if it follows your intention completely. I had to tweak some checks in compute_type_outlives_goal otherwise it'd emit spammy recursion limit errors for may test cases on next solver.

lcnr · 2026-02-15T23:00:56Z

compiler/rustc_next_trait_solver/src/solve/mod.rs

+            self.probe(|_| inspect::ProbeKind::NormalizedSelfTyAssembly)
+                .enter(|ecx| ecx.deeply_normalize_for_outlives(goal.param_env, ty))
+                .ok()
+                .unwrap_or(ty)


what do you mean with "this avoids spurious overflows"?

I would expect us to always deeply_normalize and if we returb Err(Ok(certainty)) we don't register a type outlives but instead call evaluate_added_goals_and_make_canonical_response with that certainty. Actually, please change deeply_normalize_for_outlvies to return a MaybeCause instead as we should never return Certainty::Yes

Ahhh, I see, that's why other compiler error tests emit recursion limit errors on that change.
Changed the response type and corrected the checks with MaybeCause: 1d8274e

lcnr · 2026-02-16T13:33:15Z

compiler/rustc_next_trait_solver/src/solve/mod.rs

+        let ty = self.shallow_resolve(ty);
+        if !ty.has_aliases() {
+            return Ok(ty);
+        }
+        if ty.has_non_region_infer() {
+            return Err(Ok(MaybeCause::Ambiguity));
+        }
+        if ty.has_opaque_types() || ty.has_non_region_param() || ty.has_non_region_placeholders() {
+            return Ok(ty);
+        }


this seems odd, why?

why return if it has opaques or placeholders

We do need to normalize if the ty has no aliases but infer vars

Otherwise tests/ui/traits/next-solver/cycles/cycle-modulo-ambig-aliases.rs starts to emit overflow errors, is it ok here?

this test already emits overflow errors, so this seems fine 🤔

Addressed in fd760f2.
But it now returns a lot of recursion errors, maybe we could stop emitting by comparing alias ty with arg ty?
And I've also seen a lot of unwanted (I think) errors/ICEs. I've struggled with this for a while but no clue, I'd like to ask what's I'm wrong here.

lcnr · 2026-02-16T13:34:56Z

compiler/rustc_next_trait_solver/src/solve/mod.rs

+                    || ty.has_non_region_placeholders()
+                {
+                    return Err(Ok(MaybeCause::Ambiguity));
+                }


match on ty. if infer, return ambiguity, if alias ,normalize, otherwise, recur

Addressed in fd760f2 (but I'm struggled, see above).

lcnr · 2026-02-16T13:36:06Z

compiler/rustc_next_trait_solver/src/placeholder.rs

 }
+
+/// The inverse of [`BoundVarReplacer`]: replaces placeholders with the bound vars from which
+/// they came.


remove the version outside of rustc_next_trait-solver and use this one in rustc_trait_selection

Ah sorry it exists, addressed in ce67440.

lcnr · 2026-02-27T13:55:26Z

compiler/rustc_next_trait_solver/src/solve/mod.rs

+                    opaque_types_jank: OpaqueTypesJank::AllGood,
+                });
+            }
+            Err(Err(NoSolution)) => ty,


propagate the Err(NoSolution) to the caller. The TypeOutlives goal doesn't hold if normalization failed

Addressed in 2bc1fa0!

rustbot · 2026-03-04T12:48:49Z

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.

rustbot assigned lcnr Feb 7, 2026

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 Feb 7, 2026

lcnr reviewed Feb 11, 2026

View reviewed changes

JohnTitor force-pushed the normalize-before-region-obl branch from af4e816 to 3f9571c Compare February 14, 2026 09:54

This comment has been minimized.

Sign in to view

lcnr reviewed Feb 14, 2026

View reviewed changes

lcnr reviewed Feb 15, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

JohnTitor force-pushed the normalize-before-region-obl branch from 471f298 to 1d8274e Compare February 16, 2026 13:20

This comment has been minimized.

Sign in to view

lcnr reviewed Feb 16, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

lcnr reviewed Feb 27, 2026

View reviewed changes

JohnTitor added 10 commits March 4, 2026 21:41

move TypeOutlives normalization out of region checking

c2dbb22

Deeply normalize TypeOutlives on next solver

fc330f4

loop is no longer necessary

41f0789

remove confusing comment

4a22b67

add comment and link to test

0e6b653

remove fast path

5681d2b

normalize correctly

1f6ac9d

return MaybeCause

bd54d19

Prefer recursive errors

fd760f2

Replace PlaceholderReplacer

ce67440

Propagate error

2bc1fa0

JohnTitor force-pushed the normalize-before-region-obl branch from 1d8274e to 2bc1fa0 Compare March 4, 2026 12:48

Uh oh!

Conversation

JohnTitor commented Feb 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JohnTitor Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Feb 14, 2026

Uh oh!

This comment has been minimized.

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!

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!

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!

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!

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!

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!

This comment has been minimized.

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

rustbot commented Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

JohnTitor Feb 14, 2026 •

edited

Loading