From 6b8b75142997b12baf374ef6c4721bec5df62849 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 9 Feb 2014 14:08:03 -0500 Subject: [PATCH] borrowck -- treak borrows from closures like other borrows --- src/librustc/middle/borrowck/check_loans.rs | 301 +++++++++------ .../borrowck/gather_loans/gather_moves.rs | 68 ++-- .../middle/borrowck/gather_loans/lifetime.rs | 46 +-- .../middle/borrowck/gather_loans/mod.rs | 296 ++++++++------- .../borrowck/gather_loans/restrictions.rs | 30 +- src/librustc/middle/borrowck/mod.rs | 356 ++++++++++-------- src/librustc/middle/borrowck/move_data.rs | 10 - 7 files changed, 631 insertions(+), 476 deletions(-) diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index cfd2c0719b0..cb1a803c35a 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -52,9 +52,11 @@ impl<'a> Visitor<()> for CheckLoanCtxt<'a> { fn visit_pat(&mut self, p: &ast::Pat, _: ()) { check_loans_in_pat(self, p); } - fn visit_fn(&mut self, fk: &visit::FnKind, fd: &ast::FnDecl, - b: &ast::Block, s: Span, n: ast::NodeId, _: ()) { - check_loans_in_fn(self, fk, fd, b, s, n); + fn visit_fn(&mut self, _fk: &visit::FnKind, _fd: &ast::FnDecl, + _b: &ast::Block, _s: Span, _n: ast::NodeId, _: ()) { + // Don't process nested items or closures here, + // the outer loop will take care of it. + return; } // FIXME(#10894) should continue recursing @@ -218,9 +220,19 @@ impl<'a> CheckLoanCtxt<'a> { loan2.repr(self.tcx())); // Restrictions that would cause the new loan to be illegal: - let illegal_if = match loan2.mutbl { - MutableMutability => RESTR_FREEZE | RESTR_CLAIM, - ImmutableMutability => RESTR_FREEZE, + let illegal_if = match loan2.kind { + // Look for restrictions against mutation. These are + // generated by all other borrows. + ty::MutBorrow => RESTR_MUTATE, + + // Look for restrictions against freezing (immutable borrows). + // These are generated by `&mut` borrows. + ty::ImmBorrow => RESTR_FREEZE, + + // No matter how the data is borrowed (as `&`, as `&mut`, + // or as `&unique imm`) it will always generate a + // restriction against mutating the data. So look for those. + ty::UniqueImmBorrow => RESTR_MUTATE, }; debug!("illegal_if={:?}", illegal_if); @@ -228,47 +240,107 @@ impl<'a> CheckLoanCtxt<'a> { if !restr.set.intersects(illegal_if) { continue; } if restr.loan_path != loan2.loan_path { continue; } - match (new_loan.mutbl, old_loan.mutbl) { - (_, MutableMutability) => { - let var = self.bccx.loan_path_to_str(new_loan.loan_path); + let old_pronoun = if new_loan.loan_path == old_loan.loan_path { + ~"it" + } else { + format!("`{}`", + self.bccx.loan_path_to_str(old_loan.loan_path)) + }; + + match (new_loan.kind, old_loan.kind) { + (ty::MutBorrow, ty::MutBorrow) => { self.bccx.span_err( new_loan.span, - format!("cannot borrow `{}` because it is already \ - borrowed as mutable", var)); - self.bccx.span_note( - old_loan.span, - format!("previous borrow of `{0}` as mutable occurs \ - here; the mutable borrow prevents subsequent \ - moves, borrows, or modification of `{0}` \ - until the borrow ends", var)); + format!("cannot borrow `{}` as mutable \ + more than once at a time", + self.bccx.loan_path_to_str(new_loan.loan_path))); } - (_, mutability) => { + (ty::UniqueImmBorrow, _) => { + self.bccx.span_err( + new_loan.span, + format!("closure requires unique access to `{}` \ + but {} is already borrowed", + self.bccx.loan_path_to_str(new_loan.loan_path), + old_pronoun)); + } + + (_, ty::UniqueImmBorrow) => { self.bccx.span_err( new_loan.span, format!("cannot borrow `{}` as {} because \ - it is already borrowed as {}", - self.bccx.loan_path_to_str(new_loan.loan_path), - self.bccx.mut_to_str(new_loan.mutbl), - self.bccx.mut_to_str(old_loan.mutbl))); + previous closure requires unique access", + self.bccx.loan_path_to_str(new_loan.loan_path), + new_loan.kind.to_user_str())); + } - let var = self.bccx.loan_path_to_str(new_loan.loan_path); - let mut note = format!("previous borrow of `{}` occurs \ - here", var); - if mutability == ImmutableMutability { - note.push_str(format!("; the immutable borrow prevents \ - subsequent moves or mutable - borrows of `{}` until the - borrow ends", var)); - } - self.bccx.span_note(old_loan.span, note); + (_, _) => { + self.bccx.span_err( + new_loan.span, + format!("cannot borrow `{}` as {} because \ + {} is also borrowed as {}", + self.bccx.loan_path_to_str(new_loan.loan_path), + new_loan.kind.to_user_str(), + old_pronoun, + old_loan.kind.to_user_str())); } } + match new_loan.cause { + ClosureCapture(span) => { + self.bccx.span_note( + span, + format!("borrow occurs due to use of `{}` in closure", + self.bccx.loan_path_to_str(new_loan.loan_path))); + } + _ => { } + } + + let rule_summary = match old_loan.kind { + ty::MutBorrow => { + format!("the mutable borrow prevents subsequent \ + moves, borrows, or modification of `{0}` \ + until the borrow ends", + self.bccx.loan_path_to_str(old_loan.loan_path)) + } + + ty::ImmBorrow => { + format!("the immutable borrow prevents subsequent \ + moves or mutable borrows of `{0}` \ + until the borrow ends", + self.bccx.loan_path_to_str(old_loan.loan_path)) + } + + ty::UniqueImmBorrow => { + format!("the unique capture prevents subsequent \ + moves or borrows of `{0}` \ + until the borrow ends", + self.bccx.loan_path_to_str(old_loan.loan_path)) + } + }; + + let borrow_summary = match old_loan.cause { + ClosureCapture(_) => { + format!("previous borrow of `{}` occurs here due to \ + use in closure", + self.bccx.loan_path_to_str(old_loan.loan_path)) + } + + AddrOf | AutoRef | RefBinding => { + format!("previous borrow of `{}` occurs here", + self.bccx.loan_path_to_str(old_loan.loan_path)) + } + }; + + self.bccx.span_note( + old_loan.span, + format!("{}; {}", borrow_summary, rule_summary)); + let old_loan_span = ast_map::node_span(self.tcx().items, old_loan.kill_scope); self.bccx.span_end_note(old_loan_span, "previous borrow ends here"); + return false; } @@ -349,11 +421,23 @@ impl<'a> CheckLoanCtxt<'a> { } // Otherwise, just a plain error. - self.bccx.span_err( - expr.span, - format!("cannot assign to {} {}", - cmt.mutbl.to_user_str(), - self.bccx.cmt_to_str(cmt))); + match opt_loan_path(cmt) { + Some(lp) => { + self.bccx.span_err( + expr.span, + format!("cannot assign to {} {} `{}`", + cmt.mutbl.to_user_str(), + self.bccx.cmt_to_str(cmt), + self.bccx.loan_path_to_str(lp))); + } + None => { + self.bccx.span_err( + expr.span, + format!("cannot assign to {} {}", + cmt.mutbl.to_user_str(), + self.bccx.cmt_to_str(cmt))); + } + } return; fn mark_variable_as_used_mut(this: &CheckLoanCtxt, @@ -377,11 +461,11 @@ impl<'a> CheckLoanCtxt<'a> { return; } - mc::cat_stack_upvar(b) => { - cmt = b; + mc::cat_upvar(..) => { + return; } - mc::cat_deref(_, _, mc::gc_ptr) => { + mc::cat_deref(_, _, mc::GcPtr) => { assert_eq!(cmt.mutbl, mc::McImmutable); return; } @@ -389,25 +473,22 @@ impl<'a> CheckLoanCtxt<'a> { mc::cat_rvalue(..) | mc::cat_static_item | mc::cat_copied_upvar(..) | - mc::cat_deref(_, _, mc::unsafe_ptr(..)) | - mc::cat_deref(_, _, mc::region_ptr(..)) => { + mc::cat_deref(_, _, mc::UnsafePtr(..)) | + mc::cat_deref(_, _, mc::BorrowedPtr(..)) => { assert_eq!(cmt.mutbl, mc::McDeclared); return; } mc::cat_discr(b, _) | - mc::cat_deref(b, _, mc::uniq_ptr) => { + mc::cat_deref(b, _, mc::OwnedPtr) => { assert_eq!(cmt.mutbl, mc::McInherited); cmt = b; } mc::cat_downcast(b) | mc::cat_interior(b, _) => { - if cmt.mutbl == mc::McInherited { - cmt = b; - } else { - return; // field declared as mutable or some such - } + assert_eq!(cmt.mutbl, mc::McInherited); + cmt = b; } } } @@ -422,7 +503,7 @@ impl<'a> CheckLoanCtxt<'a> { debug!("check_for_aliasable_mutable_writes(cmt={}, guarantor={})", cmt.repr(this.tcx()), guarantor.repr(this.tcx())); match guarantor.cat { - mc::cat_deref(b, _, mc::region_ptr(ast::MutMutable, _)) => { + mc::cat_deref(b, _, mc::BorrowedPtr(ty::MutBorrow, _)) => { // Statically prohibit writes to `&mut` when aliasable check_for_aliasability_violation(this, expr, b); @@ -557,7 +638,7 @@ impl<'a> CheckLoanCtxt<'a> { // with inherited mutability and with `&mut` // pointers. LpExtend(lp_base, mc::McInherited, _) | - LpExtend(lp_base, _, LpDeref(mc::region_ptr(ast::MutMutable, _))) => { + LpExtend(lp_base, _, LpDeref(mc::BorrowedPtr(ty::MutBorrow, _))) => { loan_path = lp_base; } @@ -572,9 +653,7 @@ impl<'a> CheckLoanCtxt<'a> { // Check for a non-const loan of `loan_path` let cont = this.each_in_scope_loan(expr.id, |loan| { if loan.loan_path == loan_path { - this.report_illegal_mutation(expr, - full_loan_path, - loan); + this.report_illegal_mutation(expr, full_loan_path, loan); false } else { true @@ -603,9 +682,10 @@ impl<'a> CheckLoanCtxt<'a> { fn check_move_out_from_expr(&self, expr: &ast::Expr) { match expr.node { ast::ExprFnBlock(..) | ast::ExprProc(..) => { - // moves due to capture clauses are checked - // in `check_loans_in_fn`, so that we can - // give a better error message + // Moves due to captures are checked in + // check_captured_variables() because it allows + // us to give a more precise error message with + // a more precise span. } _ => { self.check_move_out_from_id(expr.id, expr.span) @@ -621,18 +701,59 @@ impl<'a> CheckLoanCtxt<'a> { self.bccx.span_err( span, format!("cannot move out of `{}` \ - because it is borrowed", + because it is borrowed", self.bccx.loan_path_to_str(move_path))); self.bccx.span_note( loan_span, format!("borrow of `{}` occurs here", - self.bccx.loan_path_to_str(loan_path))); + self.bccx.loan_path_to_str(loan_path))); } } true }); } + fn check_captured_variables(&self, + closure_id: ast::NodeId, + span: Span) { + let capture_map = self.bccx.capture_map.borrow(); + let cap_vars = capture_map.get().get(&closure_id); + for cap_var in cap_vars.borrow().iter() { + let var_id = ast_util::def_id_of_def(cap_var.def).node; + let var_path = @LpVar(var_id); + self.check_if_path_is_moved(closure_id, span, + MovedInCapture, var_path); + match cap_var.mode { + moves::CapRef | moves::CapCopy => {} + moves::CapMove => { + check_by_move_capture(self, closure_id, cap_var, var_path); + } + } + } + return; + + fn check_by_move_capture(this: &CheckLoanCtxt, + closure_id: ast::NodeId, + cap_var: &moves::CaptureVar, + move_path: @LoanPath) { + let move_err = this.analyze_move_out_from(closure_id, move_path); + match move_err { + MoveOk => {} + MoveWhileBorrowed(loan_path, loan_span) => { + this.bccx.span_err( + cap_var.span, + format!("cannot move `{}` into closure \ + because it is borrowed", + this.bccx.loan_path_to_str(move_path))); + this.bccx.span_note( + loan_span, + format!("borrow of `{}` occurs here", + this.bccx.loan_path_to_str(loan_path))); + } + } + } + } + pub fn analyze_move_out_from(&self, expr_id: ast::NodeId, mut move_path: @LoanPath) @@ -681,67 +802,6 @@ impl<'a> CheckLoanCtxt<'a> { } } -fn check_loans_in_fn<'a>(this: &mut CheckLoanCtxt<'a>, - fk: &visit::FnKind, - decl: &ast::FnDecl, - body: &ast::Block, - sp: Span, - id: ast::NodeId) { - match *fk { - visit::FkItemFn(..) | visit::FkMethod(..) => { - // Don't process nested items. - return; - } - - visit::FkFnBlock(..) => { - check_captured_variables(this, id, sp); - } - } - - visit::walk_fn(this, fk, decl, body, sp, id, ()); - - fn check_captured_variables(this: &CheckLoanCtxt, - closure_id: ast::NodeId, - span: Span) { - let capture_map = this.bccx.capture_map.borrow(); - let cap_vars = capture_map.get().get(&closure_id); - for cap_var in cap_vars.borrow().iter() { - let var_id = ast_util::def_id_of_def(cap_var.def).node; - let var_path = @LpVar(var_id); - this.check_if_path_is_moved(closure_id, span, - MovedInCapture, var_path); - match cap_var.mode { - moves::CapRef | moves::CapCopy => {} - moves::CapMove => { - check_by_move_capture(this, closure_id, cap_var, var_path); - } - } - } - return; - - fn check_by_move_capture(this: &CheckLoanCtxt, - closure_id: ast::NodeId, - cap_var: &moves::CaptureVar, - move_path: @LoanPath) { - let move_err = this.analyze_move_out_from(closure_id, move_path); - match move_err { - MoveOk => {} - MoveWhileBorrowed(loan_path, loan_span) => { - this.bccx.span_err( - cap_var.span, - format!("cannot move `{}` into closure \ - because it is borrowed", - this.bccx.loan_path_to_str(move_path))); - this.bccx.span_note( - loan_span, - format!("borrow of `{}` occurs here", - this.bccx.loan_path_to_str(loan_path))); - } - } - } - } -} - fn check_loans_in_local<'a>(this: &mut CheckLoanCtxt<'a>, local: &ast::Local) { visit::walk_local(this, local, ()); @@ -769,6 +829,9 @@ fn check_loans_in_expr<'a>(this: &mut CheckLoanCtxt<'a>, } } } + ast::ExprFnBlock(..) | ast::ExprProc(..) => { + this.check_captured_variables(expr.id, expr.span) + } ast::ExprAssign(dest, _) | ast::ExprAssignOp(_, _, dest, _) => { this.check_assignment(dest); diff --git a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs index 0d9b4b0b171..49b12a6db1f 100644 --- a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs +++ b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs @@ -18,9 +18,8 @@ use middle::borrowck::move_data::*; use middle::moves; use middle::ty; use syntax::ast; -use syntax::ast_util; use syntax::codemap::Span; -use util::ppaux::{UserString}; +use util::ppaux::{Repr, UserString}; pub fn gather_decl(bccx: &BorrowckCtxt, move_data: &MoveData, @@ -35,33 +34,14 @@ pub fn gather_move_from_expr(bccx: &BorrowckCtxt, move_data: &MoveData, move_expr: &ast::Expr, cmt: mc::cmt) { - gather_move_from_expr_or_pat(bccx, move_data, move_expr.id, MoveExpr, cmt); + gather_move(bccx, move_data, move_expr.id, MoveExpr, cmt); } pub fn gather_move_from_pat(bccx: &BorrowckCtxt, move_data: &MoveData, move_pat: &ast::Pat, cmt: mc::cmt) { - gather_move_from_expr_or_pat(bccx, move_data, move_pat.id, MovePat, cmt); -} - -fn gather_move_from_expr_or_pat(bccx: &BorrowckCtxt, - move_data: &MoveData, - move_id: ast::NodeId, - move_kind: MoveKind, - cmt: mc::cmt) { - if !check_is_legal_to_move_from(bccx, cmt, cmt) { - return; - } - - match opt_loan_path(cmt) { - Some(loan_path) => { - move_data.add_move(bccx.tcx, loan_path, move_id, move_kind); - } - None => { - // move from rvalue or unsafe pointer, hence ok - } - } + gather_move(bccx, move_data, move_pat.id, MovePat, cmt); } pub fn gather_captures(bccx: &BorrowckCtxt, @@ -72,16 +52,38 @@ pub fn gather_captures(bccx: &BorrowckCtxt, for captured_var in captured_vars.borrow().iter() { match captured_var.mode { moves::CapMove => { - let fvar_id = ast_util::def_id_of_def(captured_var.def).node; - let loan_path = @LpVar(fvar_id); - move_data.add_move(bccx.tcx, loan_path, closure_expr.id, - Captured); + let cmt = bccx.cat_captured_var(closure_expr.id, + closure_expr.span, + captured_var); + gather_move(bccx, move_data, closure_expr.id, Captured, cmt); } moves::CapCopy | moves::CapRef => {} } } } +fn gather_move(bccx: &BorrowckCtxt, + move_data: &MoveData, + move_id: ast::NodeId, + move_kind: MoveKind, + cmt: mc::cmt) { + debug!("gather_move(move_id={}, cmt={})", + move_id, cmt.repr(bccx.tcx)); + + if !check_is_legal_to_move_from(bccx, cmt, cmt) { + return; + } + + match opt_loan_path(cmt) { + Some(loan_path) => { + move_data.add_move(bccx.tcx, loan_path, move_id, move_kind); + } + None => { + // move from rvalue or unsafe pointer, hence ok + } + } +} + pub fn gather_assignment(bccx: &BorrowckCtxt, move_data: &MoveData, assignment_id: ast::NodeId, @@ -99,15 +101,15 @@ fn check_is_legal_to_move_from(bccx: &BorrowckCtxt, cmt0: mc::cmt, cmt: mc::cmt) -> bool { match cmt.cat { - mc::cat_deref(_, _, mc::region_ptr(..)) | - mc::cat_deref(_, _, mc::gc_ptr) | - mc::cat_deref(_, _, mc::unsafe_ptr(..)) | - mc::cat_stack_upvar(..) | + mc::cat_deref(_, _, mc::BorrowedPtr(..)) | + mc::cat_deref(_, _, mc::GcPtr) | + mc::cat_deref(_, _, mc::UnsafePtr(..)) | + mc::cat_upvar(..) | mc::cat_copied_upvar(mc::CopiedUpvar { onceness: ast::Many, .. }) => { bccx.span_err( cmt0.span, format!("cannot move out of {}", - bccx.cmt_to_str(cmt))); + bccx.cmt_to_str(cmt))); false } @@ -158,7 +160,7 @@ fn check_is_legal_to_move_from(bccx: &BorrowckCtxt, } } - mc::cat_deref(b, _, mc::uniq_ptr) | + mc::cat_deref(b, _, mc::OwnedPtr) | mc::cat_discr(b, _) => { check_is_legal_to_move_from(bccx, cmt0, b) } diff --git a/src/librustc/middle/borrowck/gather_loans/lifetime.rs b/src/librustc/middle/borrowck/gather_loans/lifetime.rs index e151f398458..c47affac683 100644 --- a/src/librustc/middle/borrowck/gather_loans/lifetime.rs +++ b/src/librustc/middle/borrowck/gather_loans/lifetime.rs @@ -26,16 +26,19 @@ pub fn guarantee_lifetime(bccx: &BorrowckCtxt, item_scope_id: ast::NodeId, root_scope_id: ast::NodeId, span: Span, + cause: LoanCause, cmt: mc::cmt, loan_region: ty::Region, - loan_mutbl: LoanMutability) -> R { + loan_kind: ty::BorrowKind) + -> Result<(),()> { debug!("guarantee_lifetime(cmt={}, loan_region={})", cmt.repr(bccx.tcx), loan_region.repr(bccx.tcx)); let ctxt = GuaranteeLifetimeContext {bccx: bccx, item_scope_id: item_scope_id, span: span, + cause: cause, loan_region: loan_region, - loan_mutbl: loan_mutbl, + loan_kind: loan_kind, cmt_original: cmt, root_scope_id: root_scope_id}; ctxt.check(cmt, None) @@ -55,8 +58,9 @@ struct GuaranteeLifetimeContext<'a> { root_scope_id: ast::NodeId, span: Span, + cause: LoanCause, loan_region: ty::Region, - loan_mutbl: LoanMutability, + loan_kind: ty::BorrowKind, cmt_original: mc::cmt } @@ -76,21 +80,18 @@ impl<'a> GuaranteeLifetimeContext<'a> { mc::cat_copied_upvar(..) | // L-Local mc::cat_local(..) | // L-Local mc::cat_arg(..) | // L-Local - mc::cat_deref(_, _, mc::region_ptr(..)) | // L-Deref-Borrowed - mc::cat_deref(_, _, mc::unsafe_ptr(..)) => { + mc::cat_upvar(..) | + mc::cat_deref(_, _, mc::BorrowedPtr(..)) | // L-Deref-Borrowed + mc::cat_deref(_, _, mc::UnsafePtr(..)) => { let scope = self.scope(cmt); self.check_scope(scope) } - mc::cat_stack_upvar(cmt) => { - self.check(cmt, discr_scope) - } - mc::cat_static_item => { Ok(()) } - mc::cat_deref(base, derefs, mc::gc_ptr) => { + mc::cat_deref(base, derefs, mc::GcPtr) => { let base_scope = self.scope(base); // L-Deref-Managed-Imm-User-Root @@ -112,7 +113,7 @@ impl<'a> GuaranteeLifetimeContext<'a> { } mc::cat_downcast(base) | - mc::cat_deref(base, _, mc::uniq_ptr) | // L-Deref-Send + mc::cat_deref(base, _, mc::OwnedPtr) | // L-Deref-Send mc::cat_interior(base, _) => { // L-Field self.check(base, discr_scope) } @@ -269,12 +270,12 @@ impl<'a> GuaranteeLifetimeContext<'a> { mc::cat_rvalue(..) | mc::cat_static_item | mc::cat_copied_upvar(..) | - mc::cat_deref(..) => { + mc::cat_deref(..) | + mc::cat_upvar(..) => { false } r @ mc::cat_downcast(..) | r @ mc::cat_interior(..) | - r @ mc::cat_stack_upvar(..) | r @ mc::cat_discr(..) => { self.tcx().sess.span_bug( cmt.span, @@ -294,6 +295,7 @@ impl<'a> GuaranteeLifetimeContext<'a> { mc::cat_rvalue(temp_scope) => { temp_scope } + mc::cat_upvar(..) | mc::cat_copied_upvar(_) => { ty::ReScope(self.item_scope_id) } @@ -304,17 +306,16 @@ impl<'a> GuaranteeLifetimeContext<'a> { mc::cat_arg(local_id) => { ty::ReScope(self.bccx.tcx.region_maps.var_scope(local_id)) } - mc::cat_deref(_, _, mc::unsafe_ptr(..)) => { + mc::cat_deref(_, _, mc::UnsafePtr(..)) => { ty::ReStatic } - mc::cat_deref(_, _, mc::region_ptr(_, r)) => { + mc::cat_deref(_, _, mc::BorrowedPtr(_, r)) => { r } mc::cat_downcast(cmt) | - mc::cat_deref(cmt, _, mc::uniq_ptr) | - mc::cat_deref(cmt, _, mc::gc_ptr) | + mc::cat_deref(cmt, _, mc::OwnedPtr) | + mc::cat_deref(cmt, _, mc::GcPtr) | mc::cat_interior(cmt, _) | - mc::cat_stack_upvar(cmt) | mc::cat_discr(cmt, _) => { self.scope(cmt) } @@ -322,10 +323,9 @@ impl<'a> GuaranteeLifetimeContext<'a> { } fn report_error(&self, code: bckerr_code) { - self.bccx.report(BckError { - cmt: self.cmt_original, - span: self.span, - code: code - }); + self.bccx.report(BckError { cmt: self.cmt_original, + span: self.span, + cause: self.cause, + code: code }); } } diff --git a/src/librustc/middle/borrowck/gather_loans/mod.rs b/src/librustc/middle/borrowck/gather_loans/mod.rs index 957073ac392..c6a77988bce 100644 --- a/src/librustc/middle/borrowck/gather_loans/mod.rs +++ b/src/librustc/middle/borrowck/gather_loans/mod.rs @@ -16,10 +16,10 @@ // their associated scopes. In phase two, checking loans, we will then make // sure that all of these loans are honored. - use middle::borrowck::*; use middle::borrowck::move_data::MoveData; use mc = middle::mem_categorization; +use middle::moves; use middle::pat_util; use middle::ty::{ty_region}; use middle::ty; @@ -28,6 +28,7 @@ use util::ppaux::{Repr}; use std::cell::RefCell; use syntax::ast; +use syntax::ast_util; use syntax::ast_util::IdRange; use syntax::codemap::Span; use syntax::print::pprust; @@ -127,22 +128,15 @@ fn add_pat_to_id_range(this: &mut GatherLoanCtxt, visit::walk_pat(this, p, ()); } -fn gather_loans_in_fn(this: &mut GatherLoanCtxt, fk: &FnKind, - decl: &ast::FnDecl, body: &ast::Block, - sp: Span, id: ast::NodeId) { - match fk { - &visit::FkItemFn(..) | &visit::FkMethod(..) => { - fail!("cannot occur, due to visit_item override"); - } - - // Visit closures as part of the containing item. - &visit::FkFnBlock(..) => { - this.push_repeating_id(body.id); - visit::walk_fn(this, fk, decl, body, sp, id, ()); - this.pop_repeating_id(body.id); - this.gather_fn_arg_patterns(decl, body); - } - } +fn gather_loans_in_fn(_v: &mut GatherLoanCtxt, + _fk: &FnKind, + _decl: &ast::FnDecl, + _body: &ast::Block, + _sp: Span, + _id: ast::NodeId) { + // Do not visit closures or fn items here, the outer loop in + // borrowck/mod will visit them for us in turn. + return; } fn gather_loans_in_block(this: &mut GatherLoanCtxt, @@ -232,8 +226,9 @@ fn gather_loans_in_expr(this: &mut GatherLoanCtxt, this.guarantee_valid(ex.id, ex.span, base_cmt, - LoanMutability::from_ast_mutability(mutbl), - scope_r); + mutbl, + scope_r, + AddrOf); } visit::walk_expr(this, ex, ()); } @@ -278,8 +273,9 @@ fn gather_loans_in_expr(this: &mut GatherLoanCtxt, this.guarantee_valid(arg.id, arg.span, arg_cmt, - ImmutableMutability, - scope_r); + ast::MutImmutable, + scope_r, + AutoRef); visit::walk_expr(this, ex, ()); } @@ -305,6 +301,7 @@ fn gather_loans_in_expr(this: &mut GatherLoanCtxt, ast::ExprFnBlock(..) | ast::ExprProc(..) => { gather_moves::gather_captures(this.bccx, &this.move_data, ex); + this.guarantee_captures(ex); visit::walk_expr(this, ex, ()); } @@ -367,49 +364,48 @@ impl<'a> GatherLoanCtxt<'a> { ty::AutoDerefRef { autoref: Some(ref autoref), autoderefs: autoderefs}) => { - let mcx = &mc::mem_categorization_ctxt { - tcx: self.tcx(), - method_map: self.bccx.method_map}; - let cmt = mcx.cat_expr_autoderefd(expr, autoderefs); + let mut mc = self.bccx.mc(); + let cmt = match mc.cat_expr_autoderefd(expr, autoderefs) { + Ok(v) => v, + Err(()) => self.tcx().sess.span_bug(expr.span, "Err from mc") + }; debug!("after autoderef, cmt={}", cmt.repr(self.tcx())); match *autoref { ty::AutoPtr(r, m) => { - let loan_mutability = - LoanMutability::from_ast_mutability(m); self.guarantee_valid(expr.id, expr.span, cmt, - loan_mutability, - r) + m, + r, + AutoRef) } ty::AutoBorrowVec(r, m) | ty::AutoBorrowVecRef(r, m) => { - let cmt_index = mcx.cat_index(expr, cmt, autoderefs+1); - let loan_mutability = - LoanMutability::from_ast_mutability(m); + let cmt_index = mc.cat_index(expr, cmt, autoderefs+1); self.guarantee_valid(expr.id, expr.span, cmt_index, - loan_mutability, - r) + m, + r, + AutoRef) } ty::AutoBorrowFn(r) => { - let cmt_deref = mcx.cat_deref_fn_or_obj(expr, cmt, 0); + let cmt_deref = mc.cat_deref_fn_or_obj(expr, cmt, 0); self.guarantee_valid(expr.id, expr.span, cmt_deref, - ImmutableMutability, - r) + ast::MutImmutable, + r, + AutoRef) } ty::AutoBorrowObj(r, m) => { - let cmt_deref = mcx.cat_deref_fn_or_obj(expr, cmt, 0); - let loan_mutability = - LoanMutability::from_ast_mutability(m); + let cmt_deref = mc.cat_deref_fn_or_obj(expr, cmt, 0); self.guarantee_valid(expr.id, expr.span, cmt_deref, - loan_mutability, - r) + m, + r, + AutoRef) } ty::AutoUnsafe(_) => {} } @@ -421,22 +417,71 @@ impl<'a> GatherLoanCtxt<'a> { } } - // Guarantees that addr_of(cmt) will be valid for the duration of - // `static_scope_r`, or reports an error. This may entail taking - // out loans, which will be added to the `req_loan_map`. This can - // also entail "rooting" GC'd pointers, which means ensuring - // dynamically that they are not freed. + fn guarantee_captures(&mut self, + closure_expr: &ast::Expr) { + let capture_map = self.bccx.capture_map.borrow(); + let captured_vars = capture_map.get().get(&closure_expr.id); + for captured_var in captured_vars.borrow().iter() { + match captured_var.mode { + moves::CapCopy | moves::CapMove => { continue; } + moves::CapRef => { } + } + + let var_id = ast_util::def_id_of_def(captured_var.def).node; + let var_cmt = self.bccx.cat_captured_var(closure_expr.id, + closure_expr.span, + captured_var); + + // Lookup the kind of borrow the callee requires + let upvar_id = ty::UpvarId { var_id: var_id, + closure_expr_id: closure_expr.id }; + let upvar_borrow_map = self.tcx().upvar_borrow_map.borrow(); + let upvar_borrow = upvar_borrow_map.get().get_copy(&upvar_id); + + self.guarantee_valid_kind(closure_expr.id, + closure_expr.span, + var_cmt, + upvar_borrow.kind, + upvar_borrow.region, + ClosureCapture(captured_var.span)); + } + } + pub fn guarantee_valid(&mut self, borrow_id: ast::NodeId, borrow_span: Span, cmt: mc::cmt, - req_mutbl: LoanMutability, - loan_region: ty::Region) { + req_mutbl: ast::Mutability, + loan_region: ty::Region, + cause: LoanCause) { + self.guarantee_valid_kind(borrow_id, + borrow_span, + cmt, + ty::BorrowKind::from_mutbl(req_mutbl), + loan_region, + cause); + } + + fn guarantee_valid_kind(&mut self, + borrow_id: ast::NodeId, + borrow_span: Span, + cmt: mc::cmt, + req_kind: ty::BorrowKind, + loan_region: ty::Region, + cause: LoanCause) { + /*! + * Guarantees that `addr_of(cmt)` will be valid for the duration of + * `static_scope_r`, or reports an error. This may entail taking + * out loans, which will be added to the `req_loan_map`. This can + * also entail "rooting" GC'd pointers, which means ensuring + * dynamically that they are not freed. + */ + debug!("guarantee_valid(borrow_id={:?}, cmt={}, \ req_mutbl={:?}, loan_region={:?})", borrow_id, cmt.repr(self.tcx()), - req_mutbl, + req_kind, loan_region); // a loan for the empty region can never be dereferenced, so @@ -450,26 +495,28 @@ impl<'a> GatherLoanCtxt<'a> { // Check that the lifetime of the borrow does not exceed // the lifetime of the data being borrowed. if lifetime::guarantee_lifetime(self.bccx, self.item_ub, root_ub, - borrow_span, cmt, loan_region, - req_mutbl).is_err() { + borrow_span, cause, cmt, loan_region, + req_kind).is_err() { return; // reported an error, no sense in reporting more. } // Check that we don't allow mutable borrows of non-mutable data. - if check_mutability(self.bccx, borrow_span, cmt, req_mutbl).is_err() { + if check_mutability(self.bccx, borrow_span, cause, + cmt, req_kind).is_err() { return; // reported an error, no sense in reporting more. } // Check that we don't allow mutable borrows of aliasable data. - if check_aliasability(self.bccx, borrow_span, cmt, req_mutbl).is_err() { + if check_aliasability(self.bccx, borrow_span, cause, + cmt, req_kind).is_err() { return; // reported an error, no sense in reporting more. } // Compute the restrictions that are required to enforce the // loan is safe. let restr = restrictions::compute_restrictions( - self.bccx, borrow_span, - cmt, loan_region, self.restriction_set(req_mutbl)); + self.bccx, borrow_span, cause, + cmt, loan_region, self.restriction_set(req_kind)); // Create the loan record (if needed). let loan = match restr { @@ -512,7 +559,7 @@ impl<'a> GatherLoanCtxt<'a> { let kill_scope = self.compute_kill_scope(loan_scope, loan_path); debug!("kill_scope = {:?}", kill_scope); - if req_mutbl == MutableMutability { + if req_kind == ty::MutBorrow { self.mark_loan_path_as_mutated(loan_path); } @@ -521,11 +568,12 @@ impl<'a> GatherLoanCtxt<'a> { index: all_loans.get().len(), loan_path: loan_path, cmt: cmt, - mutbl: req_mutbl, + kind: req_kind, gen_scope: gen_scope, kill_scope: kill_scope, span: borrow_span, - restrictions: restrictions + restrictions: restrictions, + cause: cause, } } }; @@ -568,23 +616,33 @@ impl<'a> GatherLoanCtxt<'a> { fn check_mutability(bccx: &BorrowckCtxt, borrow_span: Span, + cause: LoanCause, cmt: mc::cmt, - req_mutbl: LoanMutability) -> Result<(),()> { + req_kind: ty::BorrowKind) + -> Result<(),()> { //! Implements the M-* rules in doc.rs. - match req_mutbl { - ImmutableMutability => { - // both imm and mut data can be lent as imm; - // for mutable data, this is a freeze - Ok(()) + match req_kind { + ty::UniqueImmBorrow | ty::ImmBorrow => { + match cmt.mutbl { + // I am intentionally leaving this here to help + // refactoring if, in the future, we should add new + // kinds of mutability. + mc::McImmutable | mc::McDeclared | mc::McInherited => { + // both imm and mut data can be lent as imm; + // for mutable data, this is a freeze + Ok(()) + } + } } - MutableMutability => { + ty::MutBorrow => { // Only mutable data can be lent as mutable. if !cmt.mutbl.is_mutable() { - Err(bccx.report(BckError {span: borrow_span, - cmt: cmt, - code: err_mutbl(req_mutbl)})) + Err(bccx.report(BckError { span: borrow_span, + cause: cause, + cmt: cmt, + code: err_mutbl })) } else { Ok(()) } @@ -594,18 +652,18 @@ impl<'a> GatherLoanCtxt<'a> { fn check_aliasability(bccx: &BorrowckCtxt, borrow_span: Span, + loan_cause: LoanCause, cmt: mc::cmt, - req_mutbl: LoanMutability) -> Result<(),()> { + req_kind: ty::BorrowKind) + -> Result<(),()> { //! Implements the A-* rules in doc.rs. - match req_mutbl { - ImmutableMutability => { - // both imm and mut data can be lent as imm; - // for mutable data, this is a freeze + match req_kind { + ty::ImmBorrow => { Ok(()) } - MutableMutability => { + ty::UniqueImmBorrow | ty::MutBorrow => { // Check for those cases where we cannot control // the aliasing and make sure that we are not // being asked to. @@ -620,11 +678,11 @@ impl<'a> GatherLoanCtxt<'a> { // unsafe. At your own peril and all that. Ok(()) } - Some(cause) => { + Some(alias_cause) => { bccx.report_aliasability_violation( borrow_span, - BorrowViolation, - cause); + BorrowViolation(loan_cause), + alias_cause); Err(()) } } @@ -633,11 +691,18 @@ impl<'a> GatherLoanCtxt<'a> { } } - pub fn restriction_set(&self, req_mutbl: LoanMutability) - -> RestrictionSet { - match req_mutbl { - ImmutableMutability => RESTR_MUTATE | RESTR_CLAIM, - MutableMutability => RESTR_MUTATE | RESTR_CLAIM | RESTR_FREEZE, + fn restriction_set(&self, req_kind: ty::BorrowKind) -> RestrictionSet { + match req_kind { + // If borrowing data as immutable, no mutation allowed: + ty::ImmBorrow => RESTR_MUTATE, + + // If borrowing data as mutable, no mutation nor other + // borrows allowed: + ty::MutBorrow => RESTR_MUTATE | RESTR_FREEZE, + + // If borrowing data as unique imm, no mutation nor other + // borrows allowed: + ty::UniqueImmBorrow => RESTR_MUTATE | RESTR_FREEZE, } } @@ -719,11 +784,11 @@ impl<'a> GatherLoanCtxt<'a> { * `gather_pat()`. */ - let mc_ctxt = self.bccx.mc_ctxt(); + let mut mc = self.bccx.mc(); for arg in decl.inputs.iter() { let arg_ty = ty::node_id_to_type(self.tcx(), arg.pat.id); - let arg_cmt = mc_ctxt.cat_rvalue( + let arg_cmt = mc.cat_rvalue( arg.id, arg.pat.span, ty::ReScope(body.id), // Args live only as long as the fn body. @@ -735,7 +800,7 @@ impl<'a> GatherLoanCtxt<'a> { fn gather_pat(&mut self, discr_cmt: mc::cmt, - root_pat: &ast::Pat, + root_pat: @ast::Pat, arm_match_ids: Option<(ast::NodeId, ast::NodeId)>) { /*! * Walks patterns, examining the bindings to determine if they @@ -774,13 +839,12 @@ impl<'a> GatherLoanCtxt<'a> { } } }; - let loan_mutability = - LoanMutability::from_ast_mutability(mutbl); self.guarantee_valid(pat.id, pat.span, cmt_discr, - loan_mutability, - scope_r); + mutbl, + scope_r, + RefBinding); } ast::BindByValue(_) => { // No borrows here, but there may be moves @@ -797,14 +861,15 @@ impl<'a> GatherLoanCtxt<'a> { // original vector. This is effectively a borrow of // the elements of the vector being matched. - let slice_ty = ty::node_id_to_type(self.tcx(), - slice_pat.id); - let (slice_mutbl, slice_r) = - self.vec_slice_info(slice_pat, slice_ty); - let mcx = self.bccx.mc_ctxt(); - let cmt_index = mcx.cat_index(slice_pat, cmt, 0); - let slice_loan_mutability = - LoanMutability::from_ast_mutability(slice_mutbl); + let (slice_cmt, slice_borrow_kind, slice_r) = { + match self.bccx.mc().cat_slice_pattern(cmt, slice_pat) { + Ok(v) => v, + Err(()) => { + self.tcx().sess.span_bug(slice_pat.span, + "Err from mc") + } + } + }; // Note: We declare here that the borrow occurs upon // entering the `[...]` pattern. This implies that @@ -823,11 +888,9 @@ impl<'a> GatherLoanCtxt<'a> { // trans do the right thing, and it would only work // for `~` vectors. It seems simpler to just require // that people call `vec.pop()` or `vec.unshift()`. - self.guarantee_valid(pat.id, - pat.span, - cmt_index, - slice_loan_mutability, - slice_r); + self.guarantee_valid(pat.id, pat.span, + slice_cmt, slice_borrow_kind, slice_r, + RefBinding); } _ => {} @@ -835,33 +898,6 @@ impl<'a> GatherLoanCtxt<'a> { }) } - pub fn vec_slice_info(&self, pat: &ast::Pat, slice_ty: ty::t) - -> (ast::Mutability, ty::Region) { - /*! - * - * In a pattern like [a, b, ..c], normally `c` has slice type, - * but if you have [a, b, ..ref c], then the type of `ref c` - * will be `&&[]`, so to extract the slice details we have - * to recurse through rptrs. - */ - - match ty::get(slice_ty).sty { - ty::ty_vec(slice_mt, ty::vstore_slice(slice_r)) => { - (slice_mt.mutbl, slice_r) - } - - ty::ty_rptr(_, ref mt) => { - self.vec_slice_info(pat, mt.ty) - } - - _ => { - self.tcx().sess.span_bug( - pat.span, - format!("type of slice pattern is not a slice")); - } - } - } - pub fn pat_is_binding(&self, pat: &ast::Pat) -> bool { pat_util::pat_is_binding(self.bccx.tcx.def_map, pat) } diff --git a/src/librustc/middle/borrowck/gather_loans/restrictions.rs b/src/librustc/middle/borrowck/gather_loans/restrictions.rs index 60e0baa3534..575119ba690 100644 --- a/src/librustc/middle/borrowck/gather_loans/restrictions.rs +++ b/src/librustc/middle/borrowck/gather_loans/restrictions.rs @@ -16,8 +16,8 @@ use std::vec; use middle::borrowck::*; use mc = middle::mem_categorization; use middle::ty; -use syntax::ast::{MutImmutable, MutMutable}; use syntax::codemap::Span; +use util::ppaux::Repr; pub enum RestrictionResult { Safe, @@ -26,12 +26,14 @@ pub enum RestrictionResult { pub fn compute_restrictions(bccx: &BorrowckCtxt, span: Span, + cause: LoanCause, cmt: mc::cmt, loan_region: ty::Region, restr: RestrictionSet) -> RestrictionResult { let ctxt = RestrictionsContext { bccx: bccx, span: span, + cause: cause, cmt_original: cmt, loan_region: loan_region, }; @@ -47,12 +49,17 @@ struct RestrictionsContext<'a> { span: Span, cmt_original: mc::cmt, loan_region: ty::Region, + cause: LoanCause, } impl<'a> RestrictionsContext<'a> { fn restrict(&self, cmt: mc::cmt, restrictions: RestrictionSet) -> RestrictionResult { + debug!("restrict(cmt={}, restrictions={})", + cmt.repr(self.bccx.tcx), + restrictions.repr(self.bccx.tcx)); + match cmt.cat { mc::cat_rvalue(..) => { // Effectively, rvalues are stored into a @@ -64,7 +71,8 @@ impl<'a> RestrictionsContext<'a> { } mc::cat_local(local_id) | - mc::cat_arg(local_id) => { + mc::cat_arg(local_id) | + mc::cat_upvar(ty::UpvarId {var_id: local_id, ..}, _) => { // R-Variable let lp = @LpVar(local_id); SafeIf(lp, ~[Restriction {loan_path: lp, @@ -77,7 +85,7 @@ impl<'a> RestrictionsContext<'a> { // could cause the type of the memory to change. self.restrict( cmt_base, - restrictions | RESTR_MUTATE | RESTR_CLAIM) + restrictions | RESTR_MUTATE) } mc::cat_interior(cmt_base, i) => { @@ -90,7 +98,7 @@ impl<'a> RestrictionsContext<'a> { self.extend(result, cmt.mutbl, LpInterior(i), restrictions) } - mc::cat_deref(cmt_base, _, pk @ mc::uniq_ptr) => { + mc::cat_deref(cmt_base, _, pk @ mc::OwnedPtr) => { // R-Deref-Send-Pointer // // When we borrow the interior of an owned pointer, we @@ -98,7 +106,7 @@ impl<'a> RestrictionsContext<'a> { // would cause the unique pointer to be freed. let result = self.restrict( cmt_base, - restrictions | RESTR_MUTATE | RESTR_CLAIM); + restrictions | RESTR_MUTATE); self.extend(result, cmt.mutbl, LpDeref(pk), restrictions) } @@ -107,12 +115,14 @@ impl<'a> RestrictionsContext<'a> { Safe } - mc::cat_deref(cmt_base, _, mc::region_ptr(MutImmutable, lt)) => { + mc::cat_deref(cmt_base, _, mc::BorrowedPtr(ty::ImmBorrow, lt)) | + mc::cat_deref(cmt_base, _, mc::BorrowedPtr(ty::UniqueImmBorrow, lt)) => { // R-Deref-Imm-Borrowed if !self.bccx.is_subregion_of(self.loan_region, lt) { self.bccx.report( BckError { span: self.span, + cause: self.cause, cmt: cmt_base, code: err_borrowed_pointer_too_short( self.loan_region, lt, restrictions)}); @@ -121,17 +131,18 @@ impl<'a> RestrictionsContext<'a> { Safe } - mc::cat_deref(_, _, mc::gc_ptr) => { + mc::cat_deref(_, _, mc::GcPtr) => { // R-Deref-Imm-Managed Safe } - mc::cat_deref(cmt_base, _, pk @ mc::region_ptr(MutMutable, lt)) => { + mc::cat_deref(cmt_base, _, pk @ mc::BorrowedPtr(ty::MutBorrow, lt)) => { // R-Deref-Mut-Borrowed if !self.bccx.is_subregion_of(self.loan_region, lt) { self.bccx.report( BckError { span: self.span, + cause: self.cause, cmt: cmt_base, code: err_borrowed_pointer_too_short( self.loan_region, lt, restrictions)}); @@ -142,12 +153,11 @@ impl<'a> RestrictionsContext<'a> { self.extend(result, cmt.mutbl, LpDeref(pk), restrictions) } - mc::cat_deref(_, _, mc::unsafe_ptr(..)) => { + mc::cat_deref(_, _, mc::UnsafePtr(..)) => { // We are very trusting when working with unsafe pointers. Safe } - mc::cat_stack_upvar(cmt_base) | mc::cat_discr(cmt_base, _) => { self.restrict(cmt_base, restrictions) } diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs index 6a3e2fc63b0..acc8ece85f8 100644 --- a/src/librustc/middle/borrowck/mod.rs +++ b/src/librustc/middle/borrowck/mod.rs @@ -120,41 +120,32 @@ fn borrowck_fn(this: &mut BorrowckCtxt, body: &ast::Block, sp: Span, id: ast::NodeId) { - match fk { - &visit::FkFnBlock(..) => { - // Closures are checked as part of their containing fn item. - } + debug!("borrowck_fn(id={})", id); - &visit::FkItemFn(..) | &visit::FkMethod(..) => { - debug!("borrowck_fn(id={:?})", id); - - // Check the body of fn items. - let (id_range, all_loans, move_data) = - gather_loans::gather_loans(this, decl, body); - - let all_loans = all_loans.borrow(); - let mut loan_dfcx = DataFlowContext::new(this.tcx, - this.method_map, - LoanDataFlowOperator, - id_range, - all_loans.get().len()); - for (loan_idx, loan) in all_loans.get().iter().enumerate() { - loan_dfcx.add_gen(loan.gen_scope, loan_idx); - loan_dfcx.add_kill(loan.kill_scope, loan_idx); - } - - loan_dfcx.propagate(body); - - let flowed_moves = move_data::FlowedMoveData::new(move_data, - this.tcx, - this.method_map, - id_range, - body); - - check_loans::check_loans(this, &loan_dfcx, flowed_moves, - *all_loans.get(), body); - } + // Check the body of fn items. + let (id_range, all_loans, move_data) = + gather_loans::gather_loans(this, decl, body); + let all_loans = all_loans.borrow(); + let mut loan_dfcx = + DataFlowContext::new(this.tcx, + this.method_map, + LoanDataFlowOperator, + id_range, + all_loans.get().len()); + for (loan_idx, loan) in all_loans.get().iter().enumerate() { + loan_dfcx.add_gen(loan.gen_scope, loan_idx); + loan_dfcx.add_kill(loan.kill_scope, loan_idx); } + loan_dfcx.propagate(body); + + let flowed_moves = move_data::FlowedMoveData::new(move_data, + this.tcx, + this.method_map, + id_range, + body); + + check_loans::check_loans(this, &loan_dfcx, flowed_moves, + *all_loans.get(), body); visit::walk_fn(this, fk, decl, body, sp, id, ()); } @@ -211,41 +202,25 @@ pub enum PartialTotal { /////////////////////////////////////////////////////////////////////////// // Loans and loan paths -#[deriving(Clone, Eq)] -pub enum LoanMutability { - ImmutableMutability, - MutableMutability, -} - -impl LoanMutability { - pub fn from_ast_mutability(ast_mutability: ast::Mutability) - -> LoanMutability { - match ast_mutability { - ast::MutImmutable => ImmutableMutability, - ast::MutMutable => MutableMutability, - } - } -} - -impl ToStr for LoanMutability { - fn to_str(&self) -> ~str { - match *self { - ImmutableMutability => ~"immutable", - MutableMutability => ~"mutable", - } - } -} - /// Record of a loan that was issued. pub struct Loan { index: uint, loan_path: @LoanPath, cmt: mc::cmt, - mutbl: LoanMutability, + kind: ty::BorrowKind, restrictions: ~[Restriction], gen_scope: ast::NodeId, kill_scope: ast::NodeId, span: Span, + cause: LoanCause, +} + +#[deriving(Eq)] +pub enum LoanCause { + ClosureCapture(Span), + AddrOf, + AutoRef, + RefBinding, } #[deriving(Eq, IterBytes)] @@ -283,7 +258,9 @@ pub fn opt_loan_path(cmt: mc::cmt) -> Option<@LoanPath> { None } - mc::cat_local(id) | mc::cat_arg(id) => { + mc::cat_local(id) | + mc::cat_arg(id) | + mc::cat_upvar(ty::UpvarId {var_id: id, ..}, _) => { Some(@LpVar(id)) } @@ -300,7 +277,6 @@ pub fn opt_loan_path(cmt: mc::cmt) -> Option<@LoanPath> { } mc::cat_downcast(cmt_base) | - mc::cat_stack_upvar(cmt_base) | mc::cat_discr(cmt_base, _) => { opt_loan_path(cmt_base) } @@ -313,8 +289,7 @@ pub fn opt_loan_path(cmt: mc::cmt) -> Option<@LoanPath> { // Borrowing an lvalue often results in *restrictions* that limit what // can be done with this lvalue during the scope of the loan: // -// - `RESTR_MUTATE`: The lvalue may not be modified. -// - `RESTR_CLAIM`: `&mut` borrows of the lvalue are forbidden. +// - `RESTR_MUTATE`: The lvalue may not be modified or `&mut` borrowed. // - `RESTR_FREEZE`: `&` borrows of the lvalue are forbidden. // // In addition, no value which is restricted may be moved. Therefore, @@ -333,8 +308,7 @@ pub struct RestrictionSet { pub static RESTR_EMPTY: RestrictionSet = RestrictionSet {bits: 0b0000}; pub static RESTR_MUTATE: RestrictionSet = RestrictionSet {bits: 0b0001}; -pub static RESTR_CLAIM: RestrictionSet = RestrictionSet {bits: 0b0010}; -pub static RESTR_FREEZE: RestrictionSet = RestrictionSet {bits: 0b0100}; +pub static RESTR_FREEZE: RestrictionSet = RestrictionSet {bits: 0b0010}; impl RestrictionSet { pub fn intersects(&self, restr: RestrictionSet) -> bool { @@ -358,6 +332,12 @@ impl BitAnd for RestrictionSet { } } +impl Repr for RestrictionSet { + fn repr(&self, _tcx: ty::ctxt) -> ~str { + format!("RestrictionSet(0x{:x})", self.bits as uint) + } +} + /////////////////////////////////////////////////////////////////////////// // Rooting of managed boxes // @@ -393,10 +373,9 @@ pub fn root_map() -> root_map { // Errors that can occur #[deriving(Eq)] pub enum bckerr_code { - err_mutbl(LoanMutability), + err_mutbl, err_out_of_root_scope(ty::Region, ty::Region), // superscope, subscope err_out_of_scope(ty::Region, ty::Region), // superscope, subscope - err_freeze_aliasable_const, err_borrowed_pointer_too_short( ty::Region, ty::Region, RestrictionSet), // loan, ptr } @@ -406,13 +385,14 @@ pub enum bckerr_code { #[deriving(Eq)] pub struct BckError { span: Span, + cause: LoanCause, cmt: mc::cmt, code: bckerr_code } pub enum AliasableViolationKind { MutabilityViolation, - BorrowViolation + BorrowViolation(LoanCause) } pub enum MovedValueUseKind { @@ -439,29 +419,55 @@ impl BorrowckCtxt { moves_map.get().contains(&id) } + pub fn mc(&self) -> mc::MemCategorizationContext { + mc::MemCategorizationContext { + typer: TcxTyper { + tcx: self.tcx, + method_map: self.method_map + } + } + } + pub fn cat_expr(&self, expr: &ast::Expr) -> mc::cmt { - mc::cat_expr(self.tcx, self.method_map, expr) + match self.mc().cat_expr(expr) { + Ok(c) => c, + Err(()) => { + self.tcx.sess.span_bug(expr.span, "error in mem categorization"); + } + } } pub fn cat_expr_unadjusted(&self, expr: &ast::Expr) -> mc::cmt { - mc::cat_expr_unadjusted(self.tcx, self.method_map, expr) + match self.mc().cat_expr_unadjusted(expr) { + Ok(c) => c, + Err(()) => { + self.tcx.sess.span_bug(expr.span, "error in mem categorization"); + } + } } pub fn cat_expr_autoderefd(&self, expr: &ast::Expr, adj: &ty::AutoAdjustment) -> mc::cmt { - match *adj { + let r = match *adj { ty::AutoAddEnv(..) | ty::AutoObject(..) => { // no autoderefs - mc::cat_expr_unadjusted(self.tcx, self.method_map, expr) + self.mc().cat_expr_unadjusted(expr) } ty::AutoDerefRef( ty::AutoDerefRef { autoderefs: autoderefs, ..}) => { - mc::cat_expr_autoderefd(self.tcx, self.method_map, expr, - autoderefs) + self.mc().cat_expr_autoderefd(expr, autoderefs) + } + }; + + match r { + Ok(c) => c, + Err(()) => { + self.tcx.sess.span_bug(expr.span, + "error in mem categorization"); } } } @@ -472,7 +478,23 @@ impl BorrowckCtxt { ty: ty::t, def: ast::Def) -> mc::cmt { - mc::cat_def(self.tcx, self.method_map, id, span, ty, def) + match self.mc().cat_def(id, span, ty, def) { + Ok(c) => c, + Err(()) => { + self.tcx.sess.span_bug(span, "error in mem categorization"); + } + } + } + + pub fn cat_captured_var(&self, + id: ast::NodeId, + span: Span, + captured_var: &moves::CaptureVar) -> mc::cmt { + // Create the cmt for the variable being borrowed, from the + // caller's perspective + let var_id = ast_util::def_id_of_def(captured_var.def).node; + let var_ty = ty::node_id_to_type(self.tcx, var_id); + self.cat_def(id, span, var_ty, captured_var.def) } pub fn cat_discr(&self, cmt: mc::cmt, match_id: ast::NodeId) -> mc::cmt { @@ -481,17 +503,12 @@ impl BorrowckCtxt { ..*cmt} } - pub fn mc_ctxt(&self) -> mc::mem_categorization_ctxt { - mc::mem_categorization_ctxt {tcx: self.tcx, - method_map: self.method_map} - } - pub fn cat_pattern(&self, cmt: mc::cmt, - pat: &ast::Pat, + pat: @ast::Pat, op: |mc::cmt, &ast::Pat|) { - let mc = self.mc_ctxt(); - mc.cat_pattern(cmt, pat, op); + let r = self.mc().cat_pattern(cmt, pat, |_,x,y| op(x,y)); + assert!(r.is_ok()); } pub fn report(&self, err: BckError) { @@ -622,24 +639,35 @@ impl BorrowckCtxt { pub fn bckerr_to_str(&self, err: BckError) -> ~str { match err.code { - err_mutbl(lk) => { - format!("cannot borrow {} {} as {}", - err.cmt.mutbl.to_user_str(), - self.cmt_to_str(err.cmt), - self.mut_to_str(lk)) + err_mutbl => { + let descr = match opt_loan_path(err.cmt) { + None => format!("{} {}", + err.cmt.mutbl.to_user_str(), + self.cmt_to_str(err.cmt)), + Some(lp) => format!("{} {} `{}`", + err.cmt.mutbl.to_user_str(), + self.cmt_to_str(err.cmt), + self.loan_path_to_str(lp)), + }; + + match err.cause { + ClosureCapture(_) => { + format!("closure cannot assign to {}", descr) + } + AddrOf | RefBinding | AutoRef => { + format!("cannot borrow {} as mutable", descr) + } + } } err_out_of_root_scope(..) => { format!("cannot root managed value long enough") } err_out_of_scope(..) => { - format!("borrowed value does not live long enough") - } - err_freeze_aliasable_const => { - // Means that the user borrowed a ~T or enum value - // residing in &const or @const pointer. Terrible - // error message, but then &const and @const are - // supposed to be going away. - format!("unsafe borrow of aliasable, const value") + let msg = match opt_loan_path(err.cmt) { + None => format!("borrowed value"), + Some(lp) => format!("`{}`", self.loan_path_to_str(lp)), + }; + format!("{} does not live long enough", msg) } err_borrowed_pointer_too_short(..) => { let descr = match opt_loan_path(err.cmt) { @@ -659,8 +687,24 @@ impl BorrowckCtxt { kind: AliasableViolationKind, cause: mc::AliasableReason) { let prefix = match kind { - MutabilityViolation => "cannot assign to data", - BorrowViolation => "cannot borrow data mutably" + MutabilityViolation => { + "cannot assign to data" + } + BorrowViolation(ClosureCapture(_)) => { + // I don't think we can get aliasability violations + // with closure captures, so no need to come up with a + // good error message. The reason this cannot happen + // is because we only capture local variables in + // closures, and those are never aliasable. + self.tcx.sess.span_bug( + span, + "aliasability violation with closure"); + } + BorrowViolation(AddrOf) | + BorrowViolation(AutoRef) | + BorrowViolation(RefBinding) => { + "cannot borrow data mutably" + } }; match cause { @@ -680,7 +724,7 @@ impl BorrowckCtxt { span, format!("{} in a `@` pointer", prefix)); } - mc::AliasableBorrowed(_) => { + mc::AliasableBorrowed => { self.tcx.sess.span_err( span, format!("{} in a `&` reference", prefix)); @@ -691,7 +735,7 @@ impl BorrowckCtxt { pub fn note_and_explain_bckerr(&self, err: BckError) { let code = err.code; match code { - err_mutbl(..) | err_freeze_aliasable_const(..) => {} + err_mutbl(..) => { } err_out_of_root_scope(super_scope, sub_scope) => { note_and_explain_region( @@ -738,52 +782,16 @@ impl BorrowckCtxt { } } - pub fn append_loan_path_to_str_from_interior(&self, - loan_path: &LoanPath, - out: &mut ~str) { - match *loan_path { - LpExtend(_, _, LpDeref(_)) => { - out.push_char('('); - self.append_loan_path_to_str(loan_path, out); - out.push_char(')'); - } - LpExtend(_, _, LpInterior(_)) | - LpVar(_) => { - self.append_loan_path_to_str(loan_path, out); - } - } - } - pub fn append_loan_path_to_str(&self, loan_path: &LoanPath, out: &mut ~str) { match *loan_path { LpVar(id) => { - match self.tcx.items.find(id) { - Some(ast_map::NodeLocal(pat)) => { - match pat.node { - ast::PatIdent(_, ref path, _) => { - let ident = ast_util::path_to_ident(path); - let string = token::get_ident(ident.name); - out.push_str(string.get()); - } - _ => { - self.tcx.sess.bug( - format!("loan path LpVar({:?}) maps to {:?}, not local", - id, pat)); - } - } - } - r => { - self.tcx.sess.bug( - format!("loan path LpVar({:?}) maps to {:?}, not local", - id, r)); - } - } + out.push_str(ty::local_var_name_str(self.tcx, id).get()); } LpExtend(lp_base, _, LpInterior(mc::InteriorField(fname))) => { - self.append_loan_path_to_str_from_interior(lp_base, out); + self.append_autoderefd_loan_path_to_str(lp_base, out); match fname { mc::NamedField(ref fname) => { let string = token::get_ident(*fname); @@ -798,8 +806,8 @@ impl BorrowckCtxt { } LpExtend(lp_base, _, LpInterior(mc::InteriorElement(_))) => { - self.append_loan_path_to_str_from_interior(lp_base, out); - out.push_str("[]"); + self.append_autoderefd_loan_path_to_str(lp_base, out); + out.push_str("[..]"); } LpExtend(lp_base, _, LpDeref(_)) => { @@ -809,6 +817,23 @@ impl BorrowckCtxt { } } + pub fn append_autoderefd_loan_path_to_str(&self, + loan_path: &LoanPath, + out: &mut ~str) { + match *loan_path { + LpExtend(lp_base, _, LpDeref(_)) => { + // For a path like `(*x).f` or `(*x)[3]`, autoderef + // rules would normally allow users to omit the `*x`. + // So just serialize such paths to `x.f` or x[3]` respectively. + self.append_autoderefd_loan_path_to_str(lp_base, out) + } + + LpVar(..) | LpExtend(_, _, LpInterior(..)) => { + self.append_loan_path_to_str(loan_path, out) + } + } + } + pub fn loan_path_to_str(&self, loan_path: &LoanPath) -> ~str { let mut result = ~""; self.append_loan_path_to_str(loan_path, &mut result); @@ -816,13 +841,11 @@ impl BorrowckCtxt { } pub fn cmt_to_str(&self, cmt: mc::cmt) -> ~str { - let mc = &mc::mem_categorization_ctxt {tcx: self.tcx, - method_map: self.method_map}; - mc.cmt_to_str(cmt) + self.mc().cmt_to_str(cmt) } - pub fn mut_to_str(&self, mutbl: LoanMutability) -> ~str { - mutbl.to_str() + pub fn mut_to_str(&self, mutbl: ast::Mutability) -> ~str { + self.mc().mut_to_str(mutbl) } pub fn mut_to_keyword(&self, mutbl: ast::Mutability) -> &'static str { @@ -843,11 +866,6 @@ impl DataFlowOperator for LoanDataFlowOperator { fn join(&self, succ: uint, pred: uint) -> uint { succ | pred // loans from both preds are in scope } - - #[inline] - fn walk_closures(&self) -> bool { - true - } } impl Repr for Loan { @@ -855,7 +873,7 @@ impl Repr for Loan { format!("Loan_{:?}({}, {:?}, {:?}-{:?}, {})", self.index, self.loan_path.repr(tcx), - self.mutbl, + self.kind, self.gen_scope, self.kill_scope, self.restrictions.repr(tcx)) @@ -890,3 +908,39 @@ impl Repr for LoanPath { } } } + +/////////////////////////////////////////////////////////////////////////// + +struct TcxTyper { + tcx: ty::ctxt, + method_map: typeck::method_map, +} + +impl mc::Typer for TcxTyper { + fn tcx(&self) -> ty::ctxt { + self.tcx + } + + fn node_ty(&mut self, id: ast::NodeId) -> mc::McResult { + Ok(ty::node_id_to_type(self.tcx, id)) + } + + fn adjustment(&mut self, id: ast::NodeId) -> Option<@ty::AutoAdjustment> { + let adjustments = self.tcx.adjustments.borrow(); + adjustments.get().find_copy(&id) + } + + fn is_method_call(&mut self, id: ast::NodeId) -> bool { + let method_map = self.method_map.borrow(); + method_map.get().contains_key(&id) + } + + fn temporary_scope(&mut self, id: ast::NodeId) -> Option { + self.tcx.region_maps.temporary_scope(id) + } + + fn upvar_borrow(&mut self, id: ty::UpvarId) -> ty::UpvarBorrow { + let upvar_borrow_map = self.tcx.upvar_borrow_map.borrow(); + upvar_borrow_map.get().get_copy(&id) + } +} diff --git a/src/librustc/middle/borrowck/move_data.rs b/src/librustc/middle/borrowck/move_data.rs index 75549c5944d..34efcacc44b 100644 --- a/src/librustc/middle/borrowck/move_data.rs +++ b/src/librustc/middle/borrowck/move_data.rs @@ -722,11 +722,6 @@ impl DataFlowOperator for MoveDataFlowOperator { fn join(&self, succ: uint, pred: uint) -> uint { succ | pred // moves from both preds are in scope } - - #[inline] - fn walk_closures(&self) -> bool { - true - } } impl DataFlowOperator for AssignDataFlowOperator { @@ -739,9 +734,4 @@ impl DataFlowOperator for AssignDataFlowOperator { fn join(&self, succ: uint, pred: uint) -> uint { succ | pred // moves from both preds are in scope } - - #[inline] - fn walk_closures(&self) -> bool { - true - } }