Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit 225e43a

Browse files
ZalatharZalathar
authored
Rollup merge of rust-lang#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
2 parents a0a9657 + f28e13b commit 225e43a

File tree

6 files changed

+30
-21
lines changed

6 files changed

+30
-21
lines changed

‎compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
575575
// ---------- place
576576
self.err.multipart_suggestions(
577577
format!(
578-
"to modify a `{}`, use `.get_mut()`, `.insert()` or the entry API",
578+
"use `.insert()` to insert a value into a `{}`, `.get_mut()` \
579+
to modify it, or the entry API for more flexibility",
579580
self.ty,
580581
),
581582
vec![
@@ -592,16 +593,17 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
592593
(rv.span.shrink_to_hi(), ")".to_string()),
593594
],
594595
vec![
595-
// val.get_mut(index).map(|v| { *v = rv; });
596+
// if let Some(v) = val.get_mut(index) { *v = rv; }
597+
(val.span.shrink_to_lo(), "if let Some(val) = ".to_string()),
596598
(
597599
val.span.shrink_to_hi().with_hi(index.span.lo()),
598600
".get_mut(".to_string(),
599601
),
600602
(
601603
index.span.shrink_to_hi().with_hi(place.span.hi()),
602-
").map(|val| { *val".to_string(),
604+
") { *val".to_string(),
603605
),
604-
(rv.span.shrink_to_hi(), "; })".to_string()),
606+
(rv.span.shrink_to_hi(), "; }".to_string()),
605607
],
606608
vec![
607609
// let x = val.entry(index).or_insert(rv);
@@ -622,21 +624,22 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
622624
self.suggested = true;
623625
} else if let hir::ExprKind::MethodCall(_path, receiver, _, sp) = expr.kind
624626
&& let hir::ExprKind::Index(val, index, _) = receiver.kind
625-
&& expr.span == self.assign_span
627+
&& receiver.span == self.assign_span
626628
{
627629
// val[index].path(args..);
628630
self.err.multipart_suggestion(
629631
format!("to modify a `{}` use `.get_mut()`", self.ty),
630632
vec![
633+
(val.span.shrink_to_lo(), "if let Some(val) = ".to_string()),
631634
(
632635
val.span.shrink_to_hi().with_hi(index.span.lo()),
633636
".get_mut(".to_string(),
634637
),
635638
(
636639
index.span.shrink_to_hi().with_hi(receiver.span.hi()),
637-
").map(|val| val".to_string(),
640+
") { val".to_string(),
638641
),
639-
(sp.shrink_to_hi(), ")".to_string()),
642+
(sp.shrink_to_hi(), "; }".to_string()),
640643
],
641644
Applicability::MachineApplicable,
642645
);

‎tests/ui/borrowck/index-mut-help.stderr‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ LL | map["peter"].clear();
55
| ^^^^^^^^^^^^ cannot borrow as mutable
66
|
77
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<&str, String>`
8-
= help: to modify a `HashMap<&str, String>`, use `.get_mut()`, `.insert()` or the entry API
8+
help: to modify a `HashMap<&str, String>` use `.get_mut()`
9+
|
10+
LL | if let Some(val) = map.get_mut("peter") { val.clear(); };
11+
| ++++++++++++++++++ ~~~~~~~~~ ~~~~~~~ +++
912

1013
error[E0594]: cannot assign to data in an index of `HashMap<&str, String>`
1114
--> $DIR/index-mut-help.rs:11:5
@@ -14,12 +17,12 @@ LL | map["peter"] = "0".to_string();
1417
| ^^^^^^^^^^^^ cannot assign
1518
|
1619
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<&str, String>`
17-
help: to modify a `HashMap<&str, String>`, use `.get_mut()`, `.insert()` or the entry API
20+
help: use `.insert()` to insert a value into a `HashMap<&str, String>`, `.get_mut()` to modify it, or the entry API for more flexibility
1821
|
1922
LL | map.insert("peter", "0".to_string());
2023
| ~~~~~~~~ ~ +
21-
LL | map.get_mut("peter").map(|val| { *val = "0".to_string(); });
22-
| ~~~~~~~~~ ~~~~~~~~~~~~~~~~~~ ++++
24+
LL | if let Some(val) = map.get_mut("peter") { *val = "0".to_string(); };
25+
| ++++++++++++++++++ ~~~~~~~~~ ~~~~~~~~ +++
2326
LL | let val = map.entry("peter").or_insert("0".to_string());
2427
| +++++++++ ~~~~~~~ ~~~~~~~~~~~~ +
2528

‎tests/ui/btreemap/btreemap-index-mut-2.stderr‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@ LL | map[&0] = 1;
55
| ^^^^^^^^^^^ cannot assign
66
|
77
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `BTreeMap<u32, u32>`
8-
help: to modify a `BTreeMap<u32, u32>`, use `.get_mut()`, `.insert()` or the entry API
8+
help: use `.insert()` to insert a value into a `BTreeMap<u32, u32>`, `.get_mut()` to modify it, or the entry API for more flexibility
99
|
1010
LL | map.insert(&0, 1);
1111
| ~~~~~~~~ ~ +
12-
LL | map.get_mut(&0).map(|val| { *val = 1; });
13-
| ~~~~~~~~~ ~~~~~~~~~~~~~~~~~~ ++++
12+
LL | if let Some(val) = map.get_mut(&0) { *val = 1; };
13+
| ++++++++++++++++++ ~~~~~~~~~ ~~~~~~~~ +++
1414
LL | let val = map.entry(&0).or_insert(1);
1515
| +++++++++ ~~~~~~~ ~~~~~~~~~~~~ +
1616

‎tests/ui/btreemap/btreemap-index-mut.stderr‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@ LL | map[&0] = 1;
55
| ^^^^^^^^^^^ cannot assign
66
|
77
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `BTreeMap<u32, u32>`
8-
help: to modify a `BTreeMap<u32, u32>`, use `.get_mut()`, `.insert()` or the entry API
8+
help: use `.insert()` to insert a value into a `BTreeMap<u32, u32>`, `.get_mut()` to modify it, or the entry API for more flexibility
99
|
1010
LL | map.insert(&0, 1);
1111
| ~~~~~~~~ ~ +
12-
LL | map.get_mut(&0).map(|val| { *val = 1; });
13-
| ~~~~~~~~~ ~~~~~~~~~~~~~~~~~~ ++++
12+
LL | if let Some(val) = map.get_mut(&0) { *val = 1; };
13+
| ++++++++++++++++++ ~~~~~~~~~ ~~~~~~~~ +++
1414
LL | let val = map.entry(&0).or_insert(1);
1515
| +++++++++ ~~~~~~~ ~~~~~~~~~~~~ +
1616

‎tests/ui/hashmap/hashmap-index-mut.stderr‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@ LL | map[&0] = 1;
55
| ^^^^^^^^^^^ cannot assign
66
|
77
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<u32, u32>`
8-
help: to modify a `HashMap<u32, u32>`, use `.get_mut()`, `.insert()` or the entry API
8+
help: use `.insert()` to insert a value into a `HashMap<u32, u32>`, `.get_mut()` to modify it, or the entry API for more flexibility
99
|
1010
LL | map.insert(&0, 1);
1111
| ~~~~~~~~ ~ +
12-
LL | map.get_mut(&0).map(|val| { *val = 1; });
13-
| ~~~~~~~~~ ~~~~~~~~~~~~~~~~~~ ++++
12+
LL | if let Some(val) = map.get_mut(&0) { *val = 1; };
13+
| ++++++++++++++++++ ~~~~~~~~~ ~~~~~~~~ +++
1414
LL | let val = map.entry(&0).or_insert(1);
1515
| +++++++++ ~~~~~~~ ~~~~~~~~~~~~ +
1616

‎tests/ui/issues/issue-41726.stderr‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ LL | things[src.as_str()].sort();
55
| ^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable
66
|
77
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<String, Vec<String>>`
8-
= help: to modify a `HashMap<String, Vec<String>>`, use `.get_mut()`, `.insert()` or the entry API
8+
help: to modify a `HashMap<String, Vec<String>>` use `.get_mut()`
9+
|
10+
LL | if let Some(val) = things.get_mut(src.as_str()) { val.sort(); };
11+
| ++++++++++++++++++ ~~~~~~~~~ ~~~~~~~ +++
912

1013
error: aborting due to 1 previous error
1114

0 commit comments

Comments
 (0)