correct spans in parem_env to deduplicate errors #102502
Conversation
rustbot
added
the
T-compiler
|
r? @cjgillot (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
SparrowLii
force-pushed
the
normalize_param_env
branch
from
f4c3407 to
f0ba469
Compare
| traits::normalize_param_env_or_error(tcx, unnormalized_env, cause) | ||
| let span = tcx.def_span(def_id); | ||
|
|
||
| if is_fn { |
There was a problem hiding this comment.
I'm skeptical that all this extra code is worthwhile... since all it's here for is to deduplicate a diagnostic that's still going to end up being an error. Especially because normalize_param_env_or_error is very delicate logic.
There was a problem hiding this comment.
Thanks for pointing it out. If that isn't worthwhile, could you take a look at the first commit, which is just a little optimization of param_env, and I hope it's acceptable?
There was a problem hiding this comment.
Are you sure this is an optimization in reality? Maybe you should put it up as a separate commit (separate from the span modifications) so we can do a perf run on it.
I am still not confident that the added complexity is worth it.
| @@ -235,7 +237,15 @@ fn do_normalize_predicates<'tcx>( | |||
| // them here too, and we will remove this function when | |||
| // we move over to lazy normalization *anyway*. | |||
| tcx.infer_ctxt().ignoring_regions().enter(|infcx| { | |||
| let predicates = match fully_normalize(&infcx, cause, elaborated_env, predicates) { | |||
| let predicates = match predicates.try_map_id(|predicate| { | |||
There was a problem hiding this comment.
Could you make this a separate local variable?
Would this be simpler as a loop mutating predicates?
There was a problem hiding this comment.
Thanks for reviewing! I renamed it to normalized_predicates. I think It maybe not easy to use a mutating, since fully_normalize needs to take ownership of the elements in predicts. Similar methods that act on instances of TypeFoldable have this feature, and try_map_id is used to implement TypeFoldable for Vec.
There was a problem hiding this comment.
Also, could you make a separate variable for the try_map_id call?
There was a problem hiding this comment.
Okay. I renamed it to origin_predicate. We can pick what we want and sqaush them to one commit I think
| @@ -262,7 +272,15 @@ fn do_normalize_predicates<'tcx>( | |||
| ); | |||
| } | |||
|
|
|||
| match infcx.fully_resolve(predicates) { | |||
| match predicates.try_map_id(|predicate| { | |||
| @@ -219,6 +220,7 @@ fn do_normalize_predicates<'tcx>( | |||
| cause: ObligationCause<'tcx>, | |||
| elaborated_env: ty::ParamEnv<'tcx>, | |||
| predicates: Vec<ty::Predicate<'tcx>>, | |||
There was a problem hiding this comment.
From the use site, should we just take &mut?
There was a problem hiding this comment.
Yea, as mentioned above, fully_normalize requires taking ownership of the predicats. I could not think of a good way to deal with this for a while.
| @@ -219,6 +220,7 @@ fn do_normalize_predicates<'tcx>( | |||
| cause: ObligationCause<'tcx>, | |||
| elaborated_env: ty::ParamEnv<'tcx>, | |||
| predicates: Vec<ty::Predicate<'tcx>>, | |||
| outlives: bool, | |||
There was a problem hiding this comment.
Could you make the parameter name more explicit? I don't understand what this toggles.
There was a problem hiding this comment.
Okay. In param_env, normalization will be called twice according to whether it is a predicate of type TypeOutlives. This variable is used to distinguish between the two calls.
SparrowLii
force-pushed
the
normalize_param_env
branch
from
f45f9c4 to
bbbc9f4
Compare
SparrowLii
force-pushed
the
normalize_param_env
branch
from
6bf7dfe to
c600a8e
Compare
|
☔ The latest upstream changes (presumably #101632) made this pull request unmergeable. Please resolve the merge conflicts. |
|
as @compiler-errors said, The calculation of |
This is part of the workaround for #102185.
param_envwill set all spans to same when normalizing predictions, which is inconsistent with the behavior in typeck, causing the same error to appear twice, and cannot be deduplicated because the spans are different.This PR has two commits. The first one is to optimize the calculation process of
param_envto reduce the number of times to collectVecs. The second is to use correct spans when normalize predictions ofFnitems, so that errors generated is consistent with ones from typeck, so that they can be deduplicated.