-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Attempt to normalize FnDef
signature in InferCtxt::cmp
#100473
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Attempt to normalize FnDef
signature in InferCtxt::cmp
#100473
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
r? diagnostics (or maybe @rust-lang/types?) |
3a5a285
to
818de76
Compare
Here is a real world use case where this would have been enormously helpful: https://mobile.twitter.com/joshuayn514/status/1557913836337786880 |
I'm r+ on this, but want to make sure another pair of eyes looks at this r? rust-lang/types |
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.
nits, then r=me
818de76
to
7292a63
Compare
7292a63
to
e5602cb
Compare
Added @bors r=lcnr |
Rollup of 5 pull requests Successful merges: - rust-lang#99517 (Display raw pointer as *{mut,const} T instead of *-ptr in errors) - rust-lang#99928 (Do not leak type variables from opaque type relation) - rust-lang#100473 (Attempt to normalize `FnDef` signature in `InferCtxt::cmp`) - rust-lang#100653 (Move the cast_float_to_int fallback code to GCC) - rust-lang#100941 (Point at the string inside literal and mention if we need string inte…) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Stashes a normalization callback in
InferCtxt
so that the signature we get fromtcx.fn_sig(..).subst(..)
inInferCtxt::cmp
can be properly normalized, since we cannot expect for it to have normalized types since it comes straight from astconv.This is kind of a hack, but I will say that @jyn514 found the fact that we present unnormalized types to be very confusing in real life code, and I agree with that feeling. Though altogether I am still a bit unsure about whether this PR is worth the effort, so I'm open to alternatives and/or just closing it outright.
On the other hand, this isn't a ridiculously heavy implementation anyways -- it's less than a hundred lines of changes, and half of that is just miscellaneous cleanup.
This is stacked onto #100471 which is basically unrelated, and it can be rebased off of that when that lands or if needed.
The code:
Before:
After: