Skip to content

Commit

Permalink
Rollup merge of rust-lang#52948 - davidtwco:issue-52633-later-loop-it…
Browse files Browse the repository at this point in the history
…eration, r=pnkfelix

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

Part of rust-lang#52663.

r? @pnkfelix
  • Loading branch information
cramertj authored Aug 2, 2018
2 parents ac2165a + 1863cb7 commit 13386bc
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 13 deletions.
89 changes: 84 additions & 5 deletions src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
use borrow_check::borrow_set::BorrowData;
use borrow_check::nll::region_infer::Cause;
use borrow_check::{Context, MirBorrowckCtxt, WriteKind};
use rustc::mir::Place;
use rustc::mir::{Location, Place, TerminatorKind};
use rustc_errors::DiagnosticBuilder;

mod find_use;
Expand Down Expand Up @@ -63,10 +63,17 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {

match find_use::find(mir, regioncx, tcx, region_sub, context.loc) {
Some(Cause::LiveVar(_local, location)) => {
err.span_label(
mir.source_info(location).span,
"borrow later used here".to_string(),
);
if self.is_borrow_location_in_loop(context.loc) {
err.span_label(
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 {
Expand Down Expand Up @@ -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
}
}
2 changes: 1 addition & 1 deletion src/test/ui/borrowck/mut-borrow-outside-loop.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ LL | let inner_second = &mut inner_void; //~ ERROR cannot borrow
| ^^^^^^^^^^^^^^^ second mutable borrow occurs here
LL | inner_second.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

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issue-52126-assign-op-invariance.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ LL | let v: Vec<&str> = line.split_whitespace().collect();
| ^^^^ borrowed value does not live long enough
LL | //~^ ERROR `line` does not live long enough
LL | println!("accumulator before add_assign {:?}", acc.map);
| ------- borrow later used here
| ------- borrow used here in later iteration of loop
...
LL | }
| - `line` dropped here while still borrowed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ LL | foo.mutate();
| ^^^^^^^^^^^^ mutable borrow occurs here
LL | //~^ ERROR cannot borrow `foo` as mutable
LL | println!("foo={:?}", *string);
| ------- borrow later used here
| ------- borrow used here in later iteration of loop

error: aborting due to previous error

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0597]: `x` does not live long enough
--> $DIR/regions-escape-loop-via-variable.rs:21:13
|
LL | let x = 1 + *p;
| -- borrow later used here
| -- borrow used here in later iteration of loop
LL | p = &x;
| ^^ borrowed value does not live long enough
LL | }
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/span/regions-escape-loop-via-vec.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ LL | while x < 10 { //~ ERROR cannot use `x` because it was mutably borrowed
| ^ use of borrowed `x`
LL | let mut z = x; //~ ERROR cannot use `x` because it was mutably borrowed
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
--> $DIR/regions-escape-loop-via-vec.rs:16:21
Expand All @@ -18,15 +18,15 @@ 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
| ^ use of borrowed `x`
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
--> $DIR/regions-escape-loop-via-vec.rs:17:17
|
LL | _y.push(&mut z);
| -- ^^^^^^ borrowed value does not live long enough
| |
| borrow later used here
| borrow used here in later iteration of loop
...
LL | }
| - `z` dropped here while still borrowed
Expand All @@ -38,7 +38,7 @@ LL | let mut _y = vec![&mut x];
| ------ borrow of `x` occurs here
...
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 | x += 1; //~ ERROR cannot assign
| ^^^^^^ use of borrowed `x`
Expand Down

0 comments on commit 13386bc

Please sign in to comment.