Skip to content

Commit 60c825f

Browse files
authored
Rollup merge of #124313 - estebank:split-at-mut, r=fee1-dead
Detect borrow error involving sub-slices and suggest `split_at_mut` ``` error[E0499]: cannot borrow `foo` as mutable more than once at a time --> $DIR/suggest-split-at-mut.rs:13:18 | LL | let a = &mut foo[..2]; | --- first mutable borrow occurs here LL | let b = &mut foo[2..]; | ^^^ second mutable borrow occurs here LL | a[0] = 5; | ---- first borrow later used here | = help: use `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices ``` Address most of #58792. For follow up work, we should emit a structured suggestion for cases where we can identify the exact `let (a, b) = foo.split_at_mut(2);` call that is needed.
2 parents 9e6c4fd + 64a4cdc commit 60c825f

File tree

4 files changed

+235
-23
lines changed

4 files changed

+235
-23
lines changed

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

+66-18
Original file line numberDiff line numberDiff line change
@@ -1527,7 +1527,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
15271527
BorrowKind::Mut { kind: MutBorrowKind::Default | MutBorrowKind::TwoPhaseBorrow },
15281528
) => {
15291529
first_borrow_desc = "mutable ";
1530-
self.cannot_reborrow_already_borrowed(
1530+
let mut err = self.cannot_reborrow_already_borrowed(
15311531
span,
15321532
&desc_place,
15331533
&msg_place,
@@ -1537,7 +1537,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
15371537
"mutable",
15381538
&msg_borrow,
15391539
None,
1540-
)
1540+
);
1541+
self.suggest_slice_method_if_applicable(
1542+
&mut err,
1543+
place,
1544+
issued_borrow.borrowed_place,
1545+
span,
1546+
issued_span,
1547+
);
1548+
err
15411549
}
15421550
(
15431551
BorrowKind::Mut { kind: MutBorrowKind::Default | MutBorrowKind::TwoPhaseBorrow },
@@ -1555,6 +1563,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
15551563
&msg_borrow,
15561564
None,
15571565
);
1566+
self.suggest_slice_method_if_applicable(
1567+
&mut err,
1568+
place,
1569+
issued_borrow.borrowed_place,
1570+
span,
1571+
issued_span,
1572+
);
15581573
self.suggest_binding_for_closure_capture_self(&mut err, &issued_spans);
15591574
self.suggest_using_closure_argument_instead_of_capture(
15601575
&mut err,
@@ -1581,6 +1596,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
15811596
&mut err,
15821597
place,
15831598
issued_borrow.borrowed_place,
1599+
span,
1600+
issued_span,
15841601
);
15851602
self.suggest_using_closure_argument_instead_of_capture(
15861603
&mut err,
@@ -2011,40 +2028,47 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
20112028
err: &mut Diag<'_>,
20122029
place: Place<'tcx>,
20132030
borrowed_place: Place<'tcx>,
2031+
span: Span,
2032+
issued_span: Span,
20142033
) {
20152034
let tcx = self.infcx.tcx;
20162035
let hir = tcx.hir();
20172036

2037+
let has_split_at_mut = |ty: Ty<'tcx>| {
2038+
let ty = ty.peel_refs();
2039+
match ty.kind() {
2040+
ty::Array(..) | ty::Slice(..) => true,
2041+
ty::Adt(def, _) if tcx.get_diagnostic_item(sym::Vec) == Some(def.did()) => true,
2042+
_ if ty == tcx.types.str_ => true,
2043+
_ => false,
2044+
}
2045+
};
20182046
if let ([ProjectionElem::Index(index1)], [ProjectionElem::Index(index2)])
20192047
| (
20202048
[ProjectionElem::Deref, ProjectionElem::Index(index1)],
20212049
[ProjectionElem::Deref, ProjectionElem::Index(index2)],
20222050
) = (&place.projection[..], &borrowed_place.projection[..])
20232051
{
2052+
let decl1 = &self.body.local_decls[*index1];
2053+
let decl2 = &self.body.local_decls[*index2];
2054+
20242055
let mut note_default_suggestion = || {
20252056
err.help(
2026-
"consider using `.split_at_mut(position)` or similar method to obtain \
2027-
two mutable non-overlapping sub-slices",
2057+
"consider using `.split_at_mut(position)` or similar method to obtain two \
2058+
mutable non-overlapping sub-slices",
20282059
)
2029-
.help("consider using `.swap(index_1, index_2)` to swap elements at the specified indices");
2030-
};
2031-
2032-
let Some(body_id) = tcx.hir_node(self.mir_hir_id()).body_id() else {
2033-
note_default_suggestion();
2034-
return;
2060+
.help(
2061+
"consider using `.swap(index_1, index_2)` to swap elements at the specified \
2062+
indices",
2063+
);
20352064
};
20362065

2037-
let mut expr_finder =
2038-
FindExprBySpan::new(self.body.local_decls[*index1].source_info.span, tcx);
2039-
expr_finder.visit_expr(hir.body(body_id).value);
2040-
let Some(index1) = expr_finder.result else {
2066+
let Some(index1) = self.find_expr(decl1.source_info.span) else {
20412067
note_default_suggestion();
20422068
return;
20432069
};
20442070

2045-
expr_finder = FindExprBySpan::new(self.body.local_decls[*index2].source_info.span, tcx);
2046-
expr_finder.visit_expr(hir.body(body_id).value);
2047-
let Some(index2) = expr_finder.result else {
2071+
let Some(index2) = self.find_expr(decl2.source_info.span) else {
20482072
note_default_suggestion();
20492073
return;
20502074
};
@@ -2092,7 +2116,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
20922116
None
20932117
}
20942118
}) else {
2095-
note_default_suggestion();
2119+
let hir::Node::Expr(parent) = tcx.parent_hir_node(index1.hir_id) else { return };
2120+
let hir::ExprKind::Index(_, idx1, _) = parent.kind else { return };
2121+
let hir::Node::Expr(parent) = tcx.parent_hir_node(index2.hir_id) else { return };
2122+
let hir::ExprKind::Index(_, idx2, _) = parent.kind else { return };
2123+
if !idx1.equivalent_for_indexing(idx2) {
2124+
err.help("use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices");
2125+
}
20962126
return;
20972127
};
20982128

@@ -2102,7 +2132,25 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
21022132
format!("{obj_str}.swap({index1_str}, {index2_str})"),
21032133
Applicability::MachineApplicable,
21042134
);
2135+
return;
2136+
}
2137+
let place_ty = PlaceRef::ty(&place.as_ref(), self.body, tcx).ty;
2138+
let borrowed_place_ty = PlaceRef::ty(&borrowed_place.as_ref(), self.body, tcx).ty;
2139+
if !has_split_at_mut(place_ty) && !has_split_at_mut(borrowed_place_ty) {
2140+
// Only mention `split_at_mut` on `Vec`, array and slices.
2141+
return;
2142+
}
2143+
let Some(index1) = self.find_expr(span) else { return };
2144+
let hir::Node::Expr(parent) = tcx.parent_hir_node(index1.hir_id) else { return };
2145+
let hir::ExprKind::Index(_, idx1, _) = parent.kind else { return };
2146+
let Some(index2) = self.find_expr(issued_span) else { return };
2147+
let hir::Node::Expr(parent) = tcx.parent_hir_node(index2.hir_id) else { return };
2148+
let hir::ExprKind::Index(_, idx2, _) = parent.kind else { return };
2149+
if idx1.equivalent_for_indexing(idx2) {
2150+
// `let a = &mut foo[0]` and `let b = &mut foo[0]`? Don't mention `split_at_mut`
2151+
return;
21052152
}
2153+
err.help("use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices");
21062154
}
21072155

21082156
/// Suggest using `while let` for call `next` on an iterator in a for loop.

compiler/rustc_hir/src/hir.rs

+38
Original file line numberDiff line numberDiff line change
@@ -1811,6 +1811,44 @@ impl Expr<'_> {
18111811
}
18121812
}
18131813

1814+
/// Whether this and the `other` expression are the same for purposes of an indexing operation.
1815+
///
1816+
/// This is only used for diagnostics to see if we have things like `foo[i]` where `foo` is
1817+
/// borrowed multiple times with `i`.
1818+
pub fn equivalent_for_indexing(&self, other: &Expr<'_>) -> bool {
1819+
match (self.kind, other.kind) {
1820+
(ExprKind::Lit(lit1), ExprKind::Lit(lit2)) => lit1.node == lit2.node,
1821+
(
1822+
ExprKind::Path(QPath::LangItem(item1, _)),
1823+
ExprKind::Path(QPath::LangItem(item2, _)),
1824+
) => item1 == item2,
1825+
(
1826+
ExprKind::Path(QPath::Resolved(None, path1)),
1827+
ExprKind::Path(QPath::Resolved(None, path2)),
1828+
) => path1.res == path2.res,
1829+
(
1830+
ExprKind::Struct(QPath::LangItem(LangItem::RangeTo, _), [val1], None),
1831+
ExprKind::Struct(QPath::LangItem(LangItem::RangeTo, _), [val2], None),
1832+
)
1833+
| (
1834+
ExprKind::Struct(QPath::LangItem(LangItem::RangeToInclusive, _), [val1], None),
1835+
ExprKind::Struct(QPath::LangItem(LangItem::RangeToInclusive, _), [val2], None),
1836+
)
1837+
| (
1838+
ExprKind::Struct(QPath::LangItem(LangItem::RangeFrom, _), [val1], None),
1839+
ExprKind::Struct(QPath::LangItem(LangItem::RangeFrom, _), [val2], None),
1840+
) => val1.expr.equivalent_for_indexing(val2.expr),
1841+
(
1842+
ExprKind::Struct(QPath::LangItem(LangItem::Range, _), [val1, val3], None),
1843+
ExprKind::Struct(QPath::LangItem(LangItem::Range, _), [val2, val4], None),
1844+
) => {
1845+
val1.expr.equivalent_for_indexing(val2.expr)
1846+
&& val3.expr.equivalent_for_indexing(val4.expr)
1847+
}
1848+
_ => false,
1849+
}
1850+
}
1851+
18141852
pub fn method_ident(&self) -> Option<Ident> {
18151853
match self.kind {
18161854
ExprKind::MethodCall(receiver_method, ..) => Some(receiver_method.ident),
+55-1
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,62 @@
1-
fn main() {
1+
fn foo() {
22
let mut foo = [1, 2, 3, 4];
33
let a = &mut foo[2];
44
let b = &mut foo[3]; //~ ERROR cannot borrow `foo[_]` as mutable more than once at a time
55
*a = 5;
66
*b = 6;
77
println!("{:?} {:?}", a, b);
88
}
9+
10+
fn bar() {
11+
let mut foo = [1,2,3,4];
12+
let a = &mut foo[..2];
13+
let b = &mut foo[2..]; //~ ERROR cannot borrow `foo` as mutable more than once at a time
14+
a[0] = 5;
15+
b[0] = 6;
16+
println!("{:?} {:?}", a, b);
17+
}
18+
19+
fn baz() {
20+
let mut foo = [1,2,3,4];
21+
let a = &foo[..2];
22+
let b = &mut foo[2..]; //~ ERROR cannot borrow `foo` as mutable because it is also borrowed as immutable
23+
b[0] = 6;
24+
println!("{:?} {:?}", a, b);
25+
}
26+
27+
fn qux() {
28+
let mut foo = [1,2,3,4];
29+
let a = &mut foo[..2];
30+
let b = &foo[2..]; //~ ERROR cannot borrow `foo` as immutable because it is also borrowed as mutable
31+
a[0] = 5;
32+
println!("{:?} {:?}", a, b);
33+
}
34+
35+
fn bad() {
36+
let mut foo = [1,2,3,4];
37+
let a = &foo[1];
38+
let b = &mut foo[2]; //~ ERROR cannot borrow `foo[_]` as mutable because it is also borrowed as immutable
39+
*b = 6;
40+
println!("{:?} {:?}", a, b);
41+
}
42+
43+
fn bat() {
44+
let mut foo = [1,2,3,4];
45+
let a = &mut foo[1];
46+
let b = &foo[2]; //~ ERROR cannot borrow `foo[_]` as immutable because it is also borrowed as mutable
47+
*a = 5;
48+
println!("{:?} {:?}", a, b);
49+
}
50+
51+
fn ang() {
52+
let mut foo = [1,2,3,4];
53+
let a = &mut foo[0..];
54+
let b = &foo[0..]; //~ ERROR cannot borrow `foo` as immutable because it is also borrowed as mutable
55+
a[0] = 5;
56+
println!("{:?} {:?}", a, b);
57+
}
58+
59+
fn main() {
60+
foo();
61+
bar();
62+
}

tests/ui/suggestions/suggest-split-at-mut.stderr

+76-4
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,81 @@ LL | let b = &mut foo[3];
88
LL | *a = 5;
99
| ------ first borrow later used here
1010
|
11-
= help: consider using `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices
12-
= help: consider using `.swap(index_1, index_2)` to swap elements at the specified indices
11+
= help: use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices
1312

