Skip to content

Commit 8d3b265

Browse files
committed
Suggest use .get_mut instead of &mut when overloaded index type not impl IndexMut
Signed-off-by: xizheyin <[email protected]>
1 parent aa50ab3 commit 8d3b265

File tree

3 files changed

+80
-11
lines changed

3 files changed

+80
-11
lines changed

compiler/rustc_borrowck/src/diagnostics/mod.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -889,7 +889,7 @@ pub(super) enum BorrowedContentSource<'tcx> {
889889
DerefMutableRef,
890890
DerefSharedRef,
891891
OverloadedDeref(Ty<'tcx>),
892-
OverloadedIndex(Ty<'tcx>),
892+
OverloadedIndex(Ty<'tcx>, Ty<'tcx>),
893893
}
894894

895895
impl<'tcx> BorrowedContentSource<'tcx> {
@@ -905,7 +905,7 @@ impl<'tcx> BorrowedContentSource<'tcx> {
905905
_ => None,
906906
})
907907
.unwrap_or_else(|| format!("dereference of `{ty}`")),
908-
BorrowedContentSource::OverloadedIndex(ty) => format!("index of `{ty}`"),
908+
BorrowedContentSource::OverloadedIndex(ty, _) => format!("index of `{ty}`"),
909909
}
910910
}
911911

@@ -917,7 +917,7 @@ impl<'tcx> BorrowedContentSource<'tcx> {
917917
// Overloaded deref and index operators should be evaluated into a
918918
// temporary. So we don't need a description here.
919919
BorrowedContentSource::OverloadedDeref(_)
920-
| BorrowedContentSource::OverloadedIndex(_) => None,
920+
| BorrowedContentSource::OverloadedIndex(_, _) => None,
921921
}
922922
}
923923

@@ -935,7 +935,7 @@ impl<'tcx> BorrowedContentSource<'tcx> {
935935
_ => None,
936936
})
937937
.unwrap_or_else(|| format!("dereference of `{ty}`")),
938-
BorrowedContentSource::OverloadedIndex(ty) => format!("an index of `{ty}`"),
938+
BorrowedContentSource::OverloadedIndex(ty, _) => format!("an index of `{ty}`"),
939939
}
940940
}
941941

@@ -951,7 +951,7 @@ impl<'tcx> BorrowedContentSource<'tcx> {
951951
} else if tcx.is_lang_item(trait_id, LangItem::Index)
952952
|| tcx.is_lang_item(trait_id, LangItem::IndexMut)
953953
{
954-
Some(BorrowedContentSource::OverloadedIndex(args.type_at(0)))
954+
Some(BorrowedContentSource::OverloadedIndex(args.type_at(0), args.type_at(1)))
955955
} else {
956956
None
957957
}

compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
519519
but it is not implemented for `{ty}`",
520520
));
521521
}
522-
Some(BorrowedContentSource::OverloadedIndex(ty)) => {
522+
Some(BorrowedContentSource::OverloadedIndex(ty, _)) => {
523523
err.help(format!(
524524
"trait `IndexMut` is required to modify indexed content, \
525525
but it is not implemented for `{ty}`",
@@ -1176,6 +1176,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
11761176
self.body.source_info(location).span
11771177
}
11781178
});
1179+
11791180
match opt_assignment_rhs_span.and_then(|s| s.desugaring_kind()) {
11801181
// on for loops, RHS points to the iterator part
11811182
Some(DesugaringKind::ForLoop) => {
@@ -1196,7 +1197,13 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
11961197
None
11971198
}
11981199
None => {
1199-
if name != kw::SelfLower {
1200+
let suggest_get_mut = self.suggest_get_mut_when_not_impl_index_mut(
1201+
local,
1202+
opt_assignment_rhs_span,
1203+
);
1204+
if suggest_get_mut.is_some() {
1205+
suggest_get_mut
1206+
} else if name != kw::SelfLower {
12001207
suggest_ampmut(
12011208
self.infcx.tcx,
12021209
local_decl.ty,
@@ -1414,6 +1421,66 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
14141421
None => {}
14151422
}
14161423
}
1424+
1425+
/// check if the RHS is an overloaded index expression from hashmap or btreemap,
1426+
/// if so, suggest using .get_mut() instead of &mut, see issue #143732
1427+
/// For example
1428+
/// ```text
1429+
/// let mut map = HashMap::new();
1430+
/// let value = &map["key"];
1431+
/// ```
1432+
fn suggest_get_mut_when_not_impl_index_mut(
1433+
&self,
1434+
local: Local,
1435+
opt_assignment_rhs_span: Option<Span>,
1436+
) -> Option<AmpMutSugg> {
1437+
self.find_assignments(local)
1438+
.first()
1439+
.map(|&location| {
1440+
if let Some(mir::Statement {
1441+
source_info: _,
1442+
kind: mir::StatementKind::Assign(box (_, mir::Rvalue::Ref(_, _, place))),
1443+
..
1444+
}) = self.body[location.block].statements.get(location.statement_index)
1445+
&& let BorrowedContentSource::OverloadedIndex(ty, index_ty) =
1446+
self.borrowed_content_source(place.as_ref())
1447+
&& let Some(index_mut_trait) = self.infcx.tcx.lang_items().index_mut_trait()
1448+
&& !self
1449+
.infcx
1450+
.type_implements_trait(
1451+
index_mut_trait,
1452+
[ty, index_ty],
1453+
self.infcx.param_env,
1454+
)
1455+
.must_apply_modulo_regions()
1456+
{
1457+
if let Some(rhs_span) = opt_assignment_rhs_span
1458+
&& let Ok(rhs_str) =
1459+
self.infcx.tcx.sess.source_map().span_to_snippet(rhs_span)
1460+
&& let Some(content) = rhs_str.strip_prefix('&')
1461+
&& content.contains('[')
1462+
&& content.contains(']')
1463+
{
1464+
let bracket_start = content.find('[')?;
1465+
let bracket_end = content.rfind(']')?;
1466+
1467+
if bracket_start < bracket_end {
1468+
let map_part = &content[..bracket_start];
1469+
let key_part = &content[bracket_start + 1..bracket_end];
1470+
1471+
return Some(AmpMutSugg {
1472+
has_sugg: true,
1473+
span: rhs_span,
1474+
suggestion: format!("{}.get_mut({}).unwrap()", map_part, key_part),
1475+
additional: None,
1476+
});
1477+
}
1478+
}
1479+
}
1480+
None
1481+
})
1482+
.flatten()
1483+
}
14171484
}
14181485

14191486
struct BindingFinder {

tests/ui/borrowck/suggest-use-as-mut-for-map.stderr

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ LL | string.push_str("test");
66
|
77
help: consider changing this to be a mutable reference
88
|
9-
LL | let string = &mut map[&0];
10-
| +++
9+
LL - let string = &map[&0];
10+
LL + let string = map.get_mut(&0).unwrap();
11+
|
1112

1213
error[E0596]: cannot borrow `*string` as mutable, as it is behind a `&` reference
1314
--> $DIR/suggest-use-as-mut-for-map.rs:14:5
@@ -17,8 +18,9 @@ LL | string.push_str("test");
1718
|
1819
help: consider changing this to be a mutable reference
1920
|
20-
LL | let string = &mut map[&0];
21-
| +++
21+
LL - let string = &map[&0];
22+
LL + let string = map.get_mut(&0).unwrap();
23+
|
2224

2325
error[E0596]: cannot borrow `*string` as mutable, as it is behind a `&` reference
2426
--> $DIR/suggest-use-as-mut-for-map.rs:18:5

0 commit comments

Comments
 (0)