Suggest use `.get_mut` instead of `&mut` when overloaded index type not impl `IndexMut` by xizheyin · Pull Request #144018 · rust-lang/rust

xizheyin · 2025-07-16T11:10:10Z

r? compiler

fee1-dead · 2025-07-16T12:11:06Z

You need to compare the defid with the defid of the HashMap/BTreeMap diagnostic item and hard code the special case

xizheyin

The second commit shows the testcase difference.

fee1-dead · 2025-07-17T00:59:20Z

In your first version you checked if the assignment on the type came from an overloaded index. You should check whether the overloaded index type implements IndexMut and return instead of passing the bool into the suggest_ampmut function if they don't implement IndexMut, then before returning you can suggest get_mut

@rustbot author

rustbot · 2025-07-17T00:59:35Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

xizheyin

@rustbot ready

xizheyin · 2025-07-20T10:08:52Z

+ let mut vec = vec![String::new(), String::new()];
+ let string = &vec[0];
+ string.push_str("test"); //~ ERROR cannot borrow `*string` as mutable, as it is behind a `&` reference [E0596]


I add a case that type impls IndexMut, i.e. Vec.

xizheyin · 2025-07-20T10:09:36Z

+ .type_implements_trait(
+ index_mut_trait,
+ [ty, index_ty],
+ self.infcx.param_env,
+ )
+ .must_apply_modulo_regions()


I use type_implements_trait to judge if the type impl IndexMut.

fee1-dead · 2025-07-21T16:18:32Z

#144018 (comment)

@rustbot author

xizheyin · 2025-07-21T16:32:28Z

#144018 (comment)

This comment asks me to check the type which use overloaded index but not implement IndexMut, HashMap and BTreeMap are in that category, my understanding is that this is a enhancement over hardcoded. Why we should still hard-coding HashMap and BTreeMap? 😵‍💫

fee1-dead · 2025-07-21T16:47:41Z

because there are other types that aren't IndexMutand that aren't HashMap or BTreeMap?

xizheyin · 2025-07-21T16:55:34Z

#143732 (comment)

@Kobzol suggest to consider all the types in this category? What do you think?

If we only consider a special case, we don't have to check whether it implements IndexMut. Because these two types didnt implement IndexMut, it seems to be enough to judge directly by using defid.

Kobzol · 2025-07-21T16:59:40Z

My suggestion was:

If T doesn't implement IndexMut, don't show the hint at all
If T is a Hashmap/HashSet, suggest get_mut

xizheyin · 2025-07-21T17:02:08Z

This is clear. I forgot that some types do not have get_mut.

Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>

xizheyin

@rustbot ready

This time, I considered different scenarios:

suggest to use &mut
suggest to use .get_mut
dont suggest

I also added a new testcase.

xizheyin · 2025-07-24T13:44:39Z

I spilt the testcase into two. One is marked run-rustfix, the other not.

…or BTreeMap Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>

xizheyin

@rustbot ready

xizheyin · 2025-07-27T13:02:31Z

+ // If there is a assignment, we should return the rhs
+ rhs = Some(rvalue);
+ if let mir::Rvalue::Use(mir::Operand::Copy(place)) = rvalue {
+ span = self.body.local_decls[place.local].source_info.span;
+ }


To dedup if let StatementKind..., I return the RValue if it is a Assign and pass it into suggest_get_mut_or_ref_mut.

xizheyin · 2025-07-27T13:05:04Z

