Skip to content
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

[borrowck] Fix help on mutating &self in async fns #93221

Merged
merged 1 commit into from
Feb 2, 2022

Conversation

alyssaverkade
Copy link
Contributor

@alyssaverkade alyssaverkade commented Jan 23, 2022

Previously, when rustc was provided an async function that tried to
mutate through a shared reference to an implicit self (as shown in the
ui test), rustc would suggest modifying the parameter signature
to &mut + the fully qualified name of the ty (in the case of the repro
S). If a user modified their code to match the suggestion, the
compiler would not accept it.

This commit modifies the suggestion so that when rustc is provided the
ui test that is also attached in this commit, it suggests (correctly)
&mut self. We try to be careful about distinguishing between implicit
and explicit self annotations, since the latter seem to be handled
correctly already.

This is my first PR here so I'm pretty sure I probably missed something/could use better terminology. I also didn't try to make the match exhaustive since implicit self is the only real special case that I need to handle (that I'm aware of), and I'm pretty sure there's a cleaner way to do this so any advice would be greatly appreciated! (I'm also not terribly confident about how I wrote the ui tests)

here is your cc as requested @compiler-errors

This is an attempt to fix #93093

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 23, 2022
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jackh726 (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 23, 2022
Previously, when rustc was provided an async function that tried to
mutate through a shared reference to an implicit self (as shown in the
ui test), rustc would suggest modifying the parameter signature
to `&mut` + the fully qualified name of the ty (in the case of the repro
`S`). If a user modified their code to match the suggestion, the
compiler would not accept it.

This commit modifies the suggestion so that when rustc is provided the
ui test that is also attached in this commit, it suggests (correctly)
`&mut self`. We try to be careful about distinguishing between implicit
and explicit self annotations, since the latter seem to be handled
correctly already.

Fixes rust-lang#93093
@alyssaverkade
Copy link
Contributor Author

r? rust-lang/compiler

@jackh726
Copy link
Member

@alyssaverkade in the future, please be patient. Requesting a new reviewer after only a few days generally makes things harder.

@alyssaverkade
Copy link
Contributor Author

Sorry! I had no idea what the norm was and I didn't want to overload you with more things to do

Copy link
Member

@wesleywiser wesleywiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks for fixing this!

@wesleywiser
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 2, 2022

📌 Commit b885700 has been approved by wesleywiser

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 2, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 2, 2022
…askrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#92528 (Make `Fingerprint::combine_commutative` associative)
 - rust-lang#93221 ([borrowck] Fix help on mutating &self in async fns)
 - rust-lang#93542 (Prevent lifetime elision in type alias)
 - rust-lang#93546 (Validate that values in switch int terminator are unique)
 - rust-lang#93571 (better suggestion for duplicated `where` clause)
 - rust-lang#93574 (don't suggest adding `let` due to bad assignment expressions inside of `while` loop)
 - rust-lang#93590 (More let_else adoptions)
 - rust-lang#93592 (Remove unused dep from rustc_arena)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b622552 into rust-lang:master Feb 2, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect suggestion when trying to write to an immutable field in an async function
6 participants