From 1863cb73720c4c2059757ee27ad1addbb50ab59e Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 1 Aug 2018 17:02:43 +0200 Subject: [PATCH] Errors are more specific in cases where borrows are used in future iterations of loops. --- .../borrow_check/nll/explain_borrow/mod.rs | 89 +++++++++++++++++-- .../mut-borrow-outside-loop.nll.stderr | 2 +- ...ssue-52126-assign-op-invariance.nll.stderr | 2 +- .../borrowck-issue-49631.nll.stderr | 2 +- ...egions-escape-loop-via-variable.nll.stderr | 2 +- .../regions-escape-loop-via-vec.nll.stderr | 8 +- 6 files changed, 92 insertions(+), 13 deletions(-) diff --git a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs index cdb0351d9a8..d98bba72f7a 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs @@ -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; @@ -63,10 +63,17 @@ pub(in borrow_check) fn explain_why_borrow_contains_point( 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 { @@ -107,4 +114,76 @@ pub(in borrow_check) fn explain_why_borrow_contains_point( } } } + + /// 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 + } } diff --git a/src/test/ui/borrowck/mut-borrow-outside-loop.nll.stderr b/src/test/ui/borrowck/mut-borrow-outside-loop.nll.stderr index 55f57e97ba4..02e5b44c17c 100644 --- a/src/test/ui/borrowck/mut-borrow-outside-loop.nll.stderr +++ b/src/test/ui/borrowck/mut-borrow-outside-loop.nll.stderr @@ -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 diff --git a/src/test/ui/issue-52126-assign-op-invariance.nll.stderr b/src/test/ui/issue-52126-assign-op-invariance.nll.stderr index ccbb852b145..e3e389d1197 100644 --- a/src/test/ui/issue-52126-assign-op-invariance.nll.stderr +++ b/src/test/ui/issue-52126-assign-op-invariance.nll.stderr @@ -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 diff --git a/src/test/ui/rfc-2005-default-binding-mode/borrowck-issue-49631.nll.stderr b/src/test/ui/rfc-2005-default-binding-mode/borrowck-issue-49631.nll.stderr index 10384e3b7ca..606d6785422 100644 --- a/src/test/ui/rfc-2005-default-binding-mode/borrowck-issue-49631.nll.stderr +++ b/src/test/ui/rfc-2005-default-binding-mode/borrowck-issue-49631.nll.stderr @@ -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 diff --git a/src/test/ui/span/regions-escape-loop-via-variable.nll.stderr b/src/test/ui/span/regions-escape-loop-via-variable.nll.stderr index 08ca100c247..7aaec700d89 100644 --- a/src/test/ui/span/regions-escape-loop-via-variable.nll.stderr +++ b/src/test/ui/span/regions-escape-loop-via-variable.nll.stderr @@ -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 | } diff --git a/src/test/ui/span/regions-escape-loop-via-vec.nll.stderr b/src/test/ui/span/regions-escape-loop-via-vec.nll.stderr index 4d81211673e..2dc758428ef 100644 --- a/src/test/ui/span/regions-escape-loop-via-vec.nll.stderr +++ b/src/test/ui/span/regions-escape-loop-via-vec.nll.stderr @@ -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 @@ -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 | ^ 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 @@ -26,7 +26,7 @@ error[E0597]: `z` does not live long enough 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 @@ -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` -- 2.44.0