diff options
| author | Stuart Cook <Zalathar@users.noreply.github.com> | 2025-01-01 16:35:31 +1100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2025-01-01 16:35:31 +1100 |
| commit | f91bfd97bfb411bd3f27cc5dacc88baa345c4162 (patch) | |
| tree | 0408945131e51c9f2e091e8c64d2666437683631 | |
| parent | 1ea1db5b0875c579c9b9e4d078c2ee4bf0ffe6e5 (diff) | |
| parent | f28e13b0552ab7b95e0cc764c5285ebac23fcaa2 (diff) | |
| download | rust-f91bfd97bfb411bd3f27cc5dacc88baa345c4162.tar.gz rust-f91bfd97bfb411bd3f27cc5dacc88baa345c4162.zip | |
Rollup merge of #134945 - compiler-errors:map-mutate-nits, r=estebank
Some small nits to the borrowck suggestions for mutating a map through index
1. Suggesting users to either use `.insert` or `.get_mut` (which do totally different things) can be a bit of a footgun, so let's make that a bit more nuanced.
2. I find the suggestion of `.get_mut(|val| { *val = whatever; })` to be a bit awkward. I changed this to be an if-let instead.
3. Fix a bug which was suppressing the structured suggestion for some mutations via the index operator on `HashMap`/`BTreeMap`.
r? estebank or reassign
| -rw-r--r-- | compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs | 17 | ||||
| -rw-r--r-- | tests/ui/borrowck/index-mut-help.stderr | 11 | ||||
| -rw-r--r-- | tests/ui/btreemap/btreemap-index-mut-2.stderr | 6 | ||||
| -rw-r--r-- | tests/ui/btreemap/btreemap-index-mut.stderr | 6 | ||||
| -rw-r--r-- | tests/ui/hashmap/hashmap-index-mut.stderr | 6 | ||||
| -rw-r--r-- | tests/ui/issues/issue-41726.stderr | 5 |
6 files changed, 30 insertions, 21 deletions
diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 2484f817a06..c690789b587 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -575,7 +575,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { // ---------- place self.err.multipart_suggestions( format!( - "to modify a `{}`, use `.get_mut()`, `.insert()` or the entry API", + "use `.insert()` to insert a value into a `{}`, `.get_mut()` \ + to modify it, or the entry API for more flexibility", self.ty, ), vec![ @@ -592,16 +593,17 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { (rv.span.shrink_to_hi(), ")".to_string()), ], vec![ - // val.get_mut(index).map(|v| { *v = rv; }); + // 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()), - ").map(|val| { *val".to_string(), + ") { *val".to_string(), ), - (rv.span.shrink_to_hi(), "; })".to_string()), + (rv.span.shrink_to_hi(), "; }".to_string()), ], vec![ // let x = val.entry(index).or_insert(rv); @@ -622,21 +624,22 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { self.suggested = true; } else if let hir::ExprKind::MethodCall(_path, receiver, _, sp) = expr.kind && let hir::ExprKind::Index(val, index, _) = receiver.kind - && expr.span == self.assign_span + && receiver.span == self.assign_span { // 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()), ".get_mut(".to_string(), ), ( index.span.shrink_to_hi().with_hi(receiver.span.hi()), - ").map(|val| val".to_string(), + ") { val".to_string(), ), - (sp.shrink_to_hi(), ")".to_string()), + (sp.shrink_to_hi(), "; }".to_string()), ], Applicability::MachineApplicable, ); diff --git a/tests/ui/borrowck/index-mut-help.stderr b/tests/ui/borrowck/index-mut-help.stderr index fde2b5dc076..c4c9c1c5313 100644 --- a/tests/ui/borrowck/index-mut-help.stderr +++ b/tests/ui/borrowck/index-mut-help.stderr @@ -5,7 +5,10 @@ LL | map["peter"].clear(); | ^^^^^^^^^^^^ cannot borrow as mutable | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<&str, String>` - = help: to modify a `HashMap<&str, String>`, use `.get_mut()`, `.insert()` or the entry API +help: to modify a `HashMap<&str, String>` use `.get_mut()` + | +LL | if let Some(val) = map.get_mut("peter") { val.clear(); }; + | ++++++++++++++++++ ~~~~~~~~~ ~~~~~~~ +++ error[E0594]: cannot assign to data in an index of `HashMap<&str, String>` --> $DIR/index-mut-help.rs:11:5 @@ -14,12 +17,12 @@ 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: to modify a `HashMap<&str, String>`, use `.get_mut()`, `.insert()` or the entry API +help: use `.insert()` to insert a value into a `HashMap<&str, String>`, `.get_mut()` to modify it, or the entry API for more flexibility | LL | map.insert("peter", "0".to_string()); | ~~~~~~~~ ~ + -LL | map.get_mut("peter").map(|val| { *val = "0".to_string(); }); - | ~~~~~~~~~ ~~~~~~~~~~~~~~~~~~ ++++ +LL | if let Some(val) = map.get_mut("peter") { *val = "0".to_string(); }; + | ++++++++++++++++++ ~~~~~~~~~ ~~~~~~~~ +++ LL | let val = map.entry("peter").or_insert("0".to_string()); | +++++++++ ~~~~~~~ ~~~~~~~~~~~~ + diff --git a/tests/ui/btreemap/btreemap-index-mut-2.stderr b/tests/ui/btreemap/btreemap-index-mut-2.stderr index 0b8c77cb9e1..c42462ee1eb 100644 --- a/tests/ui/btreemap/btreemap-index-mut-2.stderr +++ b/tests/ui/btreemap/btreemap-index-mut-2.stderr @@ -5,12 +5,12 @@ LL | map[&0] = 1; | ^^^^^^^^^^^ cannot assign | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `BTreeMap<u32, u32>` -help: to modify a `BTreeMap<u32, u32>`, use `.get_mut()`, `.insert()` or the entry API +help: use `.insert()` to insert a value into a `BTreeMap<u32, u32>`, `.get_mut()` to modify it, or the entry API for more flexibility | LL | map.insert(&0, 1); | ~~~~~~~~ ~ + -LL | map.get_mut(&0).map(|val| { *val = 1; }); - | ~~~~~~~~~ ~~~~~~~~~~~~~~~~~~ ++++ +LL | if let Some(val) = map.get_mut(&0) { *val = 1; }; + | ++++++++++++++++++ ~~~~~~~~~ ~~~~~~~~ +++ LL | let val = map.entry(&0).or_insert(1); | +++++++++ ~~~~~~~ ~~~~~~~~~~~~ + diff --git a/tests/ui/btreemap/btreemap-index-mut.stderr b/tests/ui/btreemap/btreemap-index-mut.stderr index cc465fbf3de..f402f503c15 100644 --- a/tests/ui/btreemap/btreemap-index-mut.stderr +++ b/tests/ui/btreemap/btreemap-index-mut.stderr @@ -5,12 +5,12 @@ LL | map[&0] = 1; | ^^^^^^^^^^^ cannot assign | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `BTreeMap<u32, u32>` -help: to modify a `BTreeMap<u32, u32>`, use `.get_mut()`, `.insert()` or the entry API +help: use `.insert()` to insert a value into a `BTreeMap<u32, u32>`, `.get_mut()` to modify it, or the entry API for more flexibility | LL | map.insert(&0, 1); | ~~~~~~~~ ~ + -LL | map.get_mut(&0).map(|val| { *val = 1; }); - | ~~~~~~~~~ ~~~~~~~~~~~~~~~~~~ ++++ +LL | if let Some(val) = map.get_mut(&0) { *val = 1; }; + | ++++++++++++++++++ ~~~~~~~~~ ~~~~~~~~ +++ LL | let val = map.entry(&0).or_insert(1); | +++++++++ ~~~~~~~ ~~~~~~~~~~~~ + diff --git a/tests/ui/hashmap/hashmap-index-mut.stderr b/tests/ui/hashmap/hashmap-index-mut.stderr index 2381b8ecb96..ad33c6f9b15 100644 --- a/tests/ui/hashmap/hashmap-index-mut.stderr +++ b/tests/ui/hashmap/hashmap-index-mut.stderr @@ -5,12 +5,12 @@ LL | map[&0] = 1; | ^^^^^^^^^^^ cannot assign | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<u32, u32>` -help: to modify a `HashMap<u32, u32>`, use `.get_mut()`, `.insert()` or the entry API +help: use `.insert()` to insert a value into a `HashMap<u32, u32>`, `.get_mut()` to modify it, or the entry API for more flexibility | LL | map.insert(&0, 1); | ~~~~~~~~ ~ + -LL | map.get_mut(&0).map(|val| { *val = 1; }); - | ~~~~~~~~~ ~~~~~~~~~~~~~~~~~~ ++++ +LL | if let Some(val) = map.get_mut(&0) { *val = 1; }; + | ++++++++++++++++++ ~~~~~~~~~ ~~~~~~~~ +++ LL | let val = map.entry(&0).or_insert(1); | +++++++++ ~~~~~~~ ~~~~~~~~~~~~ + diff --git a/tests/ui/issues/issue-41726.stderr b/tests/ui/issues/issue-41726.stderr index fe7d4df7067..250bba222bf 100644 --- a/tests/ui/issues/issue-41726.stderr +++ b/tests/ui/issues/issue-41726.stderr @@ -5,7 +5,10 @@ LL | things[src.as_str()].sort(); | ^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<String, Vec<String>>` - = help: to modify a `HashMap<String, Vec<String>>`, use `.get_mut()`, `.insert()` or the entry API +help: to modify a `HashMap<String, Vec<String>>` use `.get_mut()` + | +LL | if let Some(val) = things.get_mut(src.as_str()) { val.sort(); }; + | ++++++++++++++++++ ~~~~~~~~~ ~~~~~~~ +++ error: aborting due to 1 previous error |
