Skip to content

Commit

Permalink
Rollup merge of rust-lang#73534 - estebank:borrowck-suggestions, r=ma…
Browse files Browse the repository at this point in the history
…tthewjasper

Provide suggestions for some moved value errors

When encountering an used moved value where the previous move happened
in a `match` or `if let` pattern, suggest using `ref`. Fix rust-lang#63988.

When encountering a `&mut` value that is used in multiple iterations of
a loop, suggest reborrowing it with `&mut *`. Fix rust-lang#62112.
  • Loading branch information
Manishearth authored Jun 21, 2020
2 parents cb81d38 + 5faf657 commit 62ffd20
Show file tree
Hide file tree
Showing 15 changed files with 306 additions and 8 deletions.
38 changes: 34 additions & 4 deletions src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,19 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
&mut err,
format!("variable moved due to use{}", move_spans.describe()),
);
if let UseSpans::PatUse(span) = move_spans {
err.span_suggestion_verbose(
span.shrink_to_lo(),
&format!(
"borrow this field in the pattern to avoid moving {}",
self.describe_place(moved_place.as_ref())
.map(|n| format!("`{}`", n))
.unwrap_or_else(|| "the value".to_string())
),
"ref ".to_string(),
Applicability::MachineApplicable,
);
}
}
}
if let Some(DesugaringKind::ForLoop(_)) = move_span.desugaring_kind() {
Expand Down Expand Up @@ -256,11 +269,28 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
_ => true,
};

