Skip to content

Commit f9faf16

Browse files
committed
Suggest .swap() instead of mem::swap() in more cases
1 parent e927184 commit f9faf16

File tree

4 files changed

+127
-7
lines changed

4 files changed

+127
-7
lines changed

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

+92-7
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// ignore-tidy-filelength
2+
13
use either::Either;
24
use rustc_data_structures::captures::Captures;
35
use rustc_data_structures::fx::FxIndexSet;
@@ -24,6 +26,7 @@ use rustc_span::hygiene::DesugaringKind;
2426
use rustc_span::symbol::{kw, sym, Ident};
2527
use rustc_span::{BytePos, Span, Symbol};
2628
use rustc_trait_selection::infer::InferCtxtExt;
29+
use rustc_trait_selection::traits::error_reporting::FindExprBySpan;
2730
use rustc_trait_selection::traits::ObligationCtxt;
2831
use std::iter;
2932

@@ -1295,14 +1298,96 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
12951298
place: Place<'tcx>,
12961299
borrowed_place: Place<'tcx>,
12971300
) {
1298-
if let ([ProjectionElem::Index(_)], [ProjectionElem::Index(_)]) =
1299-
(&place.projection[..], &borrowed_place.projection[..])
1301+
let tcx = self.infcx.tcx;
1302+
let hir = tcx.hir();
1303+
1304+
if let ([ProjectionElem::Index(index1)], [ProjectionElem::Index(index2)])
1305+
| (
1306+
[ProjectionElem::Deref, ProjectionElem::Index(index1)],
1307+
[ProjectionElem::Deref, ProjectionElem::Index(index2)],
1308+
) = (&place.projection[..], &borrowed_place.projection[..])
13001309
{
1301-
err.help(
1302-
"consider using `.split_at_mut(position)` or similar method to obtain \
1303-
two mutable non-overlapping sub-slices",
1304-
)
1305-
.help("consider using `.swap(index_1, index_2)` to swap elements at the specified indices");
1310+
let mut note_default_suggestion = || {
1311+
err.help(
1312+
"consider using `.split_at_mut(position)` or similar method to obtain \
1313+
two mutable non-overlapping sub-slices",
1314+
)
1315+
.help("consider using `.swap(index_1, index_2)` to swap elements at the specified indices");
1316+
};
1317+
1318+
let Some(body_id) = tcx.hir_node(self.mir_hir_id()).body_id() else {
1319+
note_default_suggestion();
1320+
return;
1321+
};
1322+
1323+
let mut expr_finder =
1324+
FindExprBySpan::new(self.body.local_decls[*index1].source_info.span);
1325+
expr_finder.visit_expr(hir.body(body_id).value);
1326+
let Some(index1) = expr_finder.result else {
1327+
note_default_suggestion();
1328+
return;
1329+
};
1330+
1331+
expr_finder = FindExprBySpan::new(self.body.local_decls[*index2].source_info.span);
1332+
expr_finder.visit_expr(hir.body(body_id).value);
1333+
let Some(index2) = expr_finder.result else {
1334+
note_default_suggestion();
1335+
return;
1336+
};
1337+
1338+
let sm = tcx.sess.source_map();
1339+
1340+
let Ok(index1_str) = sm.span_to_snippet(index1.span) else {
1341+
note_default_suggestion();
1342+
return;
1343+
};
1344+
1345+
let Ok(index2_str) = sm.span_to_snippet(index2.span) else {
1346+
note_default_suggestion();
1347+
return;
1348+
};
1349+
1350+
let Some(object) = hir.parent_id_iter(index1.hir_id).find_map(|id| {
1351+
if let hir::Node::Expr(expr) = tcx.hir_node(id)
1352+
&& let hir::ExprKind::Index(obj, ..) = expr.kind
1353+
{
1354+
Some(obj)
1355+
} else {
1356+
None
1357+
}
1358+
}) else {
1359+
note_default_suggestion();
1360+
return;
1361+
};
1362+
1363+
let Ok(obj_str) = sm.span_to_snippet(object.span) else {
1364+
note_default_suggestion();
1365+
return;
1366+
};
1367+
1368+
let Some(swap_call) = hir.parent_id_iter(object.hir_id).find_map(|id| {
1369+
if let hir::Node::Expr(call) = tcx.hir_node(id)
1370+
&& let hir::ExprKind::Call(callee, ..) = call.kind
1371+
&& let hir::ExprKind::Path(qpath) = callee.kind
1372+
&& let hir::QPath::Resolved(None, res) = qpath
1373+
&& let hir::def::Res::Def(_, did) = res.res
1374+
&& tcx.is_diagnostic_item(sym::mem_swap, did)
1375+
{
1376+
Some(call)
1377+
} else {
1378+
None
1379+
}
1380+
}) else {
1381+
note_default_suggestion();
1382+
return;
1383+
};
1384+
1385+
err.span_suggestion(
1386+
swap_call.span,
1387+
"use `.swap()` to swap elements at the specified indices instead",
1388+
format!("{obj_str}.swap({index1_str}, {index2_str})"),
1389+
Applicability::MachineApplicable,
1390+
);
13061391
}
13071392
}
13081393

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// run-rustfix
2+
#![allow(dead_code)]
3+
4+
fn swap(arr: &mut [u32; 2]) {
5+
arr.swap(1, 0);
6+
//~^ ERROR cannot borrow `arr[_]` as mutable more than once at a time
7+
}
8+
9+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// run-rustfix
2+
#![allow(dead_code)]
3+
4+
fn swap(arr: &mut [u32; 2]) {
5+
std::mem::swap(&mut arr[0], &mut arr[1]);
6+
//~^ ERROR cannot borrow `arr[_]` as mutable more than once at a time
7+
}
8+
9+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
error[E0499]: cannot borrow `arr[_]` as mutable more than once at a time
2+
--> $DIR/suggest-slice-swap.rs:5:33
3+
|
4+
LL | std::mem::swap(&mut arr[0], &mut arr[1]);
5+
| -------------- ----------- ^^^^^^^^^^^ second mutable borrow occurs here
6+
| | |
7+
| | first mutable borrow occurs here
8+
| first borrow later used by call
9+
|
10+
help: use `.swap()` to swap elements at the specified indices instead
11+
|
12+
LL | arr.swap(1, 0);
13+
| ~~~~~~~~~~~~~~
14+
15+
error: aborting due to 1 previous error
16+
17+
For more information about this error, try `rustc --explain E0499`.

0 commit comments

Comments
 (0)