Additional span info for E0053 #35765
Conversation
src/librustc/ty/error.rs
Outdated
There was a problem hiding this comment.
The reason why there are so many test expectation changes is due to this line here. I realize that changing this here means that all affected error messages will have to be shown in the new format, and I haven't really touched the other ones aside from E0053.
There was a problem hiding this comment.
I know you just did all the work to change this, but this part of the change makes me nervous. If there are other errors that will also need "expected __, found ___" changes for the label, maybe it makes sense to do all of those at once.
What does this change look like without changing this line?
There was a problem hiding this comment.
Looking through the unit tests that changed, I think I definitely underestimated the impact changing the label would have.
For the time being it looks like we'll need to keep it as-is, until we can come up with another solution. If we removed the "expected" part, some of the labels just won't make sense or will become much more difficult to understand.
There was a problem hiding this comment.
That's totally fine (in fact, that's the whole reason why I have a separate commit removing all the "found ___" part). It does make me feel like it makes more sense to change all of them at once. Not sure if this interim state is desirable though, i.e. having the span label say "expected ___, found ____" instead of simply "expected ____".
There was a problem hiding this comment.
Yeah, changes like this in particular are what I was worried about. If we remove the found part the underline becomes very confusing.
|
Okay, I've removed the test expectation changes and the code is now using the older format of "expected ___, found ___". |
|
Great! Thanks for doing that. @bors r+ |
|
📌 Commit 7aef7e4 has been approved by |
|
I needed to re-update my UI test. re-r? @jonathandturner |
|
@bors r+ |
|
📌 Commit 31d56cb has been approved by |
…rner Additional span info for E0053 Part of rust-lang#35233. Fixes rust-lang#35212. r? @jonathandturner
| let arg_idx = impl_iter.zip(trait_iter) | ||
| .position(|(impl_arg_ty, trait_arg_ty)| { | ||
| *impl_arg_ty == found && *trait_arg_ty == expected | ||
| }).unwrap(); |
There was a problem hiding this comment.
This unwrap is probably the cause of the ICE / regression #35869.
4 participants
Part of #35233.
Fixes #35212.
r? @jonathandturner