Skip to content

Commit 8de487f

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #124599 - estebank:issue-41708, r=wesleywiser
Suggest borrowing on fn argument that is `impl AsRef` When encountering a move conflict, on an expression that is `!Copy` passed as an argument to an `fn` that is `impl AsRef`, suggest borrowing the expression. ``` error[E0382]: use of moved value: `bar` --> f204.rs:14:15 | 12 | let bar = Bar; | --- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait 13 | foo(bar); | --- value moved here 14 | let baa = bar; | ^^^ value used here after move | help: borrow the value to avoid moving it | 13 | foo(&bar); | + ``` Fix #41708
2 parents c92a8e4 + 2df4f7d commit 8de487f

File tree

5 files changed

+224
-19
lines changed

5 files changed

+224
-19
lines changed

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

Lines changed: 70 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -445,31 +445,81 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
445445
} else {
446446
(None, &[][..], 0)
447447
};
448+
let mut can_suggest_clone = true;
448449
if let Some(def_id) = def_id
449450
&& let node = self.infcx.tcx.hir_node_by_def_id(def_id)
450451
&& let Some(fn_sig) = node.fn_sig()
451452
&& let Some(ident) = node.ident()
452453
&& let Some(pos) = args.iter().position(|arg| arg.hir_id == expr.hir_id)
453454
&& let Some(arg) = fn_sig.decl.inputs.get(pos + offset)
454455
{
455-
let mut span: MultiSpan = arg.span.into();
456-
span.push_span_label(
457-
arg.span,
458-
"this parameter takes ownership of the value".to_string(),
459-
);
460-
let descr = match node.fn_kind() {
461-
Some(hir::intravisit::FnKind::ItemFn(..)) | None => "function",
462-
Some(hir::intravisit::FnKind::Method(..)) => "method",
463-
Some(hir::intravisit::FnKind::Closure) => "closure",
464-
};
465-
span.push_span_label(ident.span, format!("in this {descr}"));
466-
err.span_note(
467-
span,
468-
format!(
469-
"consider changing this parameter type in {descr} `{ident}` to borrow \
470-
instead if owning the value isn't necessary",
471-
),
472-
);
456+
let mut is_mut = false;
457+
if let hir::TyKind::Path(hir::QPath::Resolved(None, path)) = arg.kind
458+
&& let Res::Def(DefKind::TyParam, param_def_id) = path.res
459+
&& self
460+
.infcx
461+
.tcx
462+
.predicates_of(def_id)
463+
.instantiate_identity(self.infcx.tcx)
464+
.predicates
465+
.into_iter()
466+
.any(|pred| {
467+
if let ty::ClauseKind::Trait(predicate) = pred.kind().skip_binder()
468+
&& [
469+
self.infcx.tcx.get_diagnostic_item(sym::AsRef),
470+
self.infcx.tcx.get_diagnostic_item(sym::AsMut),
471+
self.infcx.tcx.get_diagnostic_item(sym::Borrow),
472+
self.infcx.tcx.get_diagnostic_item(sym::BorrowMut),
473+
]
474+
.contains(&Some(predicate.def_id()))
475+
&& let ty::Param(param) = predicate.self_ty().kind()
476+
&& let generics = self.infcx.tcx.generics_of(def_id)
477+
&& let param = generics.type_param(*param, self.infcx.tcx)
478+
&& param.def_id == param_def_id
479+
{
480+
if [
481+
self.infcx.tcx.get_diagnostic_item(sym::AsMut),
482+
self.infcx.tcx.get_diagnostic_item(sym::BorrowMut),
483+
]
484+
.contains(&Some(predicate.def_id()))
485+
{
486+
is_mut = true;
487+
}
488+
true
489+
} else {
490+
false
491+
}
492+
})
493+
{
494+
// The type of the argument corresponding to the expression that got moved
495+
// is a type parameter `T`, which is has a `T: AsRef` obligation.
496+
err.span_suggestion_verbose(
497+
expr.span.shrink_to_lo(),
498+
"borrow the value to avoid moving it",
499+
format!("&{}", if is_mut { "mut " } else { "" }),
500+
Applicability::MachineApplicable,
501+
);
502+
can_suggest_clone = is_mut;
503+
} else {
504+
let mut span: MultiSpan = arg.span.into();
505+
span.push_span_label(
506+
arg.span,
507+
"this parameter takes ownership of the value".to_string(),
508+
);
509+
let descr = match node.fn_kind() {
510+
Some(hir::intravisit::FnKind::ItemFn(..)) | None => "function",
511+
Some(hir::intravisit::FnKind::Method(..)) => "method",
512+
Some(hir::intravisit::FnKind::Closure) => "closure",
513+
};
514+
span.push_span_label(ident.span, format!("in this {descr}"));
515+
err.span_note(
516+
span,
517+
format!(
518+
"consider changing this parameter type in {descr} `{ident}` to \
519+
borrow instead if owning the value isn't necessary",
520+
),
521+
);
522+
}
473523
}
474524
let place = &self.move_data.move_paths[mpi].place;
475525
let ty = place.ty(self.body, self.infcx.tcx).ty;
@@ -487,9 +537,10 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
487537
ClosureKind::Coroutine(CoroutineKind::Desugared(_, CoroutineSource::Block)),
488538
..
489539
} = move_spans
540+
&& can_suggest_clone
490541
{
491542
self.suggest_cloning(err, ty, expr, None, Some(move_spans));
492-
} else if self.suggest_hoisting_call_outside_loop(err, expr) {
543+
} else if self.suggest_hoisting_call_outside_loop(err, expr) && can_suggest_clone {
493544
// The place where the type moves would be misleading to suggest clone.
494545
// #121466
495546
self.suggest_cloning(err, ty, expr, None, Some(move_spans));

‎library/core/src/borrow.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ pub trait Borrow<Borrowed: ?Sized> {
184184
/// an underlying type by providing a mutable reference. See [`Borrow<T>`]
185185
/// for more information on borrowing as another type.
186186
#[stable(feature = "rust1", since = "1.0.0")]
187+
#[rustc_diagnostic_item = "BorrowMut"]
187188
pub trait BorrowMut<Borrowed: ?Sized>: Borrow<Borrowed> {
188189
/// Mutably borrows from an owned value.
189190
///

‎tests/ui/moves/moved-value-on-as-ref-arg.fixed‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
//@ run-rustfix
2+
#![allow(unused_mut)]
3+
use std::borrow::{Borrow, BorrowMut};
4+
use std::convert::{AsMut, AsRef};
5+
struct Bar;
6+
7+
impl AsRef<Bar> for Bar {
8+
fn as_ref(&self) -> &Bar {
9+
self
10+
}
11+
}
12+
13+
impl AsMut<Bar> for Bar {
14+
fn as_mut(&mut self) -> &mut Bar {
15+
self
16+
}
17+
}
18+
19+
fn foo<T: AsRef<Bar>>(_: T) {}
20+
fn qux<T: AsMut<Bar>>(_: T) {}
21+
fn bat<T: Borrow<T>>(_: T) {}
22+
fn baz<T: BorrowMut<T>>(_: T) {}
23+
24+
pub fn main() {
25+
let bar = Bar;
26+
foo(&bar);
27+
let _baa = bar; //~ ERROR use of moved value
28+
let mut bar = Bar;
29+
qux(&mut bar);
30+
let _baa = bar; //~ ERROR use of moved value
31+
let bar = Bar;
32+
bat(&bar);
33+
let _baa = bar; //~ ERROR use of moved value
34+
let mut bar = Bar;
35+
baz(&mut bar);
36+
let _baa = bar; //~ ERROR use of moved value
37+
}

‎tests/ui/moves/moved-value-on-as-ref-arg.rs‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
//@ run-rustfix
2+
#![allow(unused_mut)]
3+
use std::borrow::{Borrow, BorrowMut};
4+
use std::convert::{AsMut, AsRef};
5+
struct Bar;
6+
7+
impl AsRef<Bar> for Bar {
8+
fn as_ref(&self) -> &Bar {
9+
self
10+
}
11+
}
12+
13+
impl AsMut<Bar> for Bar {
14+
fn as_mut(&mut self) -> &mut Bar {
15+
self
16+
}
17+
}
18+
19+
fn foo<T: AsRef<Bar>>(_: T) {}
20+
fn qux<T: AsMut<Bar>>(_: T) {}
21+
fn bat<T: Borrow<T>>(_: T) {}
22+
fn baz<T: BorrowMut<T>>(_: T) {}
23+
24+
pub fn main() {
25+
let bar = Bar;
26+
foo(bar);
27+
let _baa = bar; //~ ERROR use of moved value
28+
let mut bar = Bar;
29+
qux(bar);
30+
let _baa = bar; //~ ERROR use of moved value
31+
let bar = Bar;
32+
bat(bar);
33+
let _baa = bar; //~ ERROR use of moved value
34+
let mut bar = Bar;
35+
baz(bar);
36+
let _baa = bar; //~ ERROR use of moved value
37+
}

‎tests/ui/moves/moved-value-on-as-ref-arg.stderr‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
error[E0382]: use of moved value: `bar`
2+
--> $DIR/moved-value-on-as-ref-arg.rs:27:16
3+
|
4+
LL | let bar = Bar;
5+
| --- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait
6+
LL | foo(bar);
7+
| --- value moved here
8+
LL | let _baa = bar;
9+
| ^^^ value used here after move
10+
|
11+
help: borrow the value to avoid moving it
12+
|
13+
LL | foo(&bar);
14+
| +
15+
16+
error[E0382]: use of moved value: `bar`
17+
--> $DIR/moved-value-on-as-ref-arg.rs:30:16
18+
|
19+
LL | let mut bar = Bar;
20+
| ------- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait
21+
LL | qux(bar);
22+
| --- value moved here
23+
LL | let _baa = bar;
24+
| ^^^ value used here after move
25+
|
26+
note: if `Bar` implemented `Clone`, you could clone the value
27+
--> $DIR/moved-value-on-as-ref-arg.rs:5:1
28+
|
29+
LL | struct Bar;
30+
| ^^^^^^^^^^ consider implementing `Clone` for this type
31+
...
32+
LL | qux(bar);
33+
| --- you could clone this value
34+
help: borrow the value to avoid moving it
35+
|
36+
LL | qux(&mut bar);
37+
| ++++
38+
39+
error[E0382]: use of moved value: `bar`
40+
--> $DIR/moved-value-on-as-ref-arg.rs:33:16
41+
|
42+
LL | let bar = Bar;
43+
| --- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait
44+
LL | bat(bar);
45+
| --- value moved here
46+
LL | let _baa = bar;
47+
| ^^^ value used here after move
48+
|
49+
help: borrow the value to avoid moving it
50+
|
51+
LL | bat(&bar);
52+
| +
53+
54+
error[E0382]: use of moved value: `bar`
55+
--> $DIR/moved-value-on-as-ref-arg.rs:36:16
56+
|
57+
LL | let mut bar = Bar;
58+
| ------- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait
59+
LL | baz(bar);
60+
| --- value moved here
61+
LL | let _baa = bar;
62+
| ^^^ value used here after move
63+
|
64+
note: if `Bar` implemented `Clone`, you could clone the value
65+
--> $DIR/moved-value-on-as-ref-arg.rs:5:1
66+
|
67+
LL | struct Bar;
68+
| ^^^^^^^^^^ consider implementing `Clone` for this type
69+
...
70+
LL | baz(bar);
71+
| --- you could clone this value
72+
help: borrow the value to avoid moving it
73+
|
74+
LL | baz(&mut bar);
75+
| ++++
76+
77+
error: aborting due to 4 previous errors
78+
79+
For more information about this error, try `rustc --explain E0382`.

0 commit comments

Comments
 (0)