Skip to content

Commit

Permalink
Suggest borrowing on fn argument that is impl AsRef
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@estebank
estebank committed May 9, 2024
1 parent e6e262f commit 2df4f7d
Show file tree
Hide file tree
Showing 5 changed files with 224 additions and 19 deletions.
89 changes: 70 additions & 19 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,31 449,81 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
} else {
(None, &[][..], 0)
};
let mut can_suggest_clone = true;
if let Some(def_id) = def_id
&& let node = self.infcx.tcx.hir_node_by_def_id(def_id)
&& let Some(fn_sig) = node.fn_sig()
&& let Some(ident) = node.ident()
&& let Some(pos) = args.iter().position(|arg| arg.hir_id == expr.hir_id)
&& let Some(arg) = fn_sig.decl.inputs.get(pos offset)
{
let mut span: MultiSpan = arg.span.into();
span.push_span_label(
arg.span,
"this parameter takes ownership of the value".to_string(),
);
let descr = match node.fn_kind() {
Some(hir::intravisit::FnKind::ItemFn(..)) | None => "function",
Some(hir::intravisit::FnKind::Method(..)) => "method",
Some(hir::intravisit::FnKind::Closure) => "closure",
};
span.push_span_label(ident.span, format!("in this {descr}"));
err.span_note(
span,
format!(
"consider changing this parameter type in {descr} `{ident}` to borrow \
instead if owning the value isn't necessary",
),
);
let mut is_mut = false;
if let hir::TyKind::Path(hir::QPath::Resolved(None, path)) = arg.kind
&& let Res::Def(DefKind::TyParam, param_def_id) = path.res
&& self
.infcx
.tcx
.predicates_of(def_id)
.instantiate_identity(self.infcx.tcx)
.predicates
.into_iter()
.any(|pred| {
if let ty::ClauseKind::Trait(predicate) = pred.kind().skip_binder()
&& [
self.infcx.tcx.get_diagnostic_item(sym::AsRef),
self.infcx.tcx.get_diagnostic_item(sym::AsMut),
self.infcx.tcx.get_diagnostic_item(sym::Borrow),
self.infcx.tcx.get_diagnostic_item(sym::BorrowMut),
]
.contains(&Some(predicate.def_id()))
&& let ty::Param(param) = predicate.self_ty().kind()
&& let generics = self.infcx.tcx.generics_of(def_id)
&& let param = generics.type_param(*param, self.infcx.tcx)
&& param.def_id == param_def_id
{
if [
self.infcx.tcx.get_diagnostic_item(sym::AsMut),
self.infcx.tcx.get_diagnostic_item(sym::BorrowMut),
]
.contains(&Some(predicate.def_id()))
{
is_mut = true;
}
true
} else {
false
}
})
{
// The type of the argument corresponding to the expression that got moved
// is a type parameter `T`, which is has a `T: AsRef` obligation.
err.span_suggestion_verbose(
expr.span.shrink_to_lo(),
"borrow the value to avoid moving it",
format!("&{}", if is_mut { "mut " } else { "" }),
Applicability::MachineApplicable,
);
can_suggest_clone = is_mut;
} else {
let mut span: MultiSpan = arg.span.into();
span.push_span_label(
arg.span,
"this parameter takes ownership of the value".to_string(),
);
let descr = match node.fn_kind() {
Some(hir::intravisit::FnKind::ItemFn(..)) | None => "function",
Some(hir::intravisit::FnKind::Method(..)) => "method",
Some(hir::intravisit::FnKind::Closure) => "closure",
};
span.push_span_label(ident.span, format!("in this {descr}"));
err.span_note(
span,
format!(
"consider changing this parameter type in {descr} `{ident}` to \
borrow instead if owning the value isn't necessary",
),
);
}
}
let place = &self.move_data.move_paths[mpi].place;
let ty = place.ty(self.body, self.infcx.tcx).ty;
Expand All @@ -491,9 541,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
ClosureKind::Coroutine(CoroutineKind::Desugared(_, CoroutineSource::Block)),
..
} = move_spans
&& can_suggest_clone
{
self.suggest_cloning(err, ty, expr, None, Some(move_spans));
} else if self.suggest_hoisting_call_outside_loop(err, expr) {
} else if self.suggest_hoisting_call_outside_loop(err, expr) && can_suggest_clone {
// The place where the type moves would be misleading to suggest clone.
// #121466
self.suggest_cloning(err, ty, expr, None, Some(move_spans));
Expand Down
1 change: 1 addition & 0 deletions library/core/src/borrow.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 184,7 @@ pub trait Borrow<Borrowed: ?Sized> {
/// an underlying type by providing a mutable reference. See [`Borrow<T>`]
/// for more information on borrowing as another type.
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_diagnostic_item = "BorrowMut"]
pub trait BorrowMut<Borrowed: ?Sized>: Borrow<Borrowed> {
/// Mutably borrows from an owned value.
///
Expand Down
37 changes: 37 additions & 0 deletions tests/ui/moves/moved-value-on-as-ref-arg.fixed
View file
Original file line number Diff line number Diff line change
@@ -0,0 1,37 @@
//@ run-rustfix
#![allow(unused_mut)]
use std::borrow::{Borrow, BorrowMut};
use std::convert::{AsMut, AsRef};
struct Bar;

impl AsRef<Bar> for Bar {
fn as_ref(&self) -> &Bar {
self
}
}

impl AsMut<Bar> for Bar {
fn as_mut(&mut self) -> &mut Bar {
self
}
}

fn foo<T: AsRef<Bar>>(_: T) {}
fn qux<T: AsMut<Bar>>(_: T) {}
fn bat<T: Borrow<T>>(_: T) {}
fn baz<T: BorrowMut<T>>(_: T) {}

pub fn main() {
let bar = Bar;
foo(&bar);
let _baa = bar; //~ ERROR use of moved value
let mut bar = Bar;
qux(&mut bar);
let _baa = bar; //~ ERROR use of moved value
let bar = Bar;
bat(&bar);
let _baa = bar; //~ ERROR use of moved value
let mut bar = Bar;
baz(&mut bar);
let _baa = bar; //~ ERROR use of moved value
}
37 changes: 37 additions & 0 deletions tests/ui/moves/moved-value-on-as-ref-arg.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 1,37 @@
//@ run-rustfix
#![allow(unused_mut)]
use std::borrow::{Borrow, BorrowMut};
use std::convert::{AsMut, AsRef};
struct Bar;

impl AsRef<Bar> for Bar {
fn as_ref(&self) -> &Bar {
self
}
}

impl AsMut<Bar> for Bar {
fn as_mut(&mut self) -> &mut Bar {
self
}
}

fn foo<T: AsRef<Bar>>(_: T) {}
fn qux<T: AsMut<Bar>>(_: T) {}
fn bat<T: Borrow<T>>(_: T) {}
fn baz<T: BorrowMut<T>>(_: T) {}

pub fn main() {
let bar = Bar;
foo(bar);
let _baa = bar; //~ ERROR use of moved value
let mut bar = Bar;
qux(bar);
let _baa = bar; //~ ERROR use of moved value
let bar = Bar;
bat(bar);
let _baa = bar; //~ ERROR use of moved value
let mut bar = Bar;
baz(bar);
let _baa = bar; //~ ERROR use of moved value
}
79 changes: 79 additions & 0 deletions tests/ui/moves/moved-value-on-as-ref-arg.stderr
View file
Original file line number Diff line number Diff line change
@@ -0,0 1,79 @@
error[E0382]: use of moved value: `bar`
--> $DIR/moved-value-on-as-ref-arg.rs:27:16
|
LL | let bar = Bar;
| --- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait
LL | foo(bar);
| --- value moved here
LL | let _baa = bar;
| ^^^ value used here after move
|
help: borrow the value to avoid moving it
|
LL | foo(&bar);
|

error[E0382]: use of moved value: `bar`
--> $DIR/moved-value-on-as-ref-arg.rs:30:16
|
LL | let mut bar = Bar;
| ------- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait
LL | qux(bar);
| --- value moved here
LL | let _baa = bar;
| ^^^ value used here after move
|
note: if `Bar` implemented `Clone`, you could clone the value
--> $DIR/moved-value-on-as-ref-arg.rs:5:1
|
LL | struct Bar;
| ^^^^^^^^^^ consider implementing `Clone` for this type
...
LL | qux(bar);
| --- you could clone this value
help: borrow the value to avoid moving it
|
LL | qux(&mut bar);
|

error[E0382]: use of moved value: `bar`
--> $DIR/moved-value-on-as-ref-arg.rs:33:16
|
LL | let bar = Bar;
| --- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait
LL | bat(bar);
| --- value moved here
LL | let _baa = bar;
| ^^^ value used here after move
|
help: borrow the value to avoid moving it
|
LL | bat(&bar);
|

error[E0382]: use of moved value: `bar`
--> $DIR/moved-value-on-as-ref-arg.rs:36:16
|
LL | let mut bar = Bar;
| ------- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait
LL | baz(bar);
| --- value moved here
LL | let _baa = bar;
| ^^^ value used here after move
|
note: if `Bar` implemented `Clone`, you could clone the value
--> $DIR/moved-value-on-as-ref-arg.rs:5:1
|
LL | struct Bar;
| ^^^^^^^^^^ consider implementing `Clone` for this type
...
LL | baz(bar);
| --- you could clone this value
help: borrow the value to avoid moving it
|
LL | baz(&mut bar);
|

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0382`.

0 comments on commit 2df4f7d

Please sign in to comment.