if needs_note {
let mpi = self.move_data.moves[move_out_indices[0]].path;
let place = &self.move_data.move_paths[mpi].place;
let mpi = self.move_data.moves[move_out_indices[0]].path;
let place = &self.move_data.move_paths[mpi].place;
let ty = place.ty(self.body, self.infcx.tcx).ty;

if is_loop_move {
if let ty::Ref(_, _, hir::Mutability::Mut) = ty.kind {
// We have a `&mut` ref, we need to reborrow on each iteration (#62112).
err.span_suggestion_verbose(
span.shrink_to_lo(),
&format!(
"consider creating a fresh reborrow of {} here",
self.describe_place(moved_place)
.map(|n| format!("`{}`", n))
.unwrap_or_else(|| "the mutable reference".to_string()),
),
"&mut *".to_string(),
Applicability::MachineApplicable,
);
}
}

let ty = place.ty(self.body, self.infcx.tcx).ty;
if needs_note {
let opt_name =
self.describe_place_with_options(place.as_ref(), IncludingDowncast(true));
let note_msg = match opt_name {
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
// Used in a closure.
(LaterUseKind::ClosureCapture, var_span)
}
UseSpans::OtherUse(span) | UseSpans::FnSelfUse { var_span: span, .. } => {
UseSpans::PatUse(span)
| UseSpans::OtherUse(span)
| UseSpans::FnSelfUse { var_span: span, .. } => {
let block = &self.body.basic_blocks()[location.block];

let kind = if let Some(&Statement {
Expand Down
16 changes: 13 additions & 3 deletions src/librustc_mir/borrow_check/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,9 @@ pub(super) enum UseSpans {
fn_span: Span,
kind: FnSelfUseKind,
},
// This access has a single span associated to it: common case.
/// This access is caused by a `match` or `if let` pattern.
PatUse(Span),
/// This access has a single span associated to it: common case.
OtherUse(Span),
}

Expand All @@ -577,6 +579,7 @@ impl UseSpans {
match self {
UseSpans::ClosureUse { args_span: span, .. }
| UseSpans::FnSelfUse { var_span: span, .. }
| UseSpans::PatUse(span)
| UseSpans::OtherUse(span) => span,
}
}
Expand All @@ -585,6 +588,7 @@ impl UseSpans {
match self {
UseSpans::ClosureUse { var_span: span, .. }
| UseSpans::FnSelfUse { var_span: span, .. }
| UseSpans::PatUse(span)
| UseSpans::OtherUse(span) => span,
}
}
Expand Down Expand Up @@ -655,7 +659,7 @@ impl UseSpans {
match self {
closure @ UseSpans::ClosureUse { .. } => closure,
fn_self @ UseSpans::FnSelfUse { .. } => fn_self,
UseSpans::OtherUse(_) => if_other(),
UseSpans::PatUse(_) | UseSpans::OtherUse(_) => if_other(),
}
}
}
Expand Down Expand Up @@ -772,7 +776,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}

let normal_ret = OtherUse(stmt.source_info.span);
let normal_ret =
if moved_place.projection.iter().any(|p| matches!(p, ProjectionElem::Downcast(..))) {
PatUse(stmt.source_info.span)
} else {
OtherUse(stmt.source_info.span)
};

// We are trying to find MIR of the form:
// ```
Expand All @@ -792,6 +801,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
_ => return normal_ret,
};

debug!("move_spans: normal_ret = {:?}", normal_ret);
debug!("move_spans: target_temp = {:?}", target_temp);

if let Some(Terminator { kind: TerminatorKind::Call { func, args, fn_span, .. }, .. }) =
Expand Down
23 changes: 23 additions & 0 deletions src/test/ui/borrowck/move-in-pattern-mut.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Issue #63988
#[derive(Debug)]
struct S;
fn foo(_: Option<S>) {}

enum E {
V {
s: S,
}
}
fn bar(_: E) {}

fn main() {
let s = Some(S);
if let Some(mut x) = s {
x = S;
}
foo(s); //~ ERROR use of moved value: `s`
let mut e = E::V { s: S };
let E::V { s: mut x } = e;
x = S;
bar(e); //~ ERROR use of moved value: `e`
}
33 changes: 33 additions & 0 deletions src/test/ui/borrowck/move-in-pattern-mut.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
error[E0382]: use of moved value: `s`
--> $DIR/move-in-pattern-mut.rs:18:9
|
LL | if let Some(mut x) = s {
| ----- value moved here
...
LL | foo(s);
| ^ value used here after partial move
|
= note: move occurs because value has type `S`, which does not implement the `Copy` trait
help: borrow this field in the pattern to avoid moving `s.0`
|
LL | if let Some(ref mut x) = s {
| ^^^

error[E0382]: use of moved value: `e`
--> $DIR/move-in-pattern-mut.rs:22:9
|
LL | let E::V { s: mut x } = e;
| ----- value moved here
LL | x = S;
LL | bar(e);
| ^ value used here after partial move
|
= note: move occurs because value has type `S`, which does not implement the `Copy` trait
help: borrow this field in the pattern to avoid moving `e.s`
|
LL | let E::V { s: ref mut x } = e;
| ^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0382`.
24 changes: 24 additions & 0 deletions src/test/ui/borrowck/move-in-pattern.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// run-rustfix
// Issue #63988
#[derive(Debug)]
struct S;
fn foo(_: Option<S>) {}

enum E {
V {
s: S,
}
}
fn bar(_: E) {}

fn main() {
let s = Some(S);
if let Some(ref x) = s {
let _ = x;
}
foo(s); //~ ERROR use of moved value: `s`
let e = E::V { s: S };
let E::V { s: ref x } = e;
let _ = x;
bar(e); //~ ERROR use of moved value: `e`
}
24 changes: 24 additions & 0 deletions src/test/ui/borrowck/move-in-pattern.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// run-rustfix
// Issue #63988
#[derive(Debug)]
struct S;
fn foo(_: Option<S>) {}

enum E {
V {
s: S,
}
}
fn bar(_: E) {}

fn main() {
let s = Some(S);
if let Some(x) = s {
let _ = x;
}
foo(s); //~ ERROR use of moved value: `s`
let e = E::V { s: S };
let E::V { s: x } = e;
let _ = x;
bar(e); //~ ERROR use of moved value: `e`
}
33 changes: 33 additions & 0 deletions src/test/ui/borrowck/move-in-pattern.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
error[E0382]: use of moved value: `s`
--> $DIR/move-in-pattern.rs:19:9
|
LL | if let Some(x) = s {
| - value moved here
...
LL | foo(s);
| ^ value used here after partial move
|
= note: move occurs because value has type `S`, which does not implement the `Copy` trait
help: borrow this field in the pattern to avoid moving `s.0`
|
LL | if let Some(ref x) = s {
| ^^^

error[E0382]: use of moved value: `e`
--> $DIR/move-in-pattern.rs:23:9
|
LL | let E::V { s: x } = e;
| - value moved here
LL | let _ = x;
LL | bar(e);
| ^ value used here after partial move
|
= note: move occurs because value has type `S`, which does not implement the `Copy` trait
help: borrow this field in the pattern to avoid moving `e.s`
|
LL | let E::V { s: ref x } = e;
| ^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0382`.
35 changes: 35 additions & 0 deletions src/test/ui/borrowck/mut-borrow-in-loop-2.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// run-rustfix
#![allow(dead_code)]

struct Events<R>(R);

struct Other;

pub trait Trait<T> {
fn handle(value: T) -> Self;
}

// Blanket impl. (If you comment this out, compiler figures out that it
// is passing an `&mut` to a method that must be expecting an `&mut`,
// and injects an auto-reborrow.)
impl<T, U> Trait<U> for T where T: From<U> {
fn handle(_: U) -> Self { unimplemented!() }
}

impl<'a, R> Trait<&'a mut Events<R>> for Other {
fn handle(_: &'a mut Events<R>) -> Self { unimplemented!() }
}

fn this_compiles<'a, R>(value: &'a mut Events<R>) {
for _ in 0..3 {
Other::handle(&mut *value);
}
}

fn this_does_not<'a, R>(value: &'a mut Events<R>) {
for _ in 0..3 {
Other::handle(&mut *value); //~ ERROR use of moved value: `value`
}
}

fn main() {}
35 changes: 35 additions & 0 deletions src/test/ui/borrowck/mut-borrow-in-loop-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// run-rustfix
#![allow(dead_code)]

struct Events<R>(R);

struct Other;

pub trait Trait<T> {
fn handle(value: T) -> Self;
}

// Blanket impl. (If you comment this out, compiler figures out that it
// is passing an `&mut` to a method that must be expecting an `&mut`,
// and injects an auto-reborrow.)
impl<T, U> Trait<U> for T where T: From<U> {
fn handle(_: U) -> Self { unimplemented!() }
}

impl<'a, R> Trait<&'a mut Events<R>> for Other {
fn handle(_: &'a mut Events<R>) -> Self { unimplemented!() }
}

fn this_compiles<'a, R>(value: &'a mut Events<R>) {
for _ in 0..3 {
Other::handle(&mut *value);
}
}

fn this_does_not<'a, R>(value: &'a mut Events<R>) {
for _ in 0..3 {
Other::handle(value); //~ ERROR use of moved value: `value`
}
}

fn main() {}
17 changes: 17 additions & 0 deletions src/test/ui/borrowck/mut-borrow-in-loop-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error[E0382]: use of moved value: `value`
--> $DIR/mut-borrow-in-loop-2.rs:31:23
|
LL | fn this_does_not<'a, R>(value: &'a mut Events<R>) {
| ----- move occurs because `value` has type `&mut Events<R>`, which does not implement the `Copy` trait
LL | for _ in 0..3 {
LL | Other::handle(value);
| ^^^^^ value moved here, in previous iteration of loop
|
help: consider creating a fresh reborrow of `value` here
|
LL | Other::handle(&mut *value);
| ^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0382`.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ LL | consume(node) + r
| ^^^^ value used here after partial move
|
= note: move occurs because value has type `std::boxed::Box<List>`, which does not implement the `Copy` trait
help: borrow this field in the pattern to avoid moving `node.next.0`
|
LL | Some(ref right) => consume(right),
| ^^^

error: aborting due to previous error

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ LL | Some(_z @ ref _y) => {}
| value moved here
|
= note: move occurs because value has type `X`, which does not implement the `Copy` trait
help: borrow this field in the pattern to avoid moving `x.0`
|
LL | Some(ref _z @ ref _y) => {}
| ^^^

error[E0382]: borrow of moved value
--> $DIR/bind-by-move-neither-can-live-while-the-other-survives-1.rs:35:19
Expand All @@ -57,6 +61,10 @@ LL | Some(_z @ ref mut _y) => {}
| value moved here
|
= note: move occurs because value has type `X`, which does not implement the `Copy` trait
help: borrow this field in the pattern to avoid moving `x.0`
|
LL | Some(ref _z @ ref mut _y) => {}
| ^^^

error: aborting due to 6 previous errors

Expand Down
Loading

0 comments on commit 62ffd20

Please sign in to comment.