+ // We only suggest use `.get_mut()` for HashMap or BTreeMap
+ // We don't try suggesting use `&mut`, so break
+ ControlFlow::Break(Some(AmpMutSugg {
+ has_sugg: true,
+ span: rhs_span,
+ suggestion: format!("{}.get_mut({}).unwrap()", map_part, key_part),
+ additional: None,
+ }))
+ } else {
+ // Type not implemented `IndexMut` but not HashMap or BTreeMap or error
+ // and we did not suggest either `.get_mut()` or `&mut`, so break
+ ControlFlow::Break(None)
+ }
+ } else {
+ // Type Impl IndexMut, we should continue to try suggesting use `&mut`
+ ControlFlow::Continue(())


In these three situations, we return different values. If we don't want to try suggesting &mut, it return Break. Or we return Continue, which indicates continue to try suggesting &mut.

…m, r=nnethercote overhaul `&mut` suggestions in borrowck errors * This refactors the logic so that it does not use fuzzy string matching for suggestions; it instead uses information directly from MIR. * If something comes from a custom `Index` impl for which the `IndexMut` trait does not apply, do not suggest adding `mut` after `&`. * Suggest `get_mut` with `unwrap` if error is fired on `BTreeMap` or `HashMap`. Supersedes rust-lang#144018 cc `@xizheyin` Closesrust-lang#143732

…m, r=nnethercote overhaul `&mut` suggestions in borrowck errors * This refactors the logic so that it does not use fuzzy string matching for suggestions; it instead uses information directly from MIR. * If something comes from a custom `Index` impl for which the `IndexMut` trait does not apply, do not suggest adding `mut` after `&`. * Suggest `get_mut` with `unwrap` if error is fired on `BTreeMap` or `HashMap`. Supersedes rust-lang#144018 cc ``@xizheyin`` Closesrust-lang#143732

…m, r=nnethercote overhaul `&mut` suggestions in borrowck errors * This refactors the logic so that it does not use fuzzy string matching for suggestions; it instead uses information directly from MIR. * If something comes from a custom `Index` impl for which the `IndexMut` trait does not apply, do not suggest adding `mut` after `&`. * Suggest `get_mut` with `unwrap` if error is fired on `BTreeMap` or `HashMap`. Supersedes rust-lang#144018 cc ```@xizheyin``` Closesrust-lang#143732

Rollup merge of #145013 - fee1-dead-contrib:push-vwvsqsqnrxqm, r=nnethercote overhaul `&mut` suggestions in borrowck errors * This refactors the logic so that it does not use fuzzy string matching for suggestions; it instead uses information directly from MIR. * If something comes from a custom `Index` impl for which the `IndexMut` trait does not apply, do not suggest adding `mut` after `&`. * Suggest `get_mut` with `unwrap` if error is fired on `BTreeMap` or `HashMap`. Supersedes #144018 cc ```@xizheyin``` Closes#143732

…hercote overhaul `&mut` suggestions in borrowck errors * This refactors the logic so that it does not use fuzzy string matching for suggestions; it instead uses information directly from MIR. * If something comes from a custom `Index` impl for which the `IndexMut` trait does not apply, do not suggest adding `mut` after `&`. * Suggest `get_mut` with `unwrap` if error is fired on `BTreeMap` or `HashMap`. Supersedes rust-lang/rust#144018 cc ```@xizheyin``` Closesrust-lang/rust#143732

rustbot assigned fee1-dead Jul 16, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 16, 2025

xizheyin changed the title ~~Suggest use .get_mut instead of &mut when occur overloaded index expression~~Suggest use .get_mut instead of &mut when occur HashMap and BtreeMapJul 16, 2025

xizheyin force-pushed the 143732 branch from 6b235e6 to 4241ce9Compare July 16, 2025 13:44

xizheyin commented Jul 16, 2025
View reviewed changes

Comment threadcompiler/rustc_borrowck/src/diagnostics/mutability_errors.rs Outdated

Kobzol mentioned this pull request Jul 16, 2025
rust suggests using IndexMut for maps, even though they don't implement the trait #143732
Closed

rustbot 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 Jul 17, 2025

xizheyin force-pushed the 143732 branch 2 times, most recently from 8d3b265 to c43a91fCompare July 20, 2025 10:07

xizheyin commented Jul 20, 2025
View reviewed changes

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 20, 2025

xizheyin force-pushed the 143732 branch from c43a91f to eb246e6Compare July 20, 2025 11:45

This comment has been minimized.
Sign in to view

xizheyin force-pushed the 143732 branch from eb246e6 to 21fda2bCompare July 20, 2025 12:44

xizheyin changed the title ~~Suggest use .get_mut instead of &mut when occur HashMap and BtreeMap~~Suggest use .get_mut instead of &mut when overloaded index type not impl IndexMutJul 20, 2025

rustbot 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 Jul 21, 2025

xizheyin force-pushed the 143732 branch from 21fda2b to 17899d6Compare July 21, 2025 17:54

Add test suggest-use-as-mut-for-map.rs
e41e112
Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>

xizheyin force-pushed the 143732 branch from 17899d6 to de1e05cCompare July 21, 2025 18:21

xizheyin commented Jul 21, 2025
View reviewed changes

Comment threadtests/ui/borrowck/suggest-use-as-mut-for-map.stderr Outdated

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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, 2025

estebank reviewed Jul 23, 2025
View reviewed changes

Comment threadtests/ui/borrowck/suggest-use-as-mut-for-map.rs

xizheyin force-pushed the 143732 branch from de1e05c to d4bdd10Compare July 24, 2025 11:12

fee1-dead requested changes Jul 25, 2025
View reviewed changes

Comment threadcompiler/rustc_borrowck/src/diagnostics/mutability_errors.rs Outdated
Comment threadcompiler/rustc_borrowck/src/diagnostics/mutability_errors.rs Outdated

rustbot 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 Jul 25, 2025

Suggest use .get_mut instead of &mut when source type is HashMap …
6cdc4e9
…or BTreeMap Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>

xizheyin force-pushed the 143732 branch from d4bdd10 to 6cdc4e9Compare July 27, 2025 12:59

xizheyin commented Jul 27, 2025
View reviewed changes

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 27, 2025

fee1-dead mentioned this pull request Aug 6, 2025
overhaul &mut suggestions in borrowck errors #145013
Merged

xizheyin closed this Aug 6, 2025

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 6, 2025

Uh oh!

Uh oh!

Conversation

xizheyin commented Jul 16, 2025• edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fee1-dead commented Jul 16, 2025

Uh oh!

xizheyin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fee1-dead commented Jul 17, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 17, 2025

Uh oh!

xizheyin left a comment

Choose a reason for hiding this comment

Uh oh!

xizheyinJul 20, 2025

Choose a reason for hiding this comment

Uh oh!

xizheyinJul 20, 2025

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

fee1-dead commented Jul 21, 2025

Uh oh!

xizheyin commented Jul 21, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fee1-dead commented Jul 21, 2025

Uh oh!

xizheyin commented Jul 21, 2025

Uh oh!

Kobzol commented Jul 21, 2025

Uh oh!

xizheyin commented Jul 21, 2025

Uh oh!

xizheyin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

xizheyin commented Jul 24, 2025

Uh oh!

Uh oh!

Uh oh!

xizheyin left a comment

Choose a reason for hiding this comment

Uh oh!

xizheyinJul 27, 2025

Choose a reason for hiding this comment

Uh oh!

xizheyinJul 27, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

xizheyin commented Jul 16, 2025•
edited by rustbot
Loading

fee1-dead commented Jul 17, 2025•
edited
Loading

xizheyin commented Jul 21, 2025•
edited
Loading