From d3e9881ed16202e3faab65bf13a19e0581849bfe Mon Sep 17 00:00:00 2001 From: Shotaro Yamada Date: Sat, 11 Nov 2017 18:15:26 +0900 Subject: [PATCH] MIR-borrowck: fix diagnostics for closures --- src/librustc/hir/mod.rs | 2 +- src/librustc/mir/mod.rs | 4 +- src/librustc_mir/borrow_check.rs | 96 +++++++++++- .../ui/borrowck/borrowck-closures-two-mut.rs | 62 ++++++++ .../borrowck/borrowck-closures-two-mut.stderr | 146 ++++++++++++++++++ 5 files changed, 301 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/borrowck/borrowck-closures-two-mut.rs create mode 100644 src/test/ui/borrowck/borrowck-closures-two-mut.stderr diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index c9b1d70e7b6..39e222230e5 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1949,7 +1949,7 @@ pub fn descriptive_variant(&self) -> &str { } /// A free variable referred to in a function. -#[derive(Copy, Clone, RustcEncodable, RustcDecodable)] +#[derive(Debug, Copy, Clone, RustcEncodable, RustcDecodable)] pub struct Freevar { /// The variable being accessed free. pub def: Def, diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 0fa40f56196..cd017650633 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -267,10 +267,10 @@ pub fn source_info(&self, location: Location) -> &SourceInfo { let block = &self[location.block]; let stmts = &block.statements; let idx = location.statement_index; - if location.statement_index < stmts.len() { + if idx < stmts.len() { &stmts[idx].source_info } else { - assert!(location.statement_index == stmts.len()); + assert!(idx == stmts.len()); &block.terminator().source_info } } diff --git a/src/librustc_mir/borrow_check.rs b/src/librustc_mir/borrow_check.rs index d2524b306cf..6d4f521e99e 100644 --- a/src/librustc_mir/borrow_check.rs +++ b/src/librustc_mir/borrow_check.rs @@ -1169,8 +1169,70 @@ fn report_use_while_mutably_borrowed(&mut self, err.emit(); } + /// Finds the span of arguments of aclosure (within `maybe_closure_span`) and its usage of + /// the local assigned at `location`. + fn find_closure_span( + &self, + maybe_closure_span: Span, + location: Location, + ) -> Option<(Span, Span)> { + use rustc::hir::ExprClosure; + use rustc::mir::AggregateKind; + + let local = if let StatementKind::Assign(Lvalue::Local(local), _) = + self.mir[location.block].statements[location.statement_index].kind + { + local + } else { + return None; + }; + + for stmt in &self.mir[location.block].statements[location.statement_index + 1..] { + if maybe_closure_span != stmt.source_info.span { + break; + } + + if let StatementKind::Assign(_, Rvalue::Aggregate(ref kind, ref lvs)) = stmt.kind { + if let AggregateKind::Closure(def_id, _) = **kind { + debug!("find_closure_span: found closure {:?}", lvs); + + return if let Some(node_id) = self.tcx.hir.as_local_node_id(def_id) { + let args_span = if let ExprClosure(_, _, _, span, _) = + self.tcx.hir.expect_expr(node_id).node + { + span + } else { + return None; + }; + + self.tcx + .with_freevars(node_id, |freevars| { + for (v, lv) in freevars.iter().zip(lvs) { + if let Operand::Consume(Lvalue::Local(l)) = *lv { + if local == l { + debug!( + "find_closure_span: found captured local {:?}", + l + ); + return Some(v.span); + } + } + } + None + }) + .map(|var_span| (args_span, var_span)) + } else { + None + }; + } + } + } + + None + } + fn report_conflicting_borrow(&mut self, - _context: Context, + context: Context, common_prefix: &Lvalue, (lvalue, span): (&Lvalue, Span), gen_borrow_kind: BorrowKind, @@ -1183,38 +1245,60 @@ fn report_conflicting_borrow(&mut self, let issued_span = self.retrieve_borrow_span(issued_borrow); + let new_closure_span = self.find_closure_span(span, context.loc); + let span = new_closure_span.map(|(args, _)| args).unwrap_or(span); + let old_closure_span = self.find_closure_span(issued_span, issued_borrow.location); + let issued_span = old_closure_span.map(|(args, _)| args).unwrap_or(issued_span); + + let desc_lvalue = self.describe_lvalue(lvalue); + // FIXME: supply non-"" `opt_via` when appropriate let mut err = match (gen_borrow_kind, "immutable", "mutable", issued_borrow.kind, "immutable", "mutable") { (BorrowKind::Shared, lft, _, BorrowKind::Mut, _, rgt) | (BorrowKind::Mut, _, lft, BorrowKind::Shared, rgt, _) => self.tcx.cannot_reborrow_already_borrowed( - span, &self.describe_lvalue(lvalue), "", lft, issued_span, + span, &desc_lvalue, "", lft, issued_span, "it", rgt, "", end_issued_loan_span, Origin::Mir), (BorrowKind::Mut, _, _, BorrowKind::Mut, _, _) => self.tcx.cannot_mutably_borrow_multiply( - span, &self.describe_lvalue(lvalue), "", issued_span, + span, &desc_lvalue, "", issued_span, "", end_issued_loan_span, Origin::Mir), (BorrowKind::Unique, _, _, BorrowKind::Unique, _, _) => self.tcx.cannot_uniquely_borrow_by_two_closures( - span, &self.describe_lvalue(lvalue), issued_span, + span, &desc_lvalue, issued_span, end_issued_loan_span, Origin::Mir), (BorrowKind::Unique, _, _, _, _, _) => self.tcx.cannot_uniquely_borrow_by_one_closure( - span, &self.describe_lvalue(lvalue), "", + span, &desc_lvalue, "", issued_span, "it", "", end_issued_loan_span, Origin::Mir), (_, _, _, BorrowKind::Unique, _, _) => self.tcx.cannot_reborrow_already_uniquely_borrowed( - span, &self.describe_lvalue(lvalue), "it", "", + span, &desc_lvalue, "it", "", issued_span, "", end_issued_loan_span, Origin::Mir), (BorrowKind::Shared, _, _, BorrowKind::Shared, _, _) => unreachable!(), }; + + if let Some((_, var_span)) = old_closure_span { + err.span_label( + var_span, + format!("previous borrow occurs due to use of `{}` in closure", desc_lvalue), + ); + } + + if let Some((_, var_span)) = new_closure_span { + err.span_label( + var_span, + format!("borrow occurs due to use of `{}` in closure", desc_lvalue), + ); + } + err.emit(); } diff --git a/src/test/ui/borrowck/borrowck-closures-two-mut.rs b/src/test/ui/borrowck/borrowck-closures-two-mut.rs new file mode 100644 index 00000000000..4b6f992f097 --- /dev/null +++ b/src/test/ui/borrowck/borrowck-closures-two-mut.rs @@ -0,0 +1,62 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Tests that two closures cannot simultaneously have mutable +// access to the variable, whether that mutable access be used +// for direct assignment or for taking mutable ref. Issue #6801. + +// compile-flags: -Z emit-end-regions -Z borrowck-mir + +#![feature(box_syntax)] + +fn to_fn_mut(f: F) -> F { f } + +fn a() { + let mut x = 3; + let c1 = to_fn_mut(|| x = 4); + let c2 = to_fn_mut(|| x = 5); //~ ERROR cannot borrow `x` as mutable more than once +} + +fn set(x: &mut isize) { + *x = 4; +} + +fn b() { + let mut x = 3; + let c1 = to_fn_mut(|| set(&mut x)); + let c2 = to_fn_mut(|| set(&mut x)); //~ ERROR cannot borrow `x` as mutable more than once +} + +fn c() { + let mut x = 3; + let c1 = to_fn_mut(|| x = 5); + let c2 = to_fn_mut(|| set(&mut x)); //~ ERROR cannot borrow `x` as mutable more than once +} + +fn d() { + let mut x = 3; + let c1 = to_fn_mut(|| x = 5); + let c2 = to_fn_mut(|| { let _y = to_fn_mut(|| set(&mut x)); }); // (nested closure) + //~^ ERROR cannot borrow `x` as mutable more than once +} + +fn g() { + struct Foo { + f: Box + } + + let mut x: Box<_> = box Foo { f: box 3 }; + let c1 = to_fn_mut(|| set(&mut *x.f)); + let c2 = to_fn_mut(|| set(&mut *x.f)); + //~^ ERROR cannot borrow `x` as mutable more than once +} + +fn main() { +} diff --git a/src/test/ui/borrowck/borrowck-closures-two-mut.stderr b/src/test/ui/borrowck/borrowck-closures-two-mut.stderr new file mode 100644 index 00000000000..fc8a7f2ab60 --- /dev/null +++ b/src/test/ui/borrowck/borrowck-closures-two-mut.stderr @@ -0,0 +1,146 @@ +error[E0499]: cannot borrow `x` as mutable more than once at a time (Ast) + --> $DIR/borrowck-closures-two-mut.rs:24:24 + | +23 | let c1 = to_fn_mut(|| x = 4); + | -- - previous borrow occurs due to use of `x` in closure + | | + | first mutable borrow occurs here +24 | let c2 = to_fn_mut(|| x = 5); //~ ERROR cannot borrow `x` as mutable more than once + | ^^ - borrow occurs due to use of `x` in closure + | | + | second mutable borrow occurs here +25 | } + | - first borrow ends here + +error[E0499]: cannot borrow `x` as mutable more than once at a time (Ast) + --> $DIR/borrowck-closures-two-mut.rs:34:24 + | +33 | let c1 = to_fn_mut(|| set(&mut x)); + | -- - previous borrow occurs due to use of `x` in closure + | | + | first mutable borrow occurs here +34 | let c2 = to_fn_mut(|| set(&mut x)); //~ ERROR cannot borrow `x` as mutable more than once + | ^^ - borrow occurs due to use of `x` in closure + | | + | second mutable borrow occurs here +35 | } + | - first borrow ends here + +error[E0499]: cannot borrow `x` as mutable more than once at a time (Ast) + --> $DIR/borrowck-closures-two-mut.rs:40:24 + | +39 | let c1 = to_fn_mut(|| x = 5); + | -- - previous borrow occurs due to use of `x` in closure + | | + | first mutable borrow occurs here +40 | let c2 = to_fn_mut(|| set(&mut x)); //~ ERROR cannot borrow `x` as mutable more than once + | ^^ - borrow occurs due to use of `x` in closure + | | + | second mutable borrow occurs here +41 | } + | - first borrow ends here + +error[E0499]: cannot borrow `x` as mutable more than once at a time (Ast) + --> $DIR/borrowck-closures-two-mut.rs:46:24 + | +45 | let c1 = to_fn_mut(|| x = 5); + | -- - previous borrow occurs due to use of `x` in closure + | | + | first mutable borrow occurs here +46 | let c2 = to_fn_mut(|| { let _y = to_fn_mut(|| set(&mut x)); }); // (nested closure) + | ^^ - borrow occurs due to use of `x` in closure + | | + | second mutable borrow occurs here +47 | //~^ ERROR cannot borrow `x` as mutable more than once +48 | } + | - first borrow ends here + +error[E0499]: cannot borrow `x` as mutable more than once at a time (Ast) + --> $DIR/borrowck-closures-two-mut.rs:57:24 + | +56 | let c1 = to_fn_mut(|| set(&mut *x.f)); + | -- - previous borrow occurs due to use of `x` in closure + | | + | first mutable borrow occurs here +57 | let c2 = to_fn_mut(|| set(&mut *x.f)); + | ^^ - borrow occurs due to use of `x` in closure + | | + | second mutable borrow occurs here +58 | //~^ ERROR cannot borrow `x` as mutable more than once +59 | } + | - first borrow ends here + +error[E0499]: cannot borrow `x` as mutable more than once at a time (Mir) + --> $DIR/borrowck-closures-two-mut.rs:24:24 + | +23 | let c1 = to_fn_mut(|| x = 4); + | -- - previous borrow occurs due to use of `x` in closure + | | + | first mutable borrow occurs here +24 | let c2 = to_fn_mut(|| x = 5); //~ ERROR cannot borrow `x` as mutable more than once + | ^^ - borrow occurs due to use of `x` in closure + | | + | second mutable borrow occurs here +25 | } + | - first borrow ends here + +error[E0499]: cannot borrow `x` as mutable more than once at a time (Mir) + --> $DIR/borrowck-closures-two-mut.rs:34:24 + | +33 | let c1 = to_fn_mut(|| set(&mut x)); + | -- - previous borrow occurs due to use of `x` in closure + | | + | first mutable borrow occurs here +34 | let c2 = to_fn_mut(|| set(&mut x)); //~ ERROR cannot borrow `x` as mutable more than once + | ^^ - borrow occurs due to use of `x` in closure + | | + | second mutable borrow occurs here +35 | } + | - first borrow ends here + +error[E0499]: cannot borrow `x` as mutable more than once at a time (Mir) + --> $DIR/borrowck-closures-two-mut.rs:40:24 + | +39 | let c1 = to_fn_mut(|| x = 5); + | -- - previous borrow occurs due to use of `x` in closure + | | + | first mutable borrow occurs here +40 | let c2 = to_fn_mut(|| set(&mut x)); //~ ERROR cannot borrow `x` as mutable more than once + | ^^ - borrow occurs due to use of `x` in closure + | | + | second mutable borrow occurs here +41 | } + | - first borrow ends here + +error[E0499]: cannot borrow `x` as mutable more than once at a time (Mir) + --> $DIR/borrowck-closures-two-mut.rs:46:24 + | +45 | let c1 = to_fn_mut(|| x = 5); + | -- - previous borrow occurs due to use of `x` in closure + | | + | first mutable borrow occurs here +46 | let c2 = to_fn_mut(|| { let _y = to_fn_mut(|| set(&mut x)); }); // (nested closure) + | ^^ - borrow occurs due to use of `x` in closure + | | + | second mutable borrow occurs here +47 | //~^ ERROR cannot borrow `x` as mutable more than once +48 | } + | - first borrow ends here + +error[E0499]: cannot borrow `x` as mutable more than once at a time (Mir) + --> $DIR/borrowck-closures-two-mut.rs:57:24 + | +56 | let c1 = to_fn_mut(|| set(&mut *x.f)); + | -- - previous borrow occurs due to use of `x` in closure + | | + | first mutable borrow occurs here +57 | let c2 = to_fn_mut(|| set(&mut *x.f)); + | ^^ - borrow occurs due to use of `x` in closure + | | + | second mutable borrow occurs here +58 | //~^ ERROR cannot borrow `x` as mutable more than once +59 | } + | - first borrow ends here + +error: aborting due to 10 previous errors + -- GitLab