Skip to content

Commit

Permalink
Rollup merge of rust-lang#59429 - estebank:for-loop-move-nll, r=petro…
Browse files Browse the repository at this point in the history
…chenkov

When moving out of a for loop head, suggest borrowing it in nll mode

Follow up to rust-lang#59195 for NLL.
  • Loading branch information
Centril committed Mar 28, 2019
2 parents 06a9196 + 4bad56e commit 0f26958
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 38 deletions.
47 changes: 37 additions & 10 deletions src/librustc_mir/borrow_check/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use rustc_data_structures::indexed_vec::Idx;
use rustc_data_structures::sync::Lrc;
use rustc_errors::{Applicability, DiagnosticBuilder};
use syntax_pos::Span;
use syntax::source_map::CompilerDesugaringKind;

use super::borrow_set::BorrowData;
use super::{Context, MirBorrowckCtxt};
Expand Down Expand Up @@ -154,6 +155,18 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
span,
format!("value moved{} here, in previous iteration of loop", move_msg),
);
if Some(CompilerDesugaringKind::ForLoop) == span.compiler_desugaring_kind() {
if let Ok(snippet) = self.infcx.tcx.sess.source_map()
.span_to_snippet(span)
{
err.span_suggestion(
move_span,
"consider borrowing this to avoid moving it into the for loop",
format!("&{}", snippet),
Applicability::MaybeIncorrect,
);
}
}
is_loop_move = true;
} else if move_site.traversed_back_edge {
err.span_label(
Expand Down Expand Up @@ -291,8 +304,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
format!("move occurs due to use{}", move_spans.describe())
);

self.explain_why_borrow_contains_point(context, borrow, None)
.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
self.explain_why_borrow_contains_point(
context,
borrow,
None,
).add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "", Some(borrow_span));
err.buffer(&mut self.errors_buffer);
}

Expand Down Expand Up @@ -329,7 +345,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
});

self.explain_why_borrow_contains_point(context, borrow, None)
.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "", None);
err.buffer(&mut self.errors_buffer);
}

Expand Down Expand Up @@ -542,8 +558,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
));
}

explanation
.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, first_borrow_desc);
explanation.add_explanation_to_diagnostic(
self.infcx.tcx,
self.mir,
&mut err,
first_borrow_desc,
None,
);

err.buffer(&mut self.errors_buffer);
}
Expand Down Expand Up @@ -866,7 +887,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {

if let BorrowExplanation::MustBeValidFor { .. } = explanation {
} else {
explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
explanation.add_explanation_to_diagnostic(
self.infcx.tcx,
self.mir,
&mut err,
"",
None,
);
}
} else {
err.span_label(borrow_span, "borrowed value does not live long enough");
Expand All @@ -886,7 +913,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
format!("value captured here{}", within),
);

explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "", None);
}

err
Expand Down Expand Up @@ -946,7 +973,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
_ => {}
}

explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "", None);

err.buffer(&mut self.errors_buffer);
}
Expand Down Expand Up @@ -1027,7 +1054,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}
_ => {}
}
explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "", None);

let within = if borrow_spans.for_generator() {
" by generator"
Expand Down Expand Up @@ -1367,7 +1394,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
);

self.explain_why_borrow_contains_point(context, loan, None)
.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "");
.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "", None);

err.buffer(&mut self.errors_buffer);
}
Expand Down
18 changes: 12 additions & 6 deletions src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,23 @@ impl BorrowExplanation {
mir: &Mir<'tcx>,
err: &mut DiagnosticBuilder<'_>,
borrow_desc: &str,
borrow_span: Option<Span>,
) {
match *self {
BorrowExplanation::UsedLater(later_use_kind, var_or_use_span) => {
let message = match later_use_kind {
LaterUseKind::TraitCapture => "borrow later captured here by trait object",
LaterUseKind::ClosureCapture => "borrow later captured here by closure",
LaterUseKind::Call => "borrow later used by call",
LaterUseKind::FakeLetRead => "borrow later stored here",
LaterUseKind::Other => "borrow later used here",
LaterUseKind::TraitCapture => "captured here by trait object",
LaterUseKind::ClosureCapture => "captured here by closure",
LaterUseKind::Call => "used by call",
LaterUseKind::FakeLetRead => "stored here",
LaterUseKind::Other => "used here",
};
err.span_label(var_or_use_span, format!("{}{}", borrow_desc, message));
if !borrow_span.map(|sp| sp.overlaps(var_or_use_span)).unwrap_or(false) {
err.span_label(
var_or_use_span,
format!("{}borrow later {}", borrow_desc, message),
);
}
}
BorrowExplanation::UsedLaterInLoop(later_use_kind, var_or_use_span) => {
let message = match later_use_kind {
Expand Down
17 changes: 5 additions & 12 deletions src/test/ui/augmented-assignments.nll.stderr
Original file line number Diff line number Diff line change
@@ -1,18 +1,11 @@
error[E0505]: cannot move out of `x` because it is borrowed
--> $DIR/augmented-assignments.rs:16:5
|
LL | x
| -
| |
| _____borrow of `x` occurs here
| |
LL | |
LL | | +=
LL | | x;
| | ^
| | |
| |_____move out of `x` occurs here
| borrow later used here
LL | x
| - borrow of `x` occurs here
...
LL | x;
| ^ move out of `x` occurs here

error[E0596]: cannot borrow `y` as mutable, as it is not declared as mutable
--> $DIR/augmented-assignments.rs:21:5
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,10 @@ error[E0505]: cannot move out of `f` because it is borrowed
--> $DIR/region-bound-on-closure-outlives-call.rs:3:25
|
LL | (|x| f(x))(call_rec(f))
| ---------- ^ move out of `f` occurs here
| || |
| || borrow occurs due to use in closure
| |borrow of `f` occurs here
| borrow later used by call
| --- - ^ move out of `f` occurs here
| | |
| | borrow occurs due to use in closure
| borrow of `f` occurs here

error: aborting due to previous error

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ LL | f(Box::new(|a| {
| - ^^^ move out of `f` occurs here
| |
| borrow of `f` occurs here
| borrow later used by call
LL | foo(f);
| - move occurs due to use in closure

Expand Down
9 changes: 5 additions & 4 deletions src/test/ui/suggestions/borrow-for-loop-head.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ error[E0505]: cannot move out of `a` because it is borrowed
--> $DIR/borrow-for-loop-head.rs:4:18
|
LL | for i in &a {
| --
| |
| borrow of `a` occurs here
| borrow later used here
| -- borrow of `a` occurs here
LL | for j in a {
| ^ move out of `a` occurs here

Expand All @@ -17,6 +14,10 @@ LL | let a = vec![1, 2, 3];
LL | for i in &a {
LL | for j in a {
| ^ value moved here, in previous iteration of loop
help: consider borrowing this to avoid moving it into the for loop
|
LL | for j in &a {
| ^^

error: aborting due to 2 previous errors

Expand Down

0 comments on commit 0f26958

Please sign in to comment.