From efb2ca086ad3ffbd7b350bfd8e96ab2eb8d692d8 Mon Sep 17 00:00:00 2001 From: GTimothy <22472919+GTimothy@users.noreply.github.com> Date: Tue, 16 Dec 2025 09:48:50 +0100 Subject: [PATCH 1/9] Fix 'assign to data in an index of' collection suggestions splits the large triple suggestion into three sets them to MaybeIncorrect automatically determines the required borrowing to use. --- .../src/diagnostics/mutability_errors.rs | 137 ++++++++++-------- 1 file changed, 80 insertions(+), 57 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 96090e85e5622..4697d05e758f2 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -668,7 +668,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { err: &'a mut Diag<'infcx>, ty: Ty<'tcx>, suggested: bool, + infcx: &'a rustc_infer::infer::InferCtxt<'tcx>, } + impl<'a, 'infcx, 'tcx> Visitor<'tcx> for SuggestIndexOperatorAlternativeVisitor<'a, 'infcx, 'tcx> { fn visit_stmt(&mut self, stmt: &'tcx hir::Stmt<'tcx>) { hir::intravisit::walk_stmt(self, stmt); @@ -679,81 +681,101 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { return; } }; + + /// Taken straight from https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/fn.peel_hir_ty_refs.html + /// Adapted to mid using https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.Ty.html#method.peel_refs + /// Simplified to counting only + /// Peels off all references on the type. Returns the number of references + /// removed. + fn count_ty_refs<'tcx>(ty: Ty<'tcx>) -> usize { + let mut count = 0; + let mut ty = ty; + while let ty::Ref(_, inner_ty, _) = ty.kind() { + ty = *inner_ty; + count += 1; + } + count + } if let hir::ExprKind::Assign(place, rv, _sp) = expr.kind && let hir::ExprKind::Index(val, index, _) = place.kind && (expr.span == self.assign_span || place.span == self.assign_span) { - // val[index] = rv; - // ---------- place - self.err.multipart_suggestions( + let ref_depth_difference: usize; + let _index_is_copy_clone: bool; + + if let Some(index_ty) = + self.infcx.tcx.typeck(val.hir_id.owner.def_id).expr_ty_opt(index) + { + // we know ty is a map, with a key type at walk distance 2. + let key_type = self.ty.walk().nth(1).unwrap().expect_ty(); + let key_ref_depth = count_ty_refs(key_type); + + let index_ref_depth = count_ty_refs(index_ty); + ref_depth_difference = index_ref_depth - key_ref_depth; //index should + //be deeper than key + } else { + // no type ? + return; + }; + + // remove the exessive referencing if necessary, but get_mut requires a ref + let (prefix, gm_prefix) = match ref_depth_difference { + 0 => (String::new(), String::from("&")), + n => ("*".repeat(n), "*".repeat(n - 1)), + }; + + self.err.multipart_suggestion( + format!("use `.insert()` to insert a value into a `{}`", self.ty), + vec![ + // val.insert(index, rv); + ( + val.span.shrink_to_hi().with_hi(index.span.lo()), + format!(".insert({prefix}"), + ), + (index.span.shrink_to_hi().with_hi(rv.span.lo()), ", ".to_string()), + (rv.span.shrink_to_hi(), ")".to_string()), + ], + Applicability::MaybeIncorrect, + ); + self.err.multipart_suggestion( format!( - "use `.insert()` to insert a value into a `{}`, `.get_mut()` \ - to modify it, or the entry API for more flexibility", + "use `.get_mut()` to modify an existing key in a `{}`", self.ty, ), vec![ - vec![ - // val.insert(index, rv); - ( - val.span.shrink_to_hi().with_hi(index.span.lo()), - ".insert(".to_string(), - ), - ( - index.span.shrink_to_hi().with_hi(rv.span.lo()), - ", ".to_string(), - ), - (rv.span.shrink_to_hi(), ")".to_string()), - ], - vec![ - // if let Some(v) = val.get_mut(index) { *v = rv; } - (val.span.shrink_to_lo(), "if let Some(val) = ".to_string()), - ( - val.span.shrink_to_hi().with_hi(index.span.lo()), - ".get_mut(".to_string(), - ), - ( - index.span.shrink_to_hi().with_hi(place.span.hi()), - ") { *val".to_string(), - ), - (rv.span.shrink_to_hi(), "; }".to_string()), - ], - vec![ - // let x = val.entry(index).or_insert(rv); - (val.span.shrink_to_lo(), "let val = ".to_string()), - ( - val.span.shrink_to_hi().with_hi(index.span.lo()), - ".entry(".to_string(), - ), - ( - index.span.shrink_to_hi().with_hi(rv.span.lo()), - ").or_insert(".to_string(), - ), - (rv.span.shrink_to_hi(), ")".to_string()), - ], + // if let Some(v) = val.get_mut(index) { *v = rv; } + (val.span.shrink_to_lo(), "if let Some(val) = ".to_string()), + ( + val.span.shrink_to_hi().with_hi(index.span.lo()), + format!(".get_mut({gm_prefix}"), + ), + ( + index.span.shrink_to_hi().with_hi(place.span.hi()), + ") { *val".to_string(), + ), + (rv.span.shrink_to_hi(), "; }".to_string()), ], - Applicability::MachineApplicable, + Applicability::MaybeIncorrect, ); - self.suggested = true; - } else if let hir::ExprKind::MethodCall(_path, receiver, _, sp) = expr.kind - && let hir::ExprKind::Index(val, index, _) = receiver.kind - && receiver.span == self.assign_span - { - // val[index].path(args..); self.err.multipart_suggestion( - format!("to modify a `{}` use `.get_mut()`", self.ty), + format!( + "use the entry API to modify a `{}` for more flexibility", + self.ty + ), vec![ - (val.span.shrink_to_lo(), "if let Some(val) = ".to_string()), + // let x = val.entry(index).insert_entry(rv); + (val.span.shrink_to_lo(), "let val = ".to_string()), ( val.span.shrink_to_hi().with_hi(index.span.lo()), - ".get_mut(".to_string(), + format!(".entry({prefix}"), ), ( - index.span.shrink_to_hi().with_hi(receiver.span.hi()), - ") { val".to_string(), + index.span.shrink_to_hi().with_hi(rv.span.lo()), + ").insert_entry(".to_string(), ), - (sp.shrink_to_hi(), "; }".to_string()), + (rv.span.shrink_to_hi(), ")".to_string()), ], - Applicability::MachineApplicable, + Applicability::MaybeIncorrect, ); self.suggested = true; } @@ -768,6 +790,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { err, ty, suggested: false, + infcx: self.infcx, }; v.visit_body(&body); if !v.suggested { From 9b0864d4aec3f4b6b1973cb79130c5ee78bbca4a Mon Sep 17 00:00:00 2001 From: GTimothy <22472919+GTimothy@users.noreply.github.com> Date: Sat, 7 Feb 2026 16:39:29 +0100 Subject: [PATCH 2/9] restore missing case, method call on value of map --- .../src/diagnostics/mutability_errors.rs | 88 ++++++++++++++----- 1 file changed, 67 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 4697d05e758f2..fa54d1d81729e 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -696,32 +696,35 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { } count } + + // we know ty is a map, with a key type at walk distance 2. + let key_type = self.ty.walk().nth(1).unwrap().expect_ty(); + let key_ref_depth = count_ty_refs(key_type); + if let hir::ExprKind::Assign(place, rv, _sp) = expr.kind && let hir::ExprKind::Index(val, index, _) = place.kind && (expr.span == self.assign_span || place.span == self.assign_span) { - let ref_depth_difference: usize; - let _index_is_copy_clone: bool; - - if let Some(index_ty) = - self.infcx.tcx.typeck(val.hir_id.owner.def_id).expr_ty_opt(index) - { - // we know ty is a map, with a key type at walk distance 2. - let key_type = self.ty.walk().nth(1).unwrap().expect_ty(); - let key_ref_depth = count_ty_refs(key_type); - - let index_ref_depth = count_ty_refs(index_ty); - ref_depth_difference = index_ref_depth - key_ref_depth; //index should - //be deeper than key - } else { - // no type ? - return; - }; + let (prefix, gm_prefix) = { + let ref_depth_difference: usize; - // remove the exessive referencing if necessary, but get_mut requires a ref - let (prefix, gm_prefix) = match ref_depth_difference { - 0 => (String::new(), String::from("&")), - n => ("*".repeat(n), "*".repeat(n - 1)), + if let Some(index_ty) = + self.infcx.tcx.typeck(val.hir_id.owner.def_id).expr_ty_opt(index) + { + let index_ref_depth = count_ty_refs(index_ty); + ref_depth_difference = index_ref_depth - key_ref_depth; //index should + //be deeper than key + } else { + // no type ? + // FIXME: unsure how to handle this case + return; + }; + + // remove the excessive referencing if necessary, but get_mut requires a ref + match ref_depth_difference { + 0 => (String::new(), String::from("&")), + n => ("*".repeat(n), "*".repeat(n - 1)), + } }; self.err.multipart_suggestion( @@ -778,6 +781,49 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { Applicability::MaybeIncorrect, ); self.suggested = true; + } else if let hir::ExprKind::MethodCall(_path, receiver, _, sp) = expr.kind + && let hir::ExprKind::Index(val, index, _) = receiver.kind + && receiver.span == self.assign_span + { + let gm_prefix = { + let ref_depth_difference: usize; + + if let Some(index_ty) = + self.infcx.tcx.typeck(val.hir_id.owner.def_id).expr_ty_opt(index) + { + let index_ref_depth = count_ty_refs(index_ty); + ref_depth_difference = index_ref_depth - key_ref_depth; //index should + //be deeper than key + } else { + // no type ? + // FIXME: unsure how to handle this case + return; + }; + + // remove the excessive referencing if necessary, but get_mut requires a ref + match ref_depth_difference { + 0 => String::from("&"), + n => "*".repeat(n - 1), + } + }; + // val[index].path(args..); + self.err.multipart_suggestion( + format!("to modify a `{}` use `.get_mut()`", self.ty), + vec![ + (val.span.shrink_to_lo(), "if let Some(val) = ".to_string()), + ( + val.span.shrink_to_hi().with_hi(index.span.lo()), + format!(".get_mut({gm_prefix}"), + ), + ( + index.span.shrink_to_hi().with_hi(receiver.span.hi()), + ") { val".to_string(), + ), + (sp.shrink_to_hi(), "; }".to_string()), + ], + Applicability::MachineApplicable, + ); + self.suggested = true; } } } From 5d997efa69d50f2c4b65416d585efa38aba0e832 Mon Sep 17 00:00:00 2001 From: GTimothy <22472919+GTimothy@users.noreply.github.com> Date: Sat, 14 Feb 2026 10:21:49 +0100 Subject: [PATCH 3/9] rebless the ui tests --- tests/ui/borrowck/index-mut-help.stderr | 12 ++++++++---- .../collections/btreemap/btreemap-index-mut-2.stderr | 10 +++++++--- .../collections/btreemap/btreemap-index-mut.stderr | 10 +++++++--- .../ui/collections/hashmap/hashmap-index-mut.stderr | 10 +++++++--- 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/tests/ui/borrowck/index-mut-help.stderr b/tests/ui/borrowck/index-mut-help.stderr index 6c3bd0df20b20..b899b8360ac3f 100644 --- a/tests/ui/borrowck/index-mut-help.stderr +++ b/tests/ui/borrowck/index-mut-help.stderr @@ -8,7 +8,7 @@ LL | map["peter"].clear(); help: to modify a `HashMap<&str, String>` use `.get_mut()` | LL - map["peter"].clear(); -LL + if let Some(val) = map.get_mut("peter") { val.clear(); }; +LL + if let Some(val) = map.get_mut(&"peter") { val.clear(); }; | error[E0594]: cannot assign to data in an index of `HashMap<&str, String>` @@ -18,16 +18,20 @@ LL | map["peter"] = "0".to_string(); | ^^^^^^^^^^^^ cannot assign | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<&str, String>` -help: use `.insert()` to insert a value into a `HashMap<&str, String>`, `.get_mut()` to modify it, or the entry API for more flexibility +help: use `.insert()` to insert a value into a `HashMap<&str, String>` | LL - map["peter"] = "0".to_string(); LL + map.insert("peter", "0".to_string()); | +help: use `.get_mut()` to modify an existing key in a `HashMap<&str, String>` + | LL - map["peter"] = "0".to_string(); -LL + if let Some(val) = map.get_mut("peter") { *val = "0".to_string(); }; +LL + if let Some(val) = map.get_mut(&"peter") { *val = "0".to_string(); }; + | +help: use the entry API to modify a `HashMap<&str, String>` for more flexibility | LL - map["peter"] = "0".to_string(); -LL + let val = map.entry("peter").or_insert("0".to_string()); +LL + let val = map.entry("peter").insert_entry("0".to_string()); | error[E0596]: cannot borrow data in an index of `HashMap<&str, String>` as mutable diff --git a/tests/ui/collections/btreemap/btreemap-index-mut-2.stderr b/tests/ui/collections/btreemap/btreemap-index-mut-2.stderr index 74a8aaf8aee9a..696afd8787b66 100644 --- a/tests/ui/collections/btreemap/btreemap-index-mut-2.stderr +++ b/tests/ui/collections/btreemap/btreemap-index-mut-2.stderr @@ -5,16 +5,20 @@ LL | map[&0] = 1; | ^^^^^^^^^^^ cannot assign | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `BTreeMap` -help: use `.insert()` to insert a value into a `BTreeMap`, `.get_mut()` to modify it, or the entry API for more flexibility +help: use `.insert()` to insert a value into a `BTreeMap` | LL - map[&0] = 1; -LL + map.insert(&0, 1); +LL + map.insert(*&0, 1); + | +help: use `.get_mut()` to modify an existing key in a `BTreeMap` | LL - map[&0] = 1; LL + if let Some(val) = map.get_mut(&0) { *val = 1; }; | +help: use the entry API to modify a `BTreeMap` for more flexibility + | LL - map[&0] = 1; -LL + let val = map.entry(&0).or_insert(1); +LL + let val = map.entry(*&0).insert_entry(1); | error: aborting due to 1 previous error diff --git a/tests/ui/collections/btreemap/btreemap-index-mut.stderr b/tests/ui/collections/btreemap/btreemap-index-mut.stderr index e8850ed2a1742..f24de4cb8a5e7 100644 --- a/tests/ui/collections/btreemap/btreemap-index-mut.stderr +++ b/tests/ui/collections/btreemap/btreemap-index-mut.stderr @@ -5,16 +5,20 @@ LL | map[&0] = 1; | ^^^^^^^^^^^ cannot assign | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `BTreeMap` -help: use `.insert()` to insert a value into a `BTreeMap`, `.get_mut()` to modify it, or the entry API for more flexibility +help: use `.insert()` to insert a value into a `BTreeMap` | LL - map[&0] = 1; -LL + map.insert(&0, 1); +LL + map.insert(*&0, 1); + | +help: use `.get_mut()` to modify an existing key in a `BTreeMap` | LL - map[&0] = 1; LL + if let Some(val) = map.get_mut(&0) { *val = 1; }; | +help: use the entry API to modify a `BTreeMap` for more flexibility + | LL - map[&0] = 1; -LL + let val = map.entry(&0).or_insert(1); +LL + let val = map.entry(*&0).insert_entry(1); | error: aborting due to 1 previous error diff --git a/tests/ui/collections/hashmap/hashmap-index-mut.stderr b/tests/ui/collections/hashmap/hashmap-index-mut.stderr index e8b22350c59da..e6fdf312bd3ae 100644 --- a/tests/ui/collections/hashmap/hashmap-index-mut.stderr +++ b/tests/ui/collections/hashmap/hashmap-index-mut.stderr @@ -5,16 +5,20 @@ LL | map[&0] = 1; | ^^^^^^^^^^^ cannot assign | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap` -help: use `.insert()` to insert a value into a `HashMap`, `.get_mut()` to modify it, or the entry API for more flexibility +help: use `.insert()` to insert a value into a `HashMap` | LL - map[&0] = 1; -LL + map.insert(&0, 1); +LL + map.insert(*&0, 1); + | +help: use `.get_mut()` to modify an existing key in a `HashMap` | LL - map[&0] = 1; LL + if let Some(val) = map.get_mut(&0) { *val = 1; }; | +help: use the entry API to modify a `HashMap` for more flexibility + | LL - map[&0] = 1; -LL + let val = map.entry(&0).or_insert(1); +LL + let val = map.entry(*&0).insert_entry(1); | error: aborting due to 1 previous error From 193b59cb144405c2cb57a386a29ea7c887e74fd5 Mon Sep 17 00:00:00 2001 From: GTimothy <22472919+GTimothy@users.noreply.github.com> Date: Mon, 23 Feb 2026 17:13:51 +0100 Subject: [PATCH 4/9] second version - properly propose methods depending of the type relationship between the index used and the map key. - use whether key type is copy/clone to orient propositions. --- .../src/diagnostics/mutability_errors.rs | 178 +++++++++--------- tests/ui/borrowck/index-mut-help.stderr | 14 +- .../btreemap/btreemap-index-mut-2.stderr | 10 +- .../btreemap/btreemap-index-mut.stderr | 10 +- .../hashmap/hashmap-index-mut.stderr | 10 +- 5 files changed, 105 insertions(+), 117 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index fa54d1d81729e..d119d81b3a43c 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -682,64 +682,100 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { } }; - /// Taken straight from https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/fn.peel_hir_ty_refs.html - /// Adapted to mid using https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.Ty.html#method.peel_refs - /// Simplified to counting only - /// Peels off all references on the type. Returns the number of references - /// removed. - fn count_ty_refs<'tcx>(ty: Ty<'tcx>) -> usize { - let mut count = 0; - let mut ty = ty; - while let ty::Ref(_, inner_ty, _) = ty.kind() { - ty = *inner_ty; - count += 1; + // Lets see: + // because of TypeChecking and indexing, we know: index is &Q + // with K: Eq + Hash + Borrow, + // with Q: Eq + Hash + ?Sized,index is a &Q with K:Borrow, + // + // + // + // there is no other constraint on the types, therefore we need to look at the + // constraints of the suggestions: + // pub fn get_mut(&mut self, k: &Q) -> Option<&mut V> + // where + // K: Borrow, + // Q: Hash + Eq + ?Sized, + // + // pub fn insert(&mut self, k: K, v: V) -> Option + // pub fn entry(&mut self, key: K) -> Entry<'_, K, V> + // + // But lets note that there could be also, if imported from hashbrown: + // pub fn entry_ref<'a, 'b, Q>( + // &'a mut self, + // key: &'b Q, + // ) -> EntryRef<'a, 'b, K, Q, V, S, A> + // where + // Q: Hash + Equivalent + ?Sized, + + /// testing if index is &K: + fn index_is_borrowed_key<'tcx>(index: Ty<'tcx>, key: Ty<'tcx>) -> bool { + if let ty::Ref(_, inner_ty, _) = index.kind() { + *inner_ty == key + } else { + false } - count + } + /// checking if the key is copy clone + fn key_is_copyclone<'tcx>(key: Ty<'tcx>) -> bool { + key.is_trivially_pure_clone_copy() } // we know ty is a map, with a key type at walk distance 2. let key_type = self.ty.walk().nth(1).unwrap().expect_ty(); - let key_ref_depth = count_ty_refs(key_type); if let hir::ExprKind::Assign(place, rv, _sp) = expr.kind && let hir::ExprKind::Index(val, index, _) = place.kind && (expr.span == self.assign_span || place.span == self.assign_span) { - let (prefix, gm_prefix) = { - let ref_depth_difference: usize; - - if let Some(index_ty) = - self.infcx.tcx.typeck(val.hir_id.owner.def_id).expr_ty_opt(index) - { - let index_ref_depth = count_ty_refs(index_ty); - ref_depth_difference = index_ref_depth - key_ref_depth; //index should - //be deeper than key - } else { - // no type ? - // FIXME: unsure how to handle this case - return; - }; - - // remove the excessive referencing if necessary, but get_mut requires a ref - match ref_depth_difference { - 0 => (String::new(), String::from("&")), - n => ("*".repeat(n), "*".repeat(n - 1)), - } - }; - - self.err.multipart_suggestion( - format!("use `.insert()` to insert a value into a `{}`", self.ty), - vec![ - // val.insert(index, rv); - ( - val.span.shrink_to_hi().with_hi(index.span.lo()), - format!(".insert({prefix}"), + let index_ty = + self.infcx.tcx.typeck(val.hir_id.owner.def_id).expr_ty(index); + // only suggest `insert` and `entry` if K is copy/clone because of the signature. + if index_is_borrowed_key(index_ty, key_type) && key_is_copyclone(key_type) { + self.err.multipart_suggestion( + format!("use `.insert()` to insert a value into a `{}`", self.ty), + vec![ + // val.insert(index, rv); + ( + val.span + .shrink_to_hi() + .with_hi(index.span.lo() + BytePos(1)), //remove the + //leading & + format!(".insert("), + ), + ( + index.span.shrink_to_hi().with_hi(rv.span.lo()), + ", ".to_string(), + ), + (rv.span.shrink_to_hi(), ")".to_string()), + ], + Applicability::MaybeIncorrect, + ); + self.err.multipart_suggestion( + format!( + "use the entry API to modify a `{}` for more flexibility", + self.ty ), - (index.span.shrink_to_hi().with_hi(rv.span.lo()), ", ".to_string()), - (rv.span.shrink_to_hi(), ")".to_string()), - ], - Applicability::MaybeIncorrect, - ); + vec![ + // let x = val.entry(index).insert_entry(rv); + (val.span.shrink_to_lo(), "let val = ".to_string()), + ( + val.span + .shrink_to_hi() + .with_hi(index.span.lo() + BytePos(1)), //remove the + //leading & + format!(".entry("), + ), + ( + index.span.shrink_to_hi().with_hi(rv.span.lo()), + ").insert_entry(".to_string(), + ), + (rv.span.shrink_to_hi(), ")".to_string()), + ], + Applicability::MaybeIncorrect, + ); + } + // in all cases, suggest get_mut because K:Borrow or Q:Borrow as a + // requirement of indexing. self.err.multipart_suggestion( format!( "use `.get_mut()` to modify an existing key in a `{}`", @@ -750,7 +786,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { (val.span.shrink_to_lo(), "if let Some(val) = ".to_string()), ( val.span.shrink_to_hi().with_hi(index.span.lo()), - format!(".get_mut({gm_prefix}"), + ".get_mut(".to_string(), ), ( index.span.shrink_to_hi().with_hi(place.span.hi()), @@ -760,52 +796,14 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { ], Applicability::MaybeIncorrect, ); - self.err.multipart_suggestion( - format!( - "use the entry API to modify a `{}` for more flexibility", - self.ty - ), - vec![ - // let x = val.entry(index).insert_entry(rv); - (val.span.shrink_to_lo(), "let val = ".to_string()), - ( - val.span.shrink_to_hi().with_hi(index.span.lo()), - format!(".entry({prefix}"), - ), - ( - index.span.shrink_to_hi().with_hi(rv.span.lo()), - ").insert_entry(".to_string(), - ), - (rv.span.shrink_to_hi(), ")".to_string()), - ], - Applicability::MaybeIncorrect, - ); + //FIXME: in the future, include the ref_entry suggestion here if it is added + //to std. + self.suggested = true; } else if let hir::ExprKind::MethodCall(_path, receiver, _, sp) = expr.kind && let hir::ExprKind::Index(val, index, _) = receiver.kind && receiver.span == self.assign_span { - let gm_prefix = { - let ref_depth_difference: usize; - - if let Some(index_ty) = - self.infcx.tcx.typeck(val.hir_id.owner.def_id).expr_ty_opt(index) - { - let index_ref_depth = count_ty_refs(index_ty); - ref_depth_difference = index_ref_depth - key_ref_depth; //index should - //be deeper than key - } else { - // no type ? - // FIXME: unsure how to handle this case - return; - }; - - // remove the excessive referencing if necessary, but get_mut requires a ref - match ref_depth_difference { - 0 => String::from("&"), - n => "*".repeat(n - 1), - } - }; // val[index].path(args..); self.err.multipart_suggestion( format!("to modify a `{}` use `.get_mut()`", self.ty), @@ -813,7 +811,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { (val.span.shrink_to_lo(), "if let Some(val) = ".to_string()), ( val.span.shrink_to_hi().with_hi(index.span.lo()), - format!(".get_mut({gm_prefix}"), + format!(".get_mut("), ), ( index.span.shrink_to_hi().with_hi(receiver.span.hi()), diff --git a/tests/ui/borrowck/index-mut-help.stderr b/tests/ui/borrowck/index-mut-help.stderr index b899b8360ac3f..912516d287ee8 100644 --- a/tests/ui/borrowck/index-mut-help.stderr +++ b/tests/ui/borrowck/index-mut-help.stderr @@ -8,7 +8,7 @@ LL | map["peter"].clear(); help: to modify a `HashMap<&str, String>` use `.get_mut()` | LL - map["peter"].clear(); -LL + if let Some(val) = map.get_mut(&"peter") { val.clear(); }; +LL + if let Some(val) = map.get_mut("peter") { val.clear(); }; | error[E0594]: cannot assign to data in an index of `HashMap<&str, String>` @@ -18,20 +18,10 @@ LL | map["peter"] = "0".to_string(); | ^^^^^^^^^^^^ cannot assign | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<&str, String>` -help: use `.insert()` to insert a value into a `HashMap<&str, String>` - | -LL - map["peter"] = "0".to_string(); -LL + map.insert("peter", "0".to_string()); - | help: use `.get_mut()` to modify an existing key in a `HashMap<&str, String>` | LL - map["peter"] = "0".to_string(); -LL + if let Some(val) = map.get_mut(&"peter") { *val = "0".to_string(); }; - | -help: use the entry API to modify a `HashMap<&str, String>` for more flexibility - | -LL - map["peter"] = "0".to_string(); -LL + let val = map.entry("peter").insert_entry("0".to_string()); +LL + if let Some(val) = map.get_mut("peter") { *val = "0".to_string(); }; | error[E0596]: cannot borrow data in an index of `HashMap<&str, String>` as mutable diff --git a/tests/ui/collections/btreemap/btreemap-index-mut-2.stderr b/tests/ui/collections/btreemap/btreemap-index-mut-2.stderr index 696afd8787b66..bb82ef43ff112 100644 --- a/tests/ui/collections/btreemap/btreemap-index-mut-2.stderr +++ b/tests/ui/collections/btreemap/btreemap-index-mut-2.stderr @@ -8,17 +8,17 @@ LL | map[&0] = 1; help: use `.insert()` to insert a value into a `BTreeMap` | LL - map[&0] = 1; -LL + map.insert(*&0, 1); +LL + map.insert(0, 1); | -help: use `.get_mut()` to modify an existing key in a `BTreeMap` +help: use the entry API to modify a `BTreeMap` for more flexibility | LL - map[&0] = 1; -LL + if let Some(val) = map.get_mut(&0) { *val = 1; }; +LL + let val = map.entry(0).insert_entry(1); | -help: use the entry API to modify a `BTreeMap` for more flexibility +help: use `.get_mut()` to modify an existing key in a `BTreeMap` | LL - map[&0] = 1; -LL + let val = map.entry(*&0).insert_entry(1); +LL + if let Some(val) = map.get_mut(&0) { *val = 1; }; | error: aborting due to 1 previous error diff --git a/tests/ui/collections/btreemap/btreemap-index-mut.stderr b/tests/ui/collections/btreemap/btreemap-index-mut.stderr index f24de4cb8a5e7..e09ede34db563 100644 --- a/tests/ui/collections/btreemap/btreemap-index-mut.stderr +++ b/tests/ui/collections/btreemap/btreemap-index-mut.stderr @@ -8,17 +8,17 @@ LL | map[&0] = 1; help: use `.insert()` to insert a value into a `BTreeMap` | LL - map[&0] = 1; -LL + map.insert(*&0, 1); +LL + map.insert(0, 1); | -help: use `.get_mut()` to modify an existing key in a `BTreeMap` +help: use the entry API to modify a `BTreeMap` for more flexibility | LL - map[&0] = 1; -LL + if let Some(val) = map.get_mut(&0) { *val = 1; }; +LL + let val = map.entry(0).insert_entry(1); | -help: use the entry API to modify a `BTreeMap` for more flexibility +help: use `.get_mut()` to modify an existing key in a `BTreeMap` | LL - map[&0] = 1; -LL + let val = map.entry(*&0).insert_entry(1); +LL + if let Some(val) = map.get_mut(&0) { *val = 1; }; | error: aborting due to 1 previous error diff --git a/tests/ui/collections/hashmap/hashmap-index-mut.stderr b/tests/ui/collections/hashmap/hashmap-index-mut.stderr index e6fdf312bd3ae..bb291367dd438 100644 --- a/tests/ui/collections/hashmap/hashmap-index-mut.stderr +++ b/tests/ui/collections/hashmap/hashmap-index-mut.stderr @@ -8,17 +8,17 @@ LL | map[&0] = 1; help: use `.insert()` to insert a value into a `HashMap` | LL - map[&0] = 1; -LL + map.insert(*&0, 1); +LL + map.insert(0, 1); | -help: use `.get_mut()` to modify an existing key in a `HashMap` +help: use the entry API to modify a `HashMap` for more flexibility | LL - map[&0] = 1; -LL + if let Some(val) = map.get_mut(&0) { *val = 1; }; +LL + let val = map.entry(0).insert_entry(1); | -help: use the entry API to modify a `HashMap` for more flexibility +help: use `.get_mut()` to modify an existing key in a `HashMap` | LL - map[&0] = 1; -LL + let val = map.entry(*&0).insert_entry(1); +LL + if let Some(val) = map.get_mut(&0) { *val = 1; }; | error: aborting due to 1 previous error From 2e221027cc56832dd0f52e1ef769641c1451b1f9 Mon Sep 17 00:00:00 2001 From: GTimothy <22472919+GTimothy@users.noreply.github.com> Date: Tue, 24 Feb 2026 12:33:51 +0100 Subject: [PATCH 5/9] fix: the same borrowed type (ignoring lifetime differences) for index and key is ok too for `insert` and `entry` --- .../src/diagnostics/mutability_errors.rs | 29 ++++++++++++++----- tests/ui/borrowck/index-mut-help.stderr | 10 +++++++ 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index d119d81b3a43c..421b5cb8f3ed3 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -707,6 +707,20 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { // where // Q: Hash + Equivalent + ?Sized, + /// testing if index AND key are &Q. We cannot simply test equality because the + /// lifetimes differ. + fn index_and_key_are_same_borrowed_type<'tcx>( + index: Ty<'tcx>, + key: Ty<'tcx>, + ) -> bool { + if let (ty::Ref(_, index_inner_ty, _), ty::Ref(_, key_inner_ty, _)) = + (index.kind(), key.kind()) + { + *index_inner_ty == *key_inner_ty + } else { + false + } + } /// testing if index is &K: fn index_is_borrowed_key<'tcx>(index: Ty<'tcx>, key: Ty<'tcx>) -> bool { if let ty::Ref(_, inner_ty, _) = index.kind() { @@ -730,15 +744,18 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { let index_ty = self.infcx.tcx.typeck(val.hir_id.owner.def_id).expr_ty(index); // only suggest `insert` and `entry` if K is copy/clone because of the signature. - if index_is_borrowed_key(index_ty, key_type) && key_is_copyclone(key_type) { + let index_is_borrowed_key = index_is_borrowed_key(index_ty, key_type); + if (index_is_borrowed_key + || index_and_key_are_same_borrowed_type(index_ty, key_type)) + && key_is_copyclone(key_type) + { + let offset = BytePos(index_is_borrowed_key as u32); self.err.multipart_suggestion( format!("use `.insert()` to insert a value into a `{}`", self.ty), vec![ // val.insert(index, rv); ( - val.span - .shrink_to_hi() - .with_hi(index.span.lo() + BytePos(1)), //remove the + val.span.shrink_to_hi().with_hi(index.span.lo() + offset), //remove the //leading & format!(".insert("), ), @@ -759,9 +776,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { // let x = val.entry(index).insert_entry(rv); (val.span.shrink_to_lo(), "let val = ".to_string()), ( - val.span - .shrink_to_hi() - .with_hi(index.span.lo() + BytePos(1)), //remove the + val.span.shrink_to_hi().with_hi(index.span.lo() + offset), //remove the //leading & format!(".entry("), ), diff --git a/tests/ui/borrowck/index-mut-help.stderr b/tests/ui/borrowck/index-mut-help.stderr index 912516d287ee8..fc8d02b0f0b43 100644 --- a/tests/ui/borrowck/index-mut-help.stderr +++ b/tests/ui/borrowck/index-mut-help.stderr @@ -18,6 +18,16 @@ LL | map["peter"] = "0".to_string(); | ^^^^^^^^^^^^ cannot assign | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<&str, String>` +help: use `.insert()` to insert a value into a `HashMap<&str, String>` + | +LL - map["peter"] = "0".to_string(); +LL + map.insert("peter", "0".to_string()); + | +help: use the entry API to modify a `HashMap<&str, String>` for more flexibility + | +LL - map["peter"] = "0".to_string(); +LL + let val = map.entry("peter").insert_entry("0".to_string()); + | help: use `.get_mut()` to modify an existing key in a `HashMap<&str, String>` | LL - map["peter"] = "0".to_string(); From 7b1ebf16b2ad959545333d7efdd6e592180a3fbd Mon Sep 17 00:00:00 2001 From: GTimothy <22472919+GTimothy@users.noreply.github.com> Date: Tue, 3 Mar 2026 19:33:04 +0100 Subject: [PATCH 6/9] remove unnecessary format! calls in favour of .to_string calls --- .../rustc_borrowck/src/diagnostics/mutability_errors.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 421b5cb8f3ed3..2518343119c77 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -757,7 +757,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { ( val.span.shrink_to_hi().with_hi(index.span.lo() + offset), //remove the //leading & - format!(".insert("), + ".insert(".to_string(), ), ( index.span.shrink_to_hi().with_hi(rv.span.lo()), @@ -778,7 +778,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { ( val.span.shrink_to_hi().with_hi(index.span.lo() + offset), //remove the //leading & - format!(".entry("), + ".entry(".to_string(), ), ( index.span.shrink_to_hi().with_hi(rv.span.lo()), @@ -826,7 +826,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { (val.span.shrink_to_lo(), "if let Some(val) = ".to_string()), ( val.span.shrink_to_hi().with_hi(index.span.lo()), - format!(".get_mut("), + ".get_mut(".to_string(), ), ( index.span.shrink_to_hi().with_hi(receiver.span.hi()), From 062ab4fd8e7bb76c263aca25685a2b89128ddec3 Mon Sep 17 00:00:00 2001 From: GTimothy <22472919+GTimothy@users.noreply.github.com> Date: Wed, 4 Mar 2026 15:57:52 +0100 Subject: [PATCH 7/9] add test for index mut suggestion causing borrow issue --- .../index-mut-suggest-borrow-issue.rs | 84 +++++++++++++++++++ .../index-mut-suggest-borrow-issue.stderr | 19 +++++ 2 files changed, 103 insertions(+) create mode 100644 tests/ui/borrowck/index-mut-suggest-borrow-issue.rs create mode 100644 tests/ui/borrowck/index-mut-suggest-borrow-issue.stderr diff --git a/tests/ui/borrowck/index-mut-suggest-borrow-issue.rs b/tests/ui/borrowck/index-mut-suggest-borrow-issue.rs new file mode 100644 index 0000000000000..a036d71511cfb --- /dev/null +++ b/tests/ui/borrowck/index-mut-suggest-borrow-issue.rs @@ -0,0 +1,84 @@ +// When mutably indexing a type that implements `Index` but not `IndexMut`, a +// special 'help' message is added to the output. +// +// The suggestions have different requirements on the index type. +// map[&idx] = val; +// If idx is later used, it results in a borrow issue if both: +// - the the suggestion that was chosen requires the idx, not a ref to it. +// - idx's type is not Copy. +// +// For now, we suggest all options, regardless of the Copy-ness of the idx. +use std::collections::HashMap; + +/// With a copy type, subsequent reuse of idx is not an issue. +fn copy_type() { + // ===&str===, a copy type. + + let mut map = HashMap::<&str, u32>::new(); + // earlier, peter is initialised with 22. + // map["peter"] = 22; //ERROR + map.insert("peter", 22); + + // at some point, if we get a &str variable peter again + let peter = "peter"; + // and we want to use it to update the map but still want to use it later? + // map[&peter] = 23; // ERROR + // we could insert again, and because &T are copy, we can use peter even if we use peter later. + map.insert(peter, 23); // WORKS + println!("my name is {peter}"); // WORKS because &str is Copy + // and we could use a &&str too in this case, because &str:Borrow<&str> (because T:Borrow) + if let Some(val) = map.get_mut(&peter) { + *val = 23; + }; // WORKS + println!("my name is {peter}"); // WORKS because &str is Copy + // even a &str directly, (because rust auto borrows peter -> &peter ?) + if let Some(val) = map.get_mut(peter) { + *val = 24; + }; // WORKS +} + +/// With a non-copy type, subsequent reuse of idx is an issue for `insert` and `entry`. +fn non_copy_type_insert() { + // ===STRING===, a non-copy type + + let mut map = HashMap::::new(); + // earlier, peter is initialised with 22. + // map[&"peter".to_string()] = 22; // ERROR cannot assign + map.insert("peter".to_string(), 22); + + // at some point, if we get a String variable peter again + let peter = "peter".to_string(); + // and we want to use it to update the map but still want to use it later? + // map[&peter] = 23; // ERROR cannot assign + // we could insert again, but we cannot use peter after. + map.insert(peter, 23); // WORKS + println!("my name is {peter}"); //~ ERROR: borrow of moved value: `peter` [E0382] +} + +/// With a non-copy type, subsequent reuse of idx is not an issue for `get_mut`. +fn non_copy_type_get_mut() { + // ===STRING===, a non-copy type + + let mut map = HashMap::::new(); + // earlier, peter is initialised with 22. + // map["peter".to_string()] = 22; // ERROR cannot assign + map.insert("peter".to_string(), 22); + + // at some point, if we get a String variable peter again + let peter = "peter".to_string(); + // and we want to use it to update the map but still want to use it later? + // map[&peter] = 23; // ERROR cannot assign + // we can use a &String in this case, so get_mut is always fine. + if let Some(val) = map.get_mut(&peter) { + *val = 23; + }; // WORKS + println!("my name is {peter}"); // WORKS + // or a &str because String:Borrow) and "peter" is &str. + + if let Some(val) = map.get_mut("peter") { + *val = 24; + }; // WORKS + println!("my name is {peter}"); // WORKS +} + +fn main() {} diff --git a/tests/ui/borrowck/index-mut-suggest-borrow-issue.stderr b/tests/ui/borrowck/index-mut-suggest-borrow-issue.stderr new file mode 100644 index 0000000000000..7182683245b37 --- /dev/null +++ b/tests/ui/borrowck/index-mut-suggest-borrow-issue.stderr @@ -0,0 +1,19 @@ +error[E0382]: borrow of moved value: `peter` + --> $DIR/index-mut-suggest-borrow-issue.rs:55:27 + | +LL | let peter = "peter".to_string(); + | ----- move occurs because `peter` has type `String`, which does not implement the `Copy` trait +... +LL | map.insert(peter, 23); // WORKS + | ----- value moved here +LL | println!("my name is {peter}"); + | ^^^^^ value borrowed here after move + | +help: consider cloning the value if the performance cost is acceptable + | +LL | map.insert(peter.clone(), 23); // WORKS + | ++++++++ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0382`. From 1b13ca49162c276eb6e65088caf6ffe0e86cc214 Mon Sep 17 00:00:00 2001 From: GTimothy <22472919+GTimothy@users.noreply.github.com> Date: Wed, 4 Mar 2026 16:47:25 +0100 Subject: [PATCH 8/9] remove copy/clone check on type of index --- .../src/diagnostics/mutability_errors.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 2518343119c77..b0f30524c7877 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -729,10 +729,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { false } } - /// checking if the key is copy clone - fn key_is_copyclone<'tcx>(key: Ty<'tcx>) -> bool { - key.is_trivially_pure_clone_copy() - } // we know ty is a map, with a key type at walk distance 2. let key_type = self.ty.walk().nth(1).unwrap().expect_ty(); @@ -743,11 +739,10 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { { let index_ty = self.infcx.tcx.typeck(val.hir_id.owner.def_id).expr_ty(index); - // only suggest `insert` and `entry` if K is copy/clone because of the signature. + // only suggest `insert` and `entry` if index is of type K or &K. let index_is_borrowed_key = index_is_borrowed_key(index_ty, key_type); - if (index_is_borrowed_key - || index_and_key_are_same_borrowed_type(index_ty, key_type)) - && key_is_copyclone(key_type) + if index_is_borrowed_key + || index_and_key_are_same_borrowed_type(index_ty, key_type) { let offset = BytePos(index_is_borrowed_key as u32); self.err.multipart_suggestion( From 9fea208dbf692d3e4f13f593ad20c948d6216e9d Mon Sep 17 00:00:00 2001 From: GTimothy <22472919+GTimothy@users.noreply.github.com> Date: Wed, 4 Mar 2026 16:57:56 +0100 Subject: [PATCH 9/9] refactor documentation/comments --- .../src/diagnostics/mutability_errors.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index b0f30524c7877..9535a4e5f089d 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -682,14 +682,13 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { } }; - // Lets see: - // because of TypeChecking and indexing, we know: index is &Q + // Because of TypeChecking and indexing, we know: index is &Q // with K: Eq + Hash + Borrow, - // with Q: Eq + Hash + ?Sized,index is a &Q with K:Borrow, + // with Q: Eq + Hash + ?Sized, // // // - // there is no other constraint on the types, therefore we need to look at the + // There is no other constraint on the types, therefore we need to look at the // constraints of the suggestions: // pub fn get_mut(&mut self, k: &Q) -> Option<&mut V> // where @@ -699,15 +698,15 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { // pub fn insert(&mut self, k: K, v: V) -> Option // pub fn entry(&mut self, key: K) -> Entry<'_, K, V> // - // But lets note that there could be also, if imported from hashbrown: + // But let's note that there could be also, if imported from hashbrown: // pub fn entry_ref<'a, 'b, Q>( - // &'a mut self, - // key: &'b Q, + // &'a mut self, + // key: &'b Q, // ) -> EntryRef<'a, 'b, K, Q, V, S, A> // where - // Q: Hash + Equivalent + ?Sized, + // Q: Hash + Equivalent + ?Sized, - /// testing if index AND key are &Q. We cannot simply test equality because the + /// Testing if index AND key are &Q. We cannot simply test equality because the /// lifetimes differ. fn index_and_key_are_same_borrowed_type<'tcx>( index: Ty<'tcx>, @@ -721,7 +720,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { false } } - /// testing if index is &K: + /// Testing if index is &K: fn index_is_borrowed_key<'tcx>(index: Ty<'tcx>, key: Ty<'tcx>) -> bool { if let ty::Ref(_, inner_ty, _) = index.kind() { *inner_ty == key