-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Check built-in operator obligation before the method's WF obligations so as to not incompletely guide inference #137811
base: master
Are you sure you want to change the base?
Conversation
Which would fix #137812. |
honestly E0284 kinda sucks lol, it's an error code for when a projection is ambiguous but when is a projection ever ambiguous without the corresponding trait goal being ambiguous? |
… so as to not incompletely guide inference
There is no functional difference between these two predicates. We used to only use E0284 for projection preds, and E0283 for trait preds. But we also use E0284 for ConstParamHasTy and other const predicates... Users don't really know the type system enough to be able to distinguish this and E0283, so let's merge them.
19695df
to
8a458c8
Compare
Some changes occurred in need_type_info.rs cc @lcnr |
cc rust-lang/trait-system-refactor-initiative#162 I would like to change the old solver to consider all builtin I am not opposed to this more localized fix but it feels kinda disappointing that it is necessary and don't have an immediate understanding of what happens here. Would need to look at it for more than 1 minute to approve this PR |
☔ The latest upstream changes (presumably #138114) made this pull request unmergeable. Please resolve the merge conflicts. |
I can give that a try. Kinda would like to land this just for the diagnostic changes, but maybe afterwards 🤔 |
Fixes #137781
See the test description. Let me know if you want a more detailed step by step explanation for why this is failing.
We could alternatively fix this by trying to make
Sized
obligation checking less incomplete in the old solver... but that seems like a much larger change.r? lcnr