14-
error: aborting due to 1 previous error
13+
error[E0499]: cannot borrow `foo` as mutable more than once at a time
14+
--> $DIR/suggest-split-at-mut.rs:13:18
15+
|
16+
LL | let a = &mut foo[..2];
17+
| --- first mutable borrow occurs here
18+
LL | let b = &mut foo[2..];
19+
| ^^^ second mutable borrow occurs here
20+
LL | a[0] = 5;
21+
| ---- first borrow later used here
22+
|
23+
= help: use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices
24+
25+
error[E0502]: cannot borrow `foo` as mutable because it is also borrowed as immutable
26+
--> $DIR/suggest-split-at-mut.rs:22:18
27+
|
28+
LL | let a = &foo[..2];
29+
| --- immutable borrow occurs here
30+
LL | let b = &mut foo[2..];
31+
| ^^^ mutable borrow occurs here
32+
LL | b[0] = 6;
33+
LL | println!("{:?} {:?}", a, b);
34+
| - immutable borrow later used here
35+
|
36+
= help: use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices
37+
38+
error[E0502]: cannot borrow `foo` as immutable because it is also borrowed as mutable
39+
--> $DIR/suggest-split-at-mut.rs:30:14
40+
|
41+
LL | let a = &mut foo[..2];
42+
| --- mutable borrow occurs here
43+
LL | let b = &foo[2..];
44+
| ^^^ immutable borrow occurs here
45+
LL | a[0] = 5;
46+
| ---- mutable borrow later used here
47+
|
48+
= help: use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices
49+
50+
error[E0502]: cannot borrow `foo[_]` as mutable because it is also borrowed as immutable
51+
--> $DIR/suggest-split-at-mut.rs:38:13
52+
|
53+
LL | let a = &foo[1];
54+
| ------- immutable borrow occurs here
55+
LL | let b = &mut foo[2];
56+
| ^^^^^^^^^^^ mutable borrow occurs here
57+
LL | *b = 6;
58+
LL | println!("{:?} {:?}", a, b);
59+
| - immutable borrow later used here
60+
|
61+
= help: use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices
62+
63+
error[E0502]: cannot borrow `foo[_]` as immutable because it is also borrowed as mutable
64+
--> $DIR/suggest-split-at-mut.rs:46:13
65+
|
66+
LL | let a = &mut foo[1];
67+
| ----------- mutable borrow occurs here
68+
LL | let b = &foo[2];
69+
| ^^^^^^^ immutable borrow occurs here
70+
LL | *a = 5;
71+
| ------ mutable borrow later used here
72+
|
73+
= help: use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices
74+
75+
error[E0502]: cannot borrow `foo` as immutable because it is also borrowed as mutable
76+
--> $DIR/suggest-split-at-mut.rs:54:14
77+
|
78+
LL | let a = &mut foo[0..];
79+
| --- mutable borrow occurs here
80+
LL | let b = &foo[0..];
81+
| ^^^ immutable borrow occurs here
82+
LL | a[0] = 5;
83+
| ---- mutable borrow later used here
84+
85+
error: aborting due to 7 previous errors
1586

16-
For more information about this error, try `rustc --explain E0499`.
87+
Some errors have detailed explanations: E0499, E0502.
88+
For more information about an error, try `rustc --explain E0499`.

0 commit comments

Comments
 (0)