-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Subtype FRU fields first in type_changing_struct_update
#98013
Conversation
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.
so to summarize:
The current approach first does subtyping + opt coercion from the base expr to TargetTy<?0..?N>
where ?0
to ?N
are fresh inference var.
The second approach first does subtyping of TargetTy<?0..?N>
to our actual target ty and then does subtyping + coercions between the inferred target ty and the base type.
The new approach can do more coercions as we know more about the target type at that point.
two nits, then r=me |
0ec16f3
to
784e28e
Compare
@bors r=lcnr |
📌 Commit 784e28e has been approved by |
Rollup of 5 pull requests Successful merges: - rust-lang#95392 (std: Stabilize feature try_reserve_2 ) - rust-lang#97798 (Hide irrelevant lines in suggestions to allow for suggestions that are far from each other to be shown) - rust-lang#97844 (Windows: No panic if function not (yet) available) - rust-lang#98013 (Subtype FRU fields first in `type_changing_struct_update`) - rust-lang#98191 (Remove the rest of unnecessary `to_string`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
So this fixes a subtle bug that
type_changing_struct_update
introduced, where it'll no longer coerce the base expr correctly. I actually think this code is easier to understand now, too.r? @lcnr since you reviewed the last one