Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 1 commit into from
Mar 29, 2019
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 37 additions & 10 deletions src/librustc_mir/borrow_check/error_reporting.rs
View file
Open in desktop
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),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this will only work if &T implements IntoIterator-- can we check for this?

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
View file
Open in desktop
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
View file
Open in desktop
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
9 changes: 4 additions & 5 deletions src/test/ui/regions/region-bound-on-closure-outlives-call.nll.stderr
View file
Open in desktop
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
1 change: 0 additions & 1 deletion src/test/ui/span/borrowck-call-is-borrow-issue-12224.nll.stderr
View file
Open in desktop
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
View file
Open in desktop
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