Skip to content

Commit

Permalink
Rollup merge of #73534 - estebank:borrowck-suggestions, r=matthewjasper
Browse files Browse the repository at this point in the history
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 #63988.

When encountering a `&mut` value that is used in multiple iterations of
a loop, suggest reborrowing it with `&mut *`. Fix #62112.
  • Loading branch information
Manishearth authored Jun 26, 2020
2 parents 3f5b8c8 + 520461f commit 4a245ae
Show file tree
Hide file tree
Showing 17 changed files with 315 additions and 10 deletions.
39 changes: 35 additions & 4 deletions src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,20 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
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 Some(DesugaringKind::ForLoop) == move_span.desugaring_kind() {
let sess = self.infcx.tcx.sess;
if let Ok(snippet) = sess.source_map().span_to_snippet(move_span) {
Expand Down Expand Up @@ -198,11 +212,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
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
// Used in a closure.
(LaterUseKind::ClosureCapture, var_span)
}
UseSpans::OtherUse(span) => {
UseSpans::PatUse(span) | UseSpans::OtherUse(span) => {
let block = &self.body.basic_blocks()[location.block];

let kind = if let Some(&Statement {
Expand Down
20 changes: 15 additions & 5 deletions src/librustc_mir/borrow_check/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,20 +542,26 @@ pub(super) enum UseSpans {
// The span of the first use of the captured variable inside the closure.
var_span: Span,
},
// 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),
}

impl UseSpans {
pub(super) fn args_or_use(self) -> Span {
match self {
UseSpans::ClosureUse { args_span: span, .. } | UseSpans::OtherUse(span) => span,
UseSpans::ClosureUse { args_span: span, .. }
| UseSpans::PatUse(span)
| UseSpans::OtherUse(span) => span,
}
}

pub(super) fn var_or_use(self) -> Span {
match self {
UseSpans::ClosureUse { var_span: span, .. } | UseSpans::OtherUse(span) => span,
UseSpans::ClosureUse { var_span: span, .. }
| UseSpans::PatUse(span)
| UseSpans::OtherUse(span) => span,
}
}

Expand Down Expand Up @@ -624,7 +630,7 @@ impl UseSpans {
{
match self {
closure @ UseSpans::ClosureUse { .. } => closure,
UseSpans::OtherUse(_) => if_other(),
UseSpans::PatUse(_) | UseSpans::OtherUse(_) => if_other(),
}
}
}
Expand Down Expand Up @@ -741,7 +747,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}

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

/// Finds the span of arguments of a closure (within `maybe_closure_span`)
Expand Down
4 changes: 4 additions & 0 deletions src/test/ui/borrowck/issue-41962.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ LL | if let Some(thing) = maybe {
| ^^^^^ value moved here, in previous iteration of loop
|
= note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
help: borrow this field in the pattern to avoid moving `maybe.0`
|
LL | if let Some(ref thing) = maybe {
| ^^^

error: aborting due to previous error

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
4 changes: 4 additions & 0 deletions src/test/ui/nll/issue-53807.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ LL | if let Some(thing) = maybe {
| ^^^^^ value moved here, in previous iteration of loop
|
= note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
help: borrow this field in the pattern to avoid moving `maybe.0`
|
LL | if let Some(ref thing) = maybe {
| ^^^

error: aborting due to previous error

Expand Down
Loading

0 comments on commit 4a245ae

Please sign in to comment.