Auto merge of #52948 - davidtwco:issue-52633-later-loop-iteration, r=pnkfelix

NLL: Better Diagnostic When "Later" means "A Future Loop Iteration"

Part of #52663.

r? @pnkfelix
This commit is contained in:
bors 2018-08-03 09:22:11 +00:00
commit e415b5ecc0
6 changed files with 92 additions and 13 deletions

View file

@ -11,7 +11,7 @@
use borrow_check::borrow_set::BorrowData; use borrow_check::borrow_set::BorrowData;
use borrow_check::nll::region_infer::Cause; use borrow_check::nll::region_infer::Cause;
use borrow_check::{Context, MirBorrowckCtxt, WriteKind}; use borrow_check::{Context, MirBorrowckCtxt, WriteKind};
use rustc::mir::Place; use rustc::mir::{Location, Place, TerminatorKind};
use rustc_errors::DiagnosticBuilder; use rustc_errors::DiagnosticBuilder;
mod find_use; mod find_use;
@ -63,10 +63,17 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
match find_use::find(mir, regioncx, tcx, region_sub, context.loc) { match find_use::find(mir, regioncx, tcx, region_sub, context.loc) {
Some(Cause::LiveVar(_local, location)) => { Some(Cause::LiveVar(_local, location)) => {
err.span_label( if self.is_borrow_location_in_loop(context.loc) {
mir.source_info(location).span, err.span_label(
"borrow later used here".to_string(), mir.source_info(location).span,
); "borrow used here in later iteration of loop".to_string(),
);
} else {
err.span_label(
mir.source_info(location).span,
"borrow later used here".to_string(),
);
}
} }
Some(Cause::DropVar(local, location)) => match &mir.local_decls[local].name { Some(Cause::DropVar(local, location)) => match &mir.local_decls[local].name {
@ -107,4 +114,76 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
} }
} }
} }
/// Check if a borrow location is within a loop.
fn is_borrow_location_in_loop(
&self,
borrow_location: Location,
) -> bool {
let mut visited_locations = Vec::new();
let mut pending_locations = vec![ borrow_location ];
debug!("is_in_loop: borrow_location={:?}", borrow_location);
while let Some(location) = pending_locations.pop() {
debug!("is_in_loop: location={:?} pending_locations={:?} visited_locations={:?}",
location, pending_locations, visited_locations);
if location == borrow_location && visited_locations.contains(&borrow_location) {
// We've managed to return to where we started (and this isn't the start of the
// search).
debug!("is_in_loop: found!");
return true;
}
// Skip locations we've been.
if visited_locations.contains(&location) { continue; }
let block = &self.mir.basic_blocks()[location.block];
if location.statement_index == block.statements.len() {
// Add start location of the next blocks to pending locations.
match block.terminator().kind {
TerminatorKind::Goto { target } => {
pending_locations.push(target.start_location());
},
TerminatorKind::SwitchInt { ref targets, .. } => {
for target in targets {
pending_locations.push(target.start_location());
}
},
TerminatorKind::Drop { target, unwind, .. } |
TerminatorKind::DropAndReplace { target, unwind, .. } |
TerminatorKind::Assert { target, cleanup: unwind, .. } |
TerminatorKind::Yield { resume: target, drop: unwind, .. } |
TerminatorKind::FalseUnwind { real_target: target, unwind, .. } => {
pending_locations.push(target.start_location());
if let Some(unwind) = unwind {
pending_locations.push(unwind.start_location());
}
},
TerminatorKind::Call { ref destination, cleanup, .. } => {
if let Some((_, destination)) = destination {
pending_locations.push(destination.start_location());
}
if let Some(cleanup) = cleanup {
pending_locations.push(cleanup.start_location());
}
},
TerminatorKind::FalseEdges { real_target, ref imaginary_targets, .. } => {
pending_locations.push(real_target.start_location());
for target in imaginary_targets {
pending_locations.push(target.start_location());
}
},
_ => {},
}
} else {
// Add the next statement to pending locations.
pending_locations.push(location.successor_within_block());
}
// Keep track of where we have visited.
visited_locations.push(location);
}
false
}
} }

View file

@ -17,7 +17,7 @@ LL | let inner_second = &mut inner_void; //~ ERROR cannot borrow
| ^^^^^^^^^^^^^^^ second mutable borrow occurs here | ^^^^^^^^^^^^^^^ second mutable borrow occurs here
LL | inner_second.use_mut(); LL | inner_second.use_mut();
LL | inner_first.use_mut(); LL | inner_first.use_mut();
| ----------- borrow later used here | ----------- borrow used here in later iteration of loop
error: aborting due to 2 previous errors error: aborting due to 2 previous errors

