Remove the NormalizesTo/Projection split#154108
Draft
ShoyuVanilla wants to merge 2 commits intorust-lang:mainfrom
Draft
Remove the NormalizesTo/Projection split#154108ShoyuVanilla wants to merge 2 commits intorust-lang:mainfrom
NormalizesTo/Projection split#154108ShoyuVanilla wants to merge 2 commits intorust-lang:mainfrom
Conversation
rustbot
added
S-waiting-on-author
ShoyuVanilla
commented
Mar 19, 2026
| ty::AliasTermKind::ProjectionTy | ||
| | ty::AliasTermKind::ProjectionConst | ||
| | ty::AliasTermKind::InherentTy | ||
| | ty::AliasTermKind::InherentConst |
Member
Author
There was a problem hiding this comment.
As the NormalizesTo is going to be replaced by Projection, this function can be called upon some non-projection aliases
Contributor
There was a problem hiding this comment.
an exhaustive match please (and comment)
This comment has been minimized.
This comment has been minimized.
ShoyuVanilla
force-pushed
the
goodbye-normalizes-to
branch
from
5b630c4 to
83d432a
Compare
ShoyuVanilla
commented
Mar 19, 2026
| // FIXME: Longterm canonical queries should deal with all placeholders | ||
| // created inside of the query directly instead of returning them to the | ||
| // caller. | ||
| let prev_universe = delegate.universe(); |
Member
Author
There was a problem hiding this comment.
The change here is
- normal instantiation: unaffected.
- instantiation inside the very same query: use the
max_input_universeinstead of the delegate's universe.
Currently, we don't instantiate the canonicalized response within the very same query, but as per rust-lang/trait-system-refactor-initiative#223 (comment) (5. the root of the Projection goal then instantiates this query result inside of the same query using the var_values with the added entry for the new infer var) we should do.
In such cases, the current instantiation is not so idempotent modulo universes.
Suppose that the current query's max_input_universe is U(N) and we have the max universe U(N + 1) when computing the projection goal by here, because we have some higher-kinded binder in the goal,
rust/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs
Lines 564 to 567 in 1f7f8ea
rust/compiler/rustc_next_trait_solver/src/canonical/mod.rs
Lines 139 to 143 in 1f7f8ea
And we have some max universe U(M) in the probe's canonicalized response, instantiating it increases the current query's universe index by M, so it becomes U(N + 1 + M) and the prev_universe is U(N + 1).
Suppose that we have a region var with universe U(i) in the probe's canonical response.
We instantiate it into the universe U(i + N + 1) in the following lines
rust/compiler/rustc_next_trait_solver/src/canonical/mod.rs
Lines 187 to 194 in 1f7f8ea
Then when we finally finishing the query, we canonicalize it as a response, but since our max_input_universe is U(N)
rust/compiler/rustc_next_trait_solver/src/canonical/canonicalizer.rs
Lines 458 to 468 in 1f7f8ea
rust/compiler/rustc_next_trait_solver/src/canonical/canonicalizer.rs
Lines 300 to 313 in 1f7f8ea
the resulting universe of that region becomes U(i + N + 1 - N) = U(i + 1), increased by 1 from the probe's response.
This might result into a leak check failure or other higher-kinded error. So, we should either do use max_input_universe to instantiate the canonical response from the probe or use the delegate's universe when canonicalizing the final response. I chose the former as it feels more hard to mistake with. (Sorry for the messy language since I'm very tired rn 😅 )
Contributor
There was a problem hiding this comment.
i agree, annoying issue. I think we shouldn't canonicalize an input while inside of a binder, so not immediately clear how to handle higher-kinded Projection goals 🤔 I think this is definitely an issue, not happy with your current fix for it though 😅
Member
Author
There was a problem hiding this comment.
Yeah, I'll think about the proper fix 😅
Edit) Looks like the intermediate step you've suggested would be the road to it, right?
ShoyuVanilla
commented
Mar 19, 2026
| } | ||
| }) | ||
| })?; | ||
| debug_assert!(resp.value.external_constraints.normalization_nested_goals.is_empty()); |
Member
Author
There was a problem hiding this comment.
ShoyuVanilla
commented
Mar 19, 2026
| return Some( | ||
|
|
||
| let term = self.next_term_infer_of_kind(goal.predicate.term); | ||
| self.eq(goal.param_env, goal.predicate.term, term)?; |
Member
Author
There was a problem hiding this comment.
I'm adding another fresh inference var to the Projection goal's term here, besides the unconstrained one below and it's a bit subtle.
Currently, NormalizesTo goals cannot be a root goal so if it's term is an alias type, it is replaced by an inference var when being added as a nested goal in EvalCtxt::add_goal.
But as it becomes the Projection goal and can be evaluated as a root goal in some places such as normalization, it's term can be an alias type, which isn't replaced by an inference var.
That's a bit sad especially when I'm calling eq_structurally_relating_alias after the probe.
It should be normalized or otherwise fail the evaluation, so I'm replacing it with an inference var and the candidate evaluation inside the probe would either normalize it or fail the current query (I feel both ways are correct)
ShoyuVanilla
commented
Mar 19, 2026
| let resp = self.probe(|_| ProbeKind::ShadowedEnvProbing).enter(|ecx| { | ||
| ecx.var_values.var_values = extended_var_values; | ||
| ecx.var_kinds = extended_var_kinds; | ||
| ecx.current_goal_kind = CurrentGoalKind::NormalizesTo; |
Member
Author
There was a problem hiding this comment.
48e119ef#diff-3cb8110806ddb0ad4c49129dde2f9656727f8baf1f9f7456bfe6d008c5222009
Exactly the same purpose on candidate selection 😄
ShoyuVanilla
commented
Mar 19, 2026
| goal: Goal<I, ty::ProjectionPredicate<I>>, | ||
| term: I::Term, | ||
| ) { | ||
| ) -> Result<(), NoSolution> { |
Member
Author
There was a problem hiding this comment.
The above premise on the function's comment is not true anymore, so Result instead of ICE
Contributor
There was a problem hiding this comment.
this should still hold inside of the nested candidate assemble instantiate 🤔 would expect that we use normal eq outside of it
Member
Author
There was a problem hiding this comment.
Okay, I'll revert this and check whether it triggers ICE
ShoyuVanilla
commented
Mar 19, 2026
| goal: Goal<I, ty::ProjectionPredicate<I>>, | ||
| term: ty::AliasTerm<I>, | ||
| ) { | ||
| ) -> Result<(), NoSolution> { |
ShoyuVanilla
commented
Mar 19, 2026
| extended_var_kinds.push(extra_var_kind); | ||
| let extended_var_kinds = cx.mk_canonical_var_kinds(&extended_var_kinds); | ||
|
|
||
| let resp = self.probe(|_| ProbeKind::ShadowedEnvProbing).enter(|ecx| { |
Member
Author
There was a problem hiding this comment.
The probe kind and the following EvalCtxt's field visivility changes are temporary PoCs 😅
Member
Author
|
r? lcnr The overall implementation is a coarse PoC so far, and I'd like check whether I'm understanding your initial design correctly 😄 I somehow ended up with |
ShoyuVanilla
force-pushed
the
goodbye-normalizes-to
branch
from
83d432a to
cfbd86e
Compare
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
lcnr
reviewed
Mar 24, 2026
Contributor
There was a problem hiding this comment.
hmm, as an intermediate step, could you remove the NormalizesTo special handling and do this nested canonical stuff inside of NormalizesTo, with a strict requiremetn that the goal itself is not higher-ranked, so we don't need to deal with the binder jank?
What is the current breakage/bugs here btw
| ty::AliasTermKind::ProjectionTy | ||
| | ty::AliasTermKind::ProjectionConst | ||
| | ty::AliasTermKind::InherentTy | ||
| | ty::AliasTermKind::InherentConst |
Contributor
There was a problem hiding this comment.
an exhaustive match please (and comment)
| // FIXME: Longterm canonical queries should deal with all placeholders | ||
| // created inside of the query directly instead of returning them to the | ||
| // caller. | ||
| let prev_universe = delegate.universe(); |
Contributor
There was a problem hiding this comment.
i agree, annoying issue. I think we shouldn't canonicalize an input while inside of a binder, so not immediately clear how to handle higher-kinded Projection goals 🤔 I think this is definitely an issue, not happy with your current fix for it though 😅
| goal: Goal<I, ty::ProjectionPredicate<I>>, | ||
| term: I::Term, | ||
| ) { | ||
| ) -> Result<(), NoSolution> { |
Contributor
There was a problem hiding this comment.
this should still hold inside of the nested candidate assemble instantiate 🤔 would expect that we use normal eq outside of it
Contributor
|
especially: how many of these issues are due to diagnostics changes, and what are the non-diagnostics bugs? |
Member
Author
Okay, I'll try this. Do you mean the following lines by rust/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs Lines 656 to 667 in 0312931
I guess currently all the remaning ones would be unblessed diagnostics drifts or fulfillment error reporting things as I haven't adjusted |
Contributor
|
☔ The latest upstream changes (presumably #154637) made this pull request unmergeable. Please resolve the merge conflicts. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cc rust-lang/trait-system-refactor-initiative#223