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.

Sign up for GitHub

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

Jump to bottom

Suggestion for call on immutable binding of mutable type #83251

Merged
merged 3 commits into from
Aug 29, 2021
Merged

Suggestion for call on immutable binding of mutable type #83251

merged 3 commits into from
Aug 29, 2021

Conversation

estebank
Copy link
Contributor

When calling a method requiring a mutable self borrow on an inmutable
to a mutable borrow of the type, suggest making the binding mutable.

Fix #83241.

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 18, 2021
@estebank
Copy link
Contributor Author

r? @oli-obk

@oli-obk
Copy link
Contributor

oli-obk commented Mar 23, 2021

All of the changed tests were a case where the previous suggestion was already applicable, and now still is. While the new version fixes the situation, it always does so by introducing another level of indirection that will immediately get autodereffed.

I think this new suggestion should not apply if the expression is a borrow expression. Also we should probably have the test from the issue in the test suite, as it does cover a case that is not covered by any of the changed tests

@crlf0710 crlf0710 added A-diagnostics Area: Messages for errors, warnings, and lints S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 9, 2021
@Dylan-DPC-zz Dylan-DPC-zz added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 9, 2021
@Dylan-DPC-zz
Copy link

@estebank judging by oli's previous comment i'm assuming there are changes expected here

@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 26, 2021
@bors
Copy link
Contributor

bors commented May 11, 2021

☔ The latest upstream changes (presumably #85100) made this pull request unmergeable. Please resolve the merge conflicts.

@bstrie bstrie added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 2, 2021
@crlf0710 crlf0710 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 19, 2021
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 5, 2021
@bstrie bstrie added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 21, 2021
@JohnCSimon
Copy link
Member

Ping from triage:
@estebank build is broken and there are merge conflicts.

@estebank estebank force-pushed the issue-83241 branch 2 times, most recently from e439111 to faf5dbd Compare August 9, 2021 10:32
@estebank
Copy link
Contributor Author

estebank commented Aug 9, 2021

Addressed comments, fixed rebase and fixed structured_suggestion (we currently incorrectly suggest removal of code, which is why this PR originally used help instead of suggestion).

@oli-obk
Copy link
Contributor

oli-obk commented Aug 9, 2021

@bors r+ rollup

@camelid
Copy link
Member

camelid commented Aug 29, 2021

@oli-obk It seems that bors didn't pick up on your approval. Can you try again?

@camelid

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
@estebank
Copy link
Contributor Author

@bors r=oli-obk rollup

@bors
Copy link
Contributor

bors commented Aug 29, 2021

📌 Commit b252b38a9f071c5ddaf10fb6f0abe7ccf0c3a7d5 has been approved by oli-obk

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 29, 2021
@estebank
Copy link
Contributor Author

@bors r-

This has old suggestion format, it'll fail to merge.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 29, 2021
@estebank
Suggestion for call on immutable binding of mutable type
e4368de 
When calling a method requiring a mutable self borrow on an inmutable
to a mutable borrow of the type, suggest making the binding mutable.

Fix rust-lang#83241.
@estebank
Provide structured suggestion for removal of &mut
a6af6d6
@estebank
rebase: fix test output
5b6f4b9
@estebank
Copy link
Contributor Author

@bors r=oli-obk rollup

@bors
Copy link
Contributor

bors commented Aug 29, 2021

📌 Commit 5b6f4b9 has been approved by oli-obk

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 29, 2021
@GuillaumeGomez GuillaumeGomez mentioned this pull request Aug 29, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 29, 2021
@bors
Auto merge of rust-lang#88461 - GuillaumeGomez:rollup-khgu0eb, r=Guil…
daa4dc9 
…laumeGomez

Rollup of 13 pull requests

Successful merges:

 - rust-lang#80543 (Notify when an `I-prioritize` issue is closed or reopened)
 - rust-lang#83251 (Suggestion for call on immutable binding of mutable type)
 - rust-lang#85534 (add rustc-demangle assertion on mangled symbol)
 - rust-lang#88173 (Refactor Markdown length-limited summary implementation)
 - rust-lang#88349 (Add const and static TAIT tests)
 - rust-lang#88357 (add unsized coercion test)
 - rust-lang#88381 (Handle stack_t.ss_sp type change for DragonFlyBSD)
 - rust-lang#88387 (Remove vestigial rustfix tests.)
 - rust-lang#88396 (Bump vulnerable crates)
 - rust-lang#88407 (Fix formatting in release notes from 52a9883)
 - rust-lang#88411 (Remove `Session.if_let_suggestions`)
 - rust-lang#88417 (RELEASES.md: fix broken link)
 - rust-lang#88419 (Fix code blocks color in Ayu theme)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 22973a8 into rust-lang:master Aug 29, 2021
@rustbot rustbot added this to the 1.56.0 milestone Aug 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@oli-obk oli-obk

Labels
A-diagnostics Area: Messages for errors, warnings, and lints S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.56.0
Development

Successfully merging this pull request may close these issues.

Confusing error message suggestion when invoking mutable trait method implemented by a mutable reference