Do not mix normalized and unnormalized caller bounds when constructing param-env for receiver_is_dispatchable
#138941
Conversation
rustbot
added
S-waiting-on-review
|
did you forget to commit that test? |
|
probably :) |
compiler-errors
force-pushed
the
receiver-is-dispatchable-bounds
branch
from
086e683 to
cd7c99b
Compare
| // after normalizing `<PF as TraitC>::Value = <PF as TraitD>::Scalar`. Since | ||
| // `normalize_param_env_or_error` ends up re-elaborating `PF: TraitD`, we'd | ||
| // end up with both versions of this predicate (normalized and unnormalized). | ||
| // Since these projections preds are not equal, we'd fail with ambiguity. |
There was a problem hiding this comment.
its during normalize_param_env_or_error that we error due to there being two candidates? or is this a weird case where it doesnt wind up actually normalizing like it should
There was a problem hiding this comment.
We error during normalize_param_env_or_error b/c we have two candidates for normalizing <PF as TraitC>::Value, which shows up in the supertrait TraitB<Self::Value>. The first candidate comes from normalizing the param-env the first time in the param_env query, and the second comes from the second time we normalize it.
| @@ -583,27 +583,29 @@ fn receiver_is_dispatchable<'tcx>( | |||
| // create a modified param env, with `Self: Unsize<U>` and `U: Trait` (and all of | |||
| // its supertraits) added to caller bounds. `U: ?Sized` is already implied here. | |||
| let param_env = { | |||
| let param_env = tcx.param_env(method.def_id); | |||
| // N.B. We generally want to emulate the construction of the `unnormalized_param_env` | |||
There was a problem hiding this comment.
So this currently misses out on the constness bounds which I guess is fine because associated type normalization doesn't care about constness? And also because DispatchFromDyn isnt a const_trait so it can only have non-const impls so again we don't really care about constness of stuff in the env.
We also miss out on the RPITIT projection preds? That's probable fine because it's not like we can define opaque types in this analysis anyway. I am assuming that the "assoc type" effectively has the item bounds of the opaque too so it won't result in being unable to prove stuff 🤷♀️
Is that roughly correct? Can u add a small comment noting why we don't need to emulate those parts of param env construction
There was a problem hiding this comment.
Precisely correct.
I could add the code to emulate those too but at that point I'd rather abstract this out into code shared by both the paramenv query and this, which is overkill imo (since it's still duplicated but slightly different for compare-impl-item).
I'll add a comment.
BoxyUwU
added
S-waiting-on-author
…g param-env for receiver_is_dispatchable
compiler-errors
force-pushed
the
receiver-is-dispatchable-bounds
branch
from
cd7c99b to
4f2baaa
Compare
|
k i added a comment |
See comments in code and in test I added.
r? @BoxyUwU since you reviewed the last PR, or reassign
Fixes #138937