Merge env and alias candidates modulo region constraints#154204
Merge env and alias candidates modulo region constraints#154204adwinwhite wants to merge 1 commit intorust-lang:mainfrom
Conversation
rustbot
added
S-waiting-on-review
There was a problem hiding this comment.
This compiles with the old solver. Intended to break?
There was a problem hiding this comment.
hm, would be good to have revisions for tests to make that difference clearly visible
idk, specialization is weird and dont have a strong opinion here. Would go even further and stop merging impl candidates at all, avoids rust-lang/trait-system-refactor-initiative#35
i guess it's a separate issue is that filter_specialized_impls checks whether the specializing impl has region constraints, instead of whether it has more region constraints than the default
This comment has been minimized.
This comment has been minimized.
adwinwhite
force-pushed
the
enhance-candidate-merge
branch
from
72751f4 to
a831800
Compare
There was a problem hiding this comment.
feels slightly iffy as is rn. I think @ShoyuVanilla deals with one of the weird things here
I think we should separately change our impl handling
I think the region constraint stuff is also weird for any placeholder constraints/unconstrained regions. I would only try to discard constraints if they reference stuff also in the var_values as it's otherwise order dependent whether two constraints are the same or not.
Imagine having two things like the following or whatever
for<'a, 'b> T: Trait<&'a T, &'b U>`
for<'b, 'a> T: Trait<&'a T, &'b U>`one results in T: 'placeholder0 and U: 'placeholder1 and hte other does has the opposite indices. Once we have eager placeholder constraint destructing, cc @BoxyUwU https://rust-lang.github.io/rust-project-goals/2026/assumptions_on_binders.html placeholder constraints will just not exist anymore, fixing that issue without us needing any complicated behavior here
| |a: &CanonicalResponse<I>, b: &CanonicalResponse<I>| { | ||
| // We ignore var_kinds as well since it changes with region constraints. | ||
| // It shouldn't be a problem since it's entirely determined by the response. | ||
| // FIXME: use an override approach? This is too fragile w.r.t field changes. |
There was a problem hiding this comment.
what u can do is to destructure one of the values, i.e.
let CanonicalResponse { max_universe, and_so_on } a;that way u get an error when changing the fields
| let one: CanonicalResponse<I> = candidates[0].result; | ||
| let eq_modulo_region_constraint = | ||
| |a: &CanonicalResponse<I>, b: &CanonicalResponse<I>| { | ||
| // We ignore var_kinds as well since it changes with region constraints. |
There was a problem hiding this comment.
hmm, so that's either
- constraining a region to
'static? - constraining an infer var to a placeholder?
The second one is concerning. The first one we want to avoid anyways at @ShoyuVanilla is currently working on changing input regions to all be universals - i.e. placeholders - instead.
That should allow us to also require the var_kinds to be the same.
One other thought is that we could actually enable param_env normalization again and only handle the unnormalized param_env while actually normalizing it. related to https://rust-lang.zulipchat.com/#narrow/channel/326132-t-types.2Fmeetings/topic/2026-03-31/with/581721469. Unsure whether that means we could avoid merging candidates like this.
There was a problem hiding this comment.
hm, would be good to have revisions for tests to make that difference clearly visible
idk, specialization is weird and dont have a strong opinion here. Would go even further and stop merging impl candidates at all, avoids rust-lang/trait-system-refactor-initiative#35
i guess it's a separate issue is that filter_specialized_impls checks whether the specializing impl has region constraints, instead of whether it has more region constraints than the default
4 participants
Partial fix for rust-lang/trait-system-refactor-initiative#265
We merge identical responses modulo region constraints if one's region constraints is a superset of another's.
The case we currently can't fix is here.
There's no test case for alias bound candidates because they don't have nested goals thus no region_constraints.
r? @lcnr