View file

@ -5,7 +5,7 @@ LL | let v: Vec<&str> = line.split_whitespace().collect();
| ^^^^ borrowed value does not live long enough | ^^^^ borrowed value does not live long enough
LL | //~^ ERROR `line` does not live long enough LL | //~^ ERROR `line` does not live long enough
LL | println!("accumulator before add_assign {:?}", acc.map); LL | println!("accumulator before add_assign {:?}", acc.map);
| ------- borrow later used here | ------- borrow used here in later iteration of loop
... ...
LL | } LL | }
| - `line` dropped here while still borrowed | - `line` dropped here while still borrowed

View file

@ -7,7 +7,7 @@ LL | foo.mutate();
| ^^^^^^^^^^^^ mutable borrow occurs here | ^^^^^^^^^^^^ mutable borrow occurs here
LL | //~^ ERROR cannot borrow `foo` as mutable LL | //~^ ERROR cannot borrow `foo` as mutable
LL | println!("foo={:?}", *string); LL | println!("foo={:?}", *string);
| ------- borrow later used here | ------- borrow used here in later iteration of loop
error: aborting due to previous error error: aborting due to previous error

View file

@ -2,7 +2,7 @@ error[E0597]: `x` does not live long enough
--> $DIR/regions-escape-loop-via-variable.rs:21:13 --> $DIR/regions-escape-loop-via-variable.rs:21:13
| |
LL | let x = 1 + *p; LL | let x = 1 + *p;
| -- borrow later used here | -- borrow used here in later iteration of loop
LL | p = &x; LL | p = &x;
| ^^ borrowed value does not live long enough | ^^ borrowed value does not live long enough
LL | } LL | }

View file

@ -7,7 +7,7 @@ LL | while x < 10 { //~ ERROR cannot use `x` because it was mutably borrowed
| ^ use of borrowed `x` | ^ use of borrowed `x`
LL | let mut z = x; //~ ERROR cannot use `x` because it was mutably borrowed LL | let mut z = x; //~ ERROR cannot use `x` because it was mutably borrowed
LL | _y.push(&mut z); LL | _y.push(&mut z);
| -- borrow later used here | -- borrow used here in later iteration of loop
error[E0503]: cannot use `x` because it was mutably borrowed error[E0503]: cannot use `x` because it was mutably borrowed
--> $DIR/regions-escape-loop-via-vec.rs:16:21 --> $DIR/regions-escape-loop-via-vec.rs:16:21
@ -18,7 +18,7 @@ LL | while x < 10 { //~ ERROR cannot use `x` because it was mutably borrowed
LL | let mut z = x; //~ ERROR cannot use `x` because it was mutably borrowed LL | let mut z = x; //~ ERROR cannot use `x` because it was mutably borrowed
| ^ use of borrowed `x` | ^ use of borrowed `x`
LL | _y.push(&mut z); LL | _y.push(&mut z);
| -- borrow later used here | -- borrow used here in later iteration of loop
error[E0597]: `z` does not live long enough error[E0597]: `z` does not live long enough
--> $DIR/regions-escape-loop-via-vec.rs:17:17 --> $DIR/regions-escape-loop-via-vec.rs:17:17
@ -26,7 +26,7 @@ error[E0597]: `z` does not live long enough
LL | _y.push(&mut z); LL | _y.push(&mut z);
| -- ^^^^^^ borrowed value does not live long enough | -- ^^^^^^ borrowed value does not live long enough
| | | |
| borrow later used here | borrow used here in later iteration of loop
... ...
LL | } LL | }
| - `z` dropped here while still borrowed | - `z` dropped here while still borrowed
@ -38,7 +38,7 @@ LL | let mut _y = vec![&mut x];
| ------ borrow of `x` occurs here | ------ borrow of `x` occurs here
... ...
LL | _y.push(&mut z); LL | _y.push(&mut z);
| -- borrow later used here | -- borrow used here in later iteration of loop
LL | //~^ ERROR `z` does not live long enough LL | //~^ ERROR `z` does not live long enough
LL | x += 1; //~ ERROR cannot assign LL | x += 1; //~ ERROR cannot assign
| ^^^^^^ use of borrowed `x` | ^^^^^^ use of borrowed `x`