-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deeply normalize TypeTrace
when reporting type error in new solver
#131756
Deeply normalize TypeTrace
when reporting type error in new solver
#131756
Conversation
@@ -1136,7 +1147,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { | |||
diag: &mut Diag<'_>, | |||
cause: &ObligationCause<'tcx>, | |||
secondary_span: Option<(Span, Cow<'static, str>)>, | |||
mut values: Option<ValuePairs<'tcx>>, | |||
mut values: Option<(ValuePairs<'tcx>, ty::ParamEnv<'tcx>)>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i feel like the param env should always be required together with the cause
🤔
looking at the uses of note_type_err
I feel like this should be a straightforward change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, so the param-env of this ValuePairs
is conceptually distinct from param-env of the TypeError
that may be emitted, since (well, in the future perhaps) the TypeError
may have instantiated binders in scope as well.
So like, imagine a type error between1 for<T: Foo<Assoc = i32>> fn(<T as Foo>::Assoc)
and fn(u32)
. The valuepairs will contain the outer types, but the type error will likely be a TypeError::Sorts
mismatch between <!T as Foo>::Assoc
2 (w/ the param-env augmented with !T: Foo<Assoc = i32>
) and u32
. I think that other param-env may ned to be tracked separately.
I think we could still pass in the "outermost" param-env as a separate arg, but it seems kinda a waste since that param-env really is tightly associated only w the ValuePairs
...
thoughts?
Footnotes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you're right 😢 😁
vibeck: ParamEnvAnd
instead of a tuple here?
anyways, r=me
r=me after nit |
I intend to fix this in the near future, can you add a |
☔ The latest upstream changes (presumably #131797) made this pull request unmergeable. Please resolve the merge conflicts. |
405013e
to
693f2bb
Compare
☔ The latest upstream changes (presumably #131948) made this pull request unmergeable. Please resolve the merge conflicts. |
693f2bb
to
4217b87
Compare
@bors r=lcnr |
Rollup of 10 pull requests Successful merges: - rust-lang#130225 (Rename Receiver -> LegacyReceiver) - rust-lang#131169 (Fix `target_vendor` in QNX Neutrino targets) - rust-lang#131623 (misc cleanups) - rust-lang#131756 (Deeply normalize `TypeTrace` when reporting type error in new solver) - rust-lang#131898 (minor `*dyn` cast cleanup) - rust-lang#131909 (Prevent overflowing enum cast from ICEing) - rust-lang#131930 (Don't allow test revisions that conflict with built in cfgs) - rust-lang#131956 (coverage: Pass coverage mappings to LLVM as separate structs) - rust-lang#132076 (HashStable for rustc_feature::Features: stop hashing compile-time constant) - rust-lang#132088 (Print safety correctly in extern static items) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#131756 - compiler-errors:deeply-normalize-type-err, r=lcnr Deeply normalize `TypeTrace` when reporting type error in new solver Normalize the values that come from the `TypeTrace` for various type mismatches. Side-note: We can't normalize the `TypeError` itself bc it may come from instantiated binders, so it may reference values from within the probe... r? lcnr
Normalize the values that come from the
TypeTrace
for various type mismatches.Side-note: We can't normalize the
TypeError
itself bc it may come from instantiated binders, so it may reference values from within the probe...r? lcnr