Skip to content

Commit d42d3fd

Browse files
estebankestebank
committed
Provide more suggestions for cloning immutable bindings
When encountering multiple mutable borrows, suggest cloning and adding derive annotations as needed. ``` error[E0596]: cannot borrow `sm.x` as mutable, as it is behind a `&` reference --> $DIR/accidentally-cloning-ref-borrow-error.rs:32:9 | LL | foo(&mut sm.x); | ^^^^^^^^^ `sm` is a `&` reference, so the data it refers to cannot be borrowed as mutable | help: `Str` doesn't implement `Clone`, so this call clones the reference `&Str` --> $DIR/accidentally-cloning-ref-borrow-error.rs:31:21 | LL | let mut sm = sr.clone(); | ^^^^^^^ help: consider annotating `Str` with `#[derive(Clone)]` | LL + #[derive(Clone)] LL | struct Str { | help: consider specifying this binding's type | LL | let mut sm: &mut Str = sr.clone(); | ++++++++++ ``` ``` error[E0596]: cannot borrow `*inner` as mutable, as it is behind a `&` reference --> $DIR/issue-91206.rs:14:5 | LL | inner.clear(); | ^^^^^ `inner` is a `&` reference, so the data it refers to cannot be borrowed as mutable | help: you can `clone` the `Vec<usize>` value and consume it, but this might not be your desired behavior --> $DIR/issue-91206.rs:11:17 | LL | let inner = client.get_inner_ref(); | ^^^^^^^^^^^^^^^^^^^^^^ help: consider specifying this binding's type | LL | let inner: &mut Vec<usize> = client.get_inner_ref(); | +++++++++++++++++ ```
1 parent 05e6067 commit d42d3fd

9 files changed

+196
-10
lines changed

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

+101-1
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,18 @@ use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed
33
use rustc_hir as hir;
44
use rustc_hir::intravisit::Visitor;
55
use rustc_hir::Node;
6+
use rustc_infer::traits;
67
use rustc_middle::mir::{Mutability, Place, PlaceRef, ProjectionElem};
7-
use rustc_middle::ty::{self, InstanceDef, Ty, TyCtxt};
8+
use rustc_middle::ty::{self, InstanceDef, ToPredicate, Ty, TyCtxt};
89
use rustc_middle::{
910
hir::place::PlaceBase,
1011
mir::{self, BindingForm, Local, LocalDecl, LocalInfo, LocalKind, Location},
1112
};
1213
use rustc_span::symbol::{kw, Symbol};
1314
use rustc_span::{sym, BytePos, DesugaringKind, Span};
1415
use rustc_target::abi::FieldIdx;
16+
use rustc_trait_selection::infer::InferCtxtExt;
17+
use rustc_trait_selection::traits::error_reporting::suggestions::TypeErrCtxtExt;
1518

1619
use crate::diagnostics::BorrowedContentSource;
1720
use crate::util::FindAssignments;
@@ -1213,6 +1216,103 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
12131216
if let Some(hir_id) = hir_id
12141217
&& let Some(hir::Node::Local(local)) = hir_map.find(hir_id)
12151218
{
1219+
let tables = self.infcx.tcx.typeck(def_id.as_local().unwrap());
1220+
if let Some(clone_trait) = self.infcx.tcx.lang_items().clone_trait()
1221+
&& let Some(expr) = local.init
1222+
&& let ty = tables.node_type_opt(expr.hir_id)
1223+
&& let Some(ty) = ty
1224+
&& let ty::Ref(..) = ty.kind()
1225+
{
1226+
match self
1227+
.infcx
1228+
.could_impl_trait(clone_trait, ty.peel_refs(), self.param_env)
1229+
.as_deref()
1230+
{
1231+
Some([]) => {
1232+
// The type implements Clone.
1233+
err.span_help(
1234+
expr.span,
1235+
format!(
1236+
"you can `clone` the `{}` value and consume it, but this \
1237+
might not be your desired behavior",
1238+
ty.peel_refs(),
1239+
),
1240+
);
1241+
}
1242+
None => {
1243+
if let hir::ExprKind::MethodCall(segment, _rcvr, [], span) =
1244+
expr.kind
1245+
&& segment.ident.name == sym::clone
1246+
{
1247+
err.span_help(
1248+
span,
1249+
format!(
1250+
"`{}` doesn't implement `Clone`, so this call clones \
1251+
the reference `{ty}`",
1252+
ty.peel_refs(),
1253+
),
1254+
);
1255+
}
1256+
// The type doesn't implement Clone.
1257+
let trait_ref = ty::Binder::dummy(ty::TraitRef::new(
1258+
self.infcx.tcx,
1259+
clone_trait,
1260+
[ty.peel_refs()],
1261+
));
1262+
let obligation = traits::Obligation::new(
1263+
self.infcx.tcx,
1264+
traits::ObligationCause::dummy(),
1265+
self.param_env,
1266+
trait_ref,
1267+
);
1268+
self.infcx.err_ctxt().suggest_derive(
1269+
&obligation,
1270+
err,
1271+
trait_ref.to_predicate(self.infcx.tcx),
1272+
);
1273+
}
1274+
Some(errors) => {
1275+
if let hir::ExprKind::MethodCall(segment, _rcvr, [], span) =
1276+
expr.kind
1277+
&& segment.ident.name == sym::clone
1278+
{
1279+
err.span_help(
1280+
span,
1281+
format!(
1282+
"`{}` doesn't implement `Clone` because its \
1283+
implementations trait bounds could not be met, so \
1284+
this call clones the reference `{ty}`",
1285+
ty.peel_refs(),
1286+
),
1287+
);
1288+
err.note(format!(
1289+
"the following trait bounds weren't met: {}",
1290+
errors
1291+
.iter()
1292+
.map(|e| e.obligation.predicate.to_string())
1293+
.collect::<Vec<_>>()
1294+
.join("\n"),
1295+
));
1296+
}
1297+
// The type doesn't implement Clone because of unmet obligations.
1298+
for error in errors {
1299+
if let traits::FulfillmentErrorCode::CodeSelectionError(
1300+
traits::SelectionError::Unimplemented,
1301+
) = error.code
1302+
&& let ty::PredicateKind::Clause(ty::ClauseKind::Trait(
1303+
pred,
1304+
)) = error.obligation.predicate.kind().skip_binder()
1305+
{
1306+
self.infcx.err_ctxt().suggest_derive(
1307+
&error.obligation,
1308+
err,
1309+
error.obligation.predicate.kind().rebind(pred),
1310+
);
1311+
}
1312+
}
1313+
}
1314+
}
1315+
}
12161316
let (changing, span, sugg) = match local.ty {
12171317
Some(ty) => ("changing", ty.span, message),
12181318
None => {

‎tests/ui/borrowck/accidentally-cloning-ref-borrow-error.rs

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
#[derive(Debug)]
2+
struct X<T>(T);
3+
4+
impl<T: Clone> Clone for X<T> {
5+
fn clone(&self) -> X<T> {
6+
X(self.0.clone())
7+
}
8+
}
9+
10+
#[derive(Debug)]
11+
struct Y;
12+
13+
#[derive(Debug)]
14+
struct Str {
15+
x: Option<i32>,
16+
}
17+
18+
fn foo(s: &mut Option<i32>) {
19+
if s.is_none() {
20+
*s = Some(0);
21+
}
22+
println!("{:?}", s);
23+
}
24+
25+
fn bar<T: std::fmt::Debug>(s: &mut X<T>) {
26+
println!("{:?}", s);
27+
}
28+
fn main() {
29+
let s = Str { x: None };
30+
let sr = &s;
31+
let mut sm = sr.clone();
32+
foo(&mut sm.x); //~ ERROR cannot borrow `sm.x` as mutable, as it is behind a `&` reference
33+
34+
let x = X(Y);
35+
let xr = &x;
36+
let mut xm = xr.clone();
37+
bar(&mut xm); //~ ERROR cannot borrow data in a `&` reference as mutable
38+
}

‎tests/ui/borrowck/accidentally-cloning-ref-borrow-error.stderr

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
error[E0596]: cannot borrow `sm.x` as mutable, as it is behind a `&` reference
2+
--> $DIR/accidentally-cloning-ref-borrow-error.rs:32:9
3+
|
4+
LL | foo(&mut sm.x);
5+
| ^^^^^^^^^ `sm` is a `&` reference, so the data it refers to cannot be borrowed as mutable
6+
|
7+
help: `Str` doesn't implement `Clone`, so this call clones the reference `&Str`
8+
--> $DIR/accidentally-cloning-ref-borrow-error.rs:31:21
9+
|
10+
LL | let mut sm = sr.clone();
11+
| ^^^^^^^
12+
help: consider annotating `Str` with `#[derive(Clone)]`
13+
|
14+
LL + #[derive(Clone)]
15+
LL | struct Str {
16+
|
17+
help: consider specifying this binding's type
18+
|
19+
LL | let mut sm: &mut Str = sr.clone();
20+
| ++++++++++
21+
22+
error[E0596]: cannot borrow data in a `&` reference as mutable
23+
--> $DIR/accidentally-cloning-ref-borrow-error.rs:37:9
24+
|
25+
LL | bar(&mut xm);
26+
| ^^^^^^^ cannot borrow as mutable
27+
28+
error: aborting due to 2 previous errors
29+
30+
For more information about this error, try `rustc --explain E0596`.

‎tests/ui/borrowck/issue-85765-closure.rs

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ fn main() {
33
let mut test = Vec::new();
44
let rofl: &Vec<Vec<i32>> = &mut test;
55
//~^ HELP consider changing this binding's type
6+
//~| HELP you can `clone` the `Vec<Vec<i32>>` value and consume it, but this might not be your desired behavior
67
rofl.push(Vec::new());
78
//~^ ERROR cannot borrow `*rofl` as mutable, as it is behind a `&` reference
89
//~| NOTE `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable

‎tests/ui/borrowck/issue-85765-closure.stderr

+9-4
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,21 @@
11
error[E0596]: cannot borrow `*rofl` as mutable, as it is behind a `&` reference
2-
--> $DIR/issue-85765-closure.rs:6:9
2+
--> $DIR/issue-85765-closure.rs:7:9
33
|
44
LL | rofl.push(Vec::new());
55
| ^^^^ `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable
66
|
7+
help: you can `clone` the `Vec<Vec<i32>>` value and consume it, but this might not be your desired behavior
8+
--> $DIR/issue-85765-closure.rs:4:36
9+
|
10+
LL | let rofl: &Vec<Vec<i32>> = &mut test;
11+
| ^^^^^^^^^
712
help: consider changing this binding's type
813
|
914
LL | let rofl: &mut Vec<Vec<i32>> = &mut test;
1015
| ~~~~~~~~~~~~~~~~~~
1116

1217
error[E0594]: cannot assign to `*r`, which is behind a `&` reference
13-
--> $DIR/issue-85765-closure.rs:13:9
18+
--> $DIR/issue-85765-closure.rs:14:9
1419
|
1520
LL | *r = 0;
1621
| ^^^^^^ `r` is a `&` reference, so the data it refers to cannot be written
@@ -21,7 +26,7 @@ LL | let r = &mut mutvar;
2126
| +++
2227

2328
error[E0594]: cannot assign to `*x`, which is behind a `&` reference
24-
--> $DIR/issue-85765-closure.rs:20:9
29+
--> $DIR/issue-85765-closure.rs:21:9
2530
|
2631
LL | *x = 1;
2732
| ^^^^^^ `x` is a `&` reference, so the data it refers to cannot be written
@@ -32,7 +37,7 @@ LL | let x: &mut usize = &mut{0};
3237
| ~~~~~~~~~~
3338

3439
error[E0594]: cannot assign to `*y`, which is behind a `&` reference
35-
--> $DIR/issue-85765-closure.rs:27:9
40+
--> $DIR/issue-85765-closure.rs:28:9
3641
|
3742
LL | *y = 1;
3843
| ^^^^^^ `y` is a `&` reference, so the data it refers to cannot be written

‎tests/ui/borrowck/issue-85765.rs

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ fn main() {
22
let mut test = Vec::new();
33
let rofl: &Vec<Vec<i32>> = &mut test;
44
//~^ HELP consider changing this binding's type
5+
//~| HELP you can `clone` the `Vec<Vec<i32>>` value and consume it, but this might not be your desired behavior
56
rofl.push(Vec::new());
67
//~^ ERROR cannot borrow `*rofl` as mutable, as it is behind a `&` reference
78
//~| NOTE `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable

‎tests/ui/borrowck/issue-85765.stderr

+9-4
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,21 @@
11
error[E0596]: cannot borrow `*rofl` as mutable, as it is behind a `&` reference
2-
--> $DIR/issue-85765.rs:5:5
2+
--> $DIR/issue-85765.rs:6:5
33
|
44
LL | rofl.push(Vec::new());
55
| ^^^^ `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable
66
|
7+
help: you can `clone` the `Vec<Vec<i32>>` value and consume it, but this might not be your desired behavior
8+
--> $DIR/issue-85765.rs:3:32
9+
|
10+
LL | let rofl: &Vec<Vec<i32>> = &mut test;
11+
| ^^^^^^^^^
712
help: consider changing this binding's type
813
|
914
LL | let rofl: &mut Vec<Vec<i32>> = &mut test;
1015
| ~~~~~~~~~~~~~~~~~~
1116

1217
error[E0594]: cannot assign to `*r`, which is behind a `&` reference
13-
--> $DIR/issue-85765.rs:12:5
18+
--> $DIR/issue-85765.rs:13:5
1419
|
1520
LL | *r = 0;
1621
| ^^^^^^ `r` is a `&` reference, so the data it refers to cannot be written
@@ -21,7 +26,7 @@ LL | let r = &mut mutvar;
2126
| +++
2227

2328
error[E0594]: cannot assign to `*x`, which is behind a `&` reference
24-
--> $DIR/issue-85765.rs:19:5
29+
--> $DIR/issue-85765.rs:20:5
2530
|
2631
LL | *x = 1;
2732
| ^^^^^^ `x` is a `&` reference, so the data it refers to cannot be written
@@ -32,7 +37,7 @@ LL | let x: &mut usize = &mut{0};
3237
| ~~~~~~~~~~
3338

3439
error[E0594]: cannot assign to `*y`, which is behind a `&` reference
35-
--> $DIR/issue-85765.rs:26:5
40+
--> $DIR/issue-85765.rs:27:5
3641
|
3742
LL | *y = 1;
3843
| ^^^^^^ `y` is a `&` reference, so the data it refers to cannot be written

‎tests/ui/borrowck/issue-91206.rs

+1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ fn main() {
1010
let client = TestClient;
1111
let inner = client.get_inner_ref();
1212
//~^ HELP consider specifying this binding's type
13+
//~| HELP you can `clone` the `Vec<usize>` value and consume it, but this might not be your desired behavior
1314
inner.clear();
1415
//~^ ERROR cannot borrow `*inner` as mutable, as it is behind a `&` reference [E0596]
1516
//~| NOTE `inner` is a `&` reference, so the data it refers to cannot be borrowed as mutable

‎tests/ui/borrowck/issue-91206.stderr

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
error[E0596]: cannot borrow `*inner` as mutable, as it is behind a `&` reference
2-
--> $DIR/issue-91206.rs:13:5
2+
--> $DIR/issue-91206.rs:14:5
33
|
44
LL | inner.clear();
55
| ^^^^^ `inner` is a `&` reference, so the data it refers to cannot be borrowed as mutable
66
|
7+
help: you can `clone` the `Vec<usize>` value and consume it, but this might not be your desired behavior
8+
--> $DIR/issue-91206.rs:11:17
9+
|
10+
LL | let inner = client.get_inner_ref();
11+
| ^^^^^^^^^^^^^^^^^^^^^^
712
help: consider specifying this binding's type
813
|
914
LL | let inner: &mut Vec<usize> = client.get_inner_ref();

0 commit comments

Comments
 (0)