borrowck: clarify an E0502 label for &self.field vs mutable self - #155116
borrowck: clarify an E0502 label for &self.field vs mutable self#155116ozankenangungor wants to merge 1 commit into
Conversation
rustbot
added
S-waiting-on-review
|
r? @nnethercote rustbot has assigned @nnethercote. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
d9b5cd3 to
e5c3dfd
Compare
This comment has been minimized.
This comment has been minimized.
e5c3dfd to
6c7f43c
Compare
There was a problem hiding this comment.
When doing a PR that changes an error message and also adds a new ui test, it's very helpful for reviewers if you add the new ui test in the first commit, and then make the error message change in the second commit. That way the change in behaviour is completely clear.
|
This is a simple PR but borrowing (and the language around it) is subtle enough that I will pass this on to someone who is much more of an expert. r? @estebank |
| let old_place_desc_for_label = if msg_place.is_empty() | ||
| && place != issued_borrow.borrowed_place | ||
| && place.local == issued_borrow.borrowed_place.local | ||
| && self.local_name(place.local) == Some(kw::SelfLower) |
There was a problem hiding this comment.
This logic seem very specific for self. Wouldn't it be much nicer to do something generic, so we get better diagnostics for other locals too? The compiler code becomes hard to understand if everything is a special case, so it's nice to try to avoid that.
There are a few other cases of kw::SelfLower checks in rustc_borrowck, but on first glance they seem easier to justify.
| None | ||
| }; | ||
|
|
||
| // FIXME: supply non-"" `opt_via` when appropriate |
There was a problem hiding this comment.
This comment was added in 018784a. I don't understand what it meant then, and I don't understand what it means now. Or, it probably just means "make diagnostics better", which we don't need to keep a FIXME for. Would you mind removing this comment in this PR, now that we change code next to it, please? To prevent puzzling future coders and reviewers.
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
6 participants
Small diagnostics-only fix for #154275.
This keeps the overall E0502 wording as-is and just makes the earlier label more specific in the &self.field / later mutable self case.