diff options
| -rw-r--r-- | src/librustc/middle/borrowck/check_loans.rs | 428 | ||||
| -rw-r--r-- | src/librustc/middle/borrowck/mod.rs | 2 | ||||
| -rw-r--r-- | src/librustc/middle/borrowck/move_data.rs | 1 | ||||
| -rw-r--r-- | src/test/compile-fail/borrowck-init-in-fru.rs | 2 | ||||
| -rw-r--r-- | src/test/compile-fail/liveness-use-after-move.rs | 2 | ||||
| -rw-r--r-- | src/test/compile-fail/moves-based-on-type-access-to-field.rs | 6 | ||||
| -rw-r--r-- | src/test/compile-fail/moves-sru-moved-field.rs | 8 | ||||
| -rw-r--r-- | src/test/compile-fail/use-after-move-self-based-on-type.rs | 2 | ||||
| -rw-r--r-- | src/test/compile-fail/use-after-move-self.rs | 2 |
9 files changed, 223 insertions, 230 deletions
diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 6b8efe55f24..fed1fb9c609 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -20,15 +20,10 @@ use middle::borrowck::*; use euv = middle::expr_use_visitor; -use middle::freevars; use mc = middle::mem_categorization; use middle::ty; -use middle::typeck::MethodCall; use syntax::ast; -use syntax::ast_util; use syntax::codemap::Span; -use syntax::visit::Visitor; -use syntax::visit; use util::ppaux::Repr; use std::rc::Rc; @@ -40,35 +35,104 @@ struct CheckLoanCtxt<'a> { all_loans: &'a [Loan], } -impl<'a> Visitor<()> for CheckLoanCtxt<'a> { +impl<'a> euv::Delegate for CheckLoanCtxt<'a> { + fn consume(&mut self, + consume_id: ast::NodeId, + consume_span: Span, + cmt: mc::cmt, + mode: euv::ConsumeMode) { + debug!("consume(consume_id={}, cmt={}, mode={})", + consume_id, cmt.repr(self.tcx()), mode); - fn visit_expr(&mut self, ex: &ast::Expr, _: ()) { - check_loans_in_expr(self, ex); + self.consume_common(consume_id, consume_span, cmt, mode); } - fn visit_local(&mut self, l: &ast::Local, _: ()) { - check_loans_in_local(self, l); - } - fn visit_block(&mut self, b: &ast::Block, _: ()) { - check_loans_in_block(self, b); + + fn consume_pat(&mut self, + consume_pat: &ast::Pat, + cmt: mc::cmt, + mode: euv::ConsumeMode) { + debug!("consume_pat(consume_pat={}, cmt={}, mode={})", + consume_pat.repr(self.tcx()), + cmt.repr(self.tcx()), + mode); + + self.consume_common(consume_pat.id, consume_pat.span, cmt, mode); } - fn visit_pat(&mut self, p: &ast::Pat, _: ()) { - check_loans_in_pat(self, p); + + fn borrow(&mut self, + borrow_id: ast::NodeId, + borrow_span: Span, + cmt: mc::cmt, + loan_region: ty::Region, + bk: ty::BorrowKind, + loan_cause: euv::LoanCause) + { + debug!("borrow(borrow_id={}, cmt={}, loan_region={}, \ + bk={}, loan_cause={:?})", + borrow_id, cmt.repr(self.tcx()), loan_region, + bk, loan_cause); + + match opt_loan_path(&cmt) { + Some(lp) => { + let moved_value_use_kind = match loan_cause { + euv::ClosureCapture(_) => MovedInCapture, + _ => MovedInUse, + }; + self.check_if_path_is_moved(borrow_id, borrow_span, moved_value_use_kind, &lp); + } + None => { } + } + + self.check_for_conflicting_loans(borrow_id); } - 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; + + fn mutate(&mut self, + assignment_id: ast::NodeId, + assignment_span: Span, + assignee_cmt: mc::cmt, + mode: euv::MutateMode) + { + debug!("mutate(assignment_id={}, assignee_cmt={})", + assignment_id, assignee_cmt.repr(self.tcx())); + + match opt_loan_path(&assignee_cmt) { + Some(lp) => { + match mode { + euv::Init | euv::JustWrite => { + // In a case like `path = 1`, then path does not + // have to be *FULLY* initialized, but we still + // must be careful lest it contains derefs of + // pointers. + self.check_if_assigned_path_is_moved(assignee_cmt.id, + assignment_span, + MovedInUse, + &lp); + } + euv::WriteAndRead => { + // In a case like `path += 1`, then path must be + // fully initialized, since we will read it before + // we write it. + self.check_if_path_is_moved(assignee_cmt.id, + assignment_span, + MovedInUse, + &lp); + } + } + } + None => { } + } + + self.check_assignment(assignment_id, assignment_span, assignee_cmt, mode); } - // FIXME(#10894) should continue recursing - fn visit_ty(&mut self, _t: &ast::Ty, _: ()) {} + fn decl_without_init(&mut self, _id: ast::NodeId, _span: Span) { } } pub fn check_loans(bccx: &BorrowckCtxt, dfcx_loans: &LoanDataFlow, move_data: move_data::FlowedMoveData, all_loans: &[Loan], + decl: &ast::FnDecl, body: &ast::Block) { debug!("check_loans(body id={:?})", body.id); @@ -79,7 +143,10 @@ pub fn check_loans(bccx: &BorrowckCtxt, all_loans: all_loans, }; - clcx.visit_block(body, ()); + { + let mut euv = euv::ExprUseVisitor::new(&mut clcx, bccx.tcx); + euv.walk_fn(decl, body); + } } #[deriving(PartialEq)] @@ -355,18 +422,130 @@ impl<'a> CheckLoanCtxt<'a> { true } - pub fn is_local_variable(&self, cmt: mc::cmt) -> bool { + pub fn is_local_variable_or_arg(&self, cmt: mc::cmt) -> bool { match cmt.cat { - mc::cat_local(_) => true, + mc::cat_local(_) | mc::cat_arg(_) => true, _ => false } } - pub fn check_if_path_is_moved(&self, - id: ast::NodeId, - span: Span, - use_kind: MovedValueUseKind, - lp: &Rc<LoanPath>) { + fn consume_common(&self, + id: ast::NodeId, + span: Span, + cmt: mc::cmt, + mode: euv::ConsumeMode) { + match opt_loan_path(&cmt) { + Some(lp) => { + let moved_value_use_kind = match mode { + euv::Copy => { + // FIXME(#12624) -- If we are copying the value, + // we don't care if it's borrowed. + MovedInUse + } + euv::Move(_) => { + match self.move_data.kind_of_move_of_path(id, &lp) { + None => { + // Sometimes moves don't have a move kind; + // this either means that the original move + // was from something illegal to move, + // or was moved from referent of an unsafe + // pointer or something like that. + MovedInUse + } + Some(move_kind) => { + self.check_for_move_of_borrowed_path(id, span, + &lp, move_kind); + if move_kind == move_data::Captured { + MovedInCapture + } else { + MovedInUse + } + } + } + } + }; + + self.check_if_path_is_moved(id, span, moved_value_use_kind, &lp); + } + None => { } + } + } + + fn check_for_move_of_borrowed_path(&self, + id: ast::NodeId, + span: Span, + move_path: &Rc<LoanPath>, + move_kind: move_data::MoveKind) { + match self.analyze_move_out_from(id, &**move_path) { + MoveOk => { } + MoveWhileBorrowed(loan_path, loan_span) => { + let err_message = match move_kind { + move_data::Captured => + format!("cannot move `{}` into closure because it is borrowed", + self.bccx.loan_path_to_str(&**move_path).as_slice()), + move_data::Declared | + move_data::MoveExpr | + move_data::MovePat => + format!("cannot move out of `{}` because it is borrowed", + self.bccx.loan_path_to_str(&**move_path).as_slice()) + }; + + self.bccx.span_err(span, err_message.as_slice()); + self.bccx.span_note( + loan_span, + format!("borrow of `{}` occurs here", + self.bccx.loan_path_to_str(&*loan_path).as_slice()) + .as_slice()); + } + } + } + + fn check_if_assigned_path_is_moved(&self, + id: ast::NodeId, + span: Span, + use_kind: MovedValueUseKind, + lp: &Rc<LoanPath>) + { + /*! + * Reports an error if assigning to `lp` will use a + * moved/uninitialized value. Mainly this is concerned with + * detecting derefs of uninitialized pointers. + * + * For example: + * + * let a: int; + * a = 10; // ok, even though a is uninitialized + * + * struct Point { x: uint, y: uint } + * let p: Point; + * p.x = 22; // ok, even though `p` is uninitialized + * + * let p: ~Point; + * (*p).x = 22; // not ok, p is uninitialized, can't deref + */ + + match **lp { + LpVar(_) => { + // assigning to `x` does not require that `x` is initialized + } + LpExtend(ref lp_base, _, LpInterior(_)) => { + // assigning to `P.f` is ok if assigning to `P` is ok + self.check_if_assigned_path_is_moved(id, span, + use_kind, lp_base); + } + LpExtend(ref lp_base, _, LpDeref(_)) => { + // assigning to `(*P)` requires that `P` be initialized + self.check_if_path_is_moved(id, span, + use_kind, lp_base); + } + } + } + + fn check_if_path_is_moved(&self, + id: ast::NodeId, + span: Span, + use_kind: MovedValueUseKind, + lp: &Rc<LoanPath>) { /*! * Reports an error if `expr` (which should be a path) * is using a moved/uninitialized value @@ -385,32 +564,21 @@ impl<'a> CheckLoanCtxt<'a> { }); } - pub fn check_assignment_expr(&self, expr: &ast::Expr) { - let assignment_id = expr.id; - let assignment_span = expr.span; - - // We don't use cat_expr() here because we don't want to treat - // auto-ref'd parameters in overloaded operators as rvalues. - let assignee_cmt = match self.bccx.tcx.adjustments.borrow().find(&assignment_id) { - None => self.bccx.cat_expr_unadjusted(expr), - Some(adj) => self.bccx.cat_expr_autoderefd(expr, adj) - }; - - self.check_assignment(assignment_id, assignment_span, assignee_cmt); - } - fn check_assignment(&self, assignment_id: ast::NodeId, assignment_span: Span, - assignee_cmt: mc::cmt) { + assignee_cmt: mc::cmt, + mode: euv::MutateMode) { debug!("check_assignment(assignee_cmt={})", assignee_cmt.repr(self.tcx())); // Mutable values can be assigned, as long as they obey loans // and aliasing restrictions: if assignee_cmt.mutbl.is_mutable() { if check_for_aliasable_mutable_writes(self, assignment_span, assignee_cmt.clone()) { - if check_for_assignment_to_restricted_or_frozen_location( - self, assignment_id, assignment_span, assignee_cmt.clone()) { + if mode != euv::Init && + check_for_assignment_to_restricted_or_frozen_location( + self, assignment_id, assignment_span, assignee_cmt.clone()) + { // Safe, but record for lint pass later: mark_variable_as_used_mut(self, assignee_cmt); } @@ -420,7 +588,7 @@ impl<'a> CheckLoanCtxt<'a> { // For immutable local variables, assignments are legal // if they cannot already have been assigned - if self.is_local_variable(assignee_cmt.clone()) { + if self.is_local_variable_or_arg(assignee_cmt.clone()) { assert!(assignee_cmt.mutbl.is_immutable()); // no "const" locals let lp = opt_loan_path(&assignee_cmt).unwrap(); self.move_data.each_assignment_of(assignment_id, &lp, |assign| { @@ -695,86 +863,6 @@ impl<'a> CheckLoanCtxt<'a> { self.bccx.loan_path_to_str(loan_path)).as_slice()); } - fn check_move_out_from_expr(&self, expr: &ast::Expr) { - match expr.node { - ast::ExprFnBlock(..) | ast::ExprProc(..) => { - // 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) - } - } - } - - fn check_move_out_from_id(&self, id: ast::NodeId, span: Span) { - self.move_data.each_path_moved_by(id, |_, move_path| { - match self.analyze_move_out_from(id, move_path) { - MoveOk => {} - MoveWhileBorrowed(loan_path, loan_span) => { - self.bccx.span_err( - span, - format!("cannot move out of `{}` \ - because it is borrowed", - self.bccx.loan_path_to_str( - move_path)).as_slice()); - self.bccx.span_note( - loan_span, - format!("borrow of `{}` occurs here", - self.bccx.loan_path_to_str( - &*loan_path)).as_slice()); - } - } - true - }); - } - - fn check_captured_variables(&self, - closure_id: ast::NodeId, - span: Span) { - let freevar_mode = freevars::get_capture_mode(self.tcx(), closure_id); - freevars::with_freevars(self.tcx(), closure_id, |freevars| { - for freevar in freevars.iter() { - let var_id = ast_util::def_id_of_def(freevar.def).node; - let var_path = Rc::new(LpVar(var_id)); - self.check_if_path_is_moved(closure_id, span, - MovedInCapture, &var_path); - match freevar_mode { - freevars::CaptureByRef => { } - freevars::CaptureByValue => { - check_by_move_capture(self, closure_id, freevar.span, &*var_path); - } - } - } - }); - return; - - fn check_by_move_capture(this: &CheckLoanCtxt, - closure_id: ast::NodeId, - freevar_span: Span, - 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( - freevar_span, - format!("cannot move `{}` into closure \ - because it is borrowed", - this.bccx.loan_path_to_str( - move_path)).as_slice()); - this.bccx.span_note( - loan_span, - format!("borrow of `{}` occurs here", - this.bccx.loan_path_to_str( - &*loan_path)).as_slice()); - } - } - } - } - pub fn analyze_move_out_from(&self, expr_id: ast::NodeId, move_path: &LoanPath) @@ -805,89 +893,5 @@ impl<'a> CheckLoanCtxt<'a> { } } } - - pub fn check_call(&self, - _expr: &ast::Expr, - _callee: Option<@ast::Expr>, - _callee_span: Span, - _args: &[@ast::Expr]) { - // NB: This call to check for conflicting loans is not truly - // necessary, because the callee_id never issues new loans. - // However, I added it for consistency and lest the system - // should change in the future. - // - // FIXME(#6268) nested method calls - // self.check_for_conflicting_loans(callee_id); - } -} - -fn check_loans_in_local<'a>(this: &mut CheckLoanCtxt<'a>, - local: &ast::Local) { - visit::walk_local(this, local, ()); -} - -fn check_loans_in_expr<'a>(this: &mut CheckLoanCtxt<'a>, - expr: &ast::Expr) { - visit::walk_expr(this, expr, ()); - - debug!("check_loans_in_expr(expr={})", - expr.repr(this.tcx())); - - this.check_for_conflicting_loans(expr.id); - this.check_move_out_from_expr(expr); - - let method_map = this.bccx.tcx.method_map.borrow(); - match expr.node { - ast::ExprPath(..) => { - if !this.move_data.is_assignee(expr.id) { - let cmt = this.bccx.cat_expr_unadjusted(expr); - debug!("path cmt={}", cmt.repr(this.tcx())); - for lp in opt_loan_path(&cmt).iter() { - this.check_if_path_is_moved(expr.id, expr.span, MovedInUse, lp); - } - } - } - ast::ExprFnBlock(..) | ast::ExprProc(..) => { - this.check_captured_variables(expr.id, expr.span) - } - ast::ExprAssign(dest, _) | - ast::ExprAssignOp(_, dest, _) => { - this.check_assignment_expr(dest); - } - ast::ExprCall(f, ref args) => { - this.check_call(expr, Some(f), f.span, args.as_slice()); - } - ast::ExprMethodCall(_, _, ref args) => { - this.check_call(expr, None, expr.span, args.as_slice()); - } - ast::ExprIndex(_, rval) | ast::ExprBinary(_, _, rval) - if method_map.contains_key(&MethodCall::expr(expr.id)) => { - this.check_call(expr, None, expr.span, [rval]); - } - ast::ExprUnary(_, _) | ast::ExprIndex(_, _) - if method_map.contains_key(&MethodCall::expr(expr.id)) => { - this.check_call(expr, None, expr.span, []); - } - ast::ExprInlineAsm(ref ia) => { - for &(_, out) in ia.outputs.iter() { - this.check_assignment_expr(out); - } - } - _ => {} - } } -fn check_loans_in_pat<'a>(this: &mut CheckLoanCtxt<'a>, - pat: &ast::Pat) -{ - this.check_for_conflicting_loans(pat.id); - this.check_move_out_from_id(pat.id, pat.span); - visit::walk_pat(this, pat, ()); -} - -fn check_loans_in_block<'a>(this: &mut CheckLoanCtxt<'a>, - blk: &ast::Block) -{ - visit::walk_block(this, blk, ()); - this.check_for_conflicting_loans(blk.id); -} diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs index ce111d3ae29..0fbcf157dac 100644 --- a/src/librustc/middle/borrowck/mod.rs +++ b/src/librustc/middle/borrowck/mod.rs @@ -142,7 +142,7 @@ fn borrowck_fn(this: &mut BorrowckCtxt, body); check_loans::check_loans(this, &loan_dfcx, flowed_moves, - all_loans.as_slice(), body); + all_loans.as_slice(), decl, body); visit::walk_fn(this, fk, decl, body, sp, ()); } diff --git a/src/librustc/middle/borrowck/move_data.rs b/src/librustc/middle/borrowck/move_data.rs index 3e38aed20ce..f7c26292334 100644 --- a/src/librustc/middle/borrowck/move_data.rs +++ b/src/librustc/middle/borrowck/move_data.rs @@ -115,6 +115,7 @@ pub struct MovePath { pub next_sibling: MovePathIndex, } +#[deriving(PartialEq)] pub enum MoveKind { Declared, // When declared, variables start out "moved". MoveExpr, // Expression or binding that moves a variable diff --git a/src/test/compile-fail/borrowck-init-in-fru.rs b/src/test/compile-fail/borrowck-init-in-fru.rs index 97fc2b4d44c..6a42989b47b 100644 --- a/src/test/compile-fail/borrowck-init-in-fru.rs +++ b/src/test/compile-fail/borrowck-init-in-fru.rs @@ -16,6 +16,6 @@ struct point { fn main() { let mut origin: point; - origin = point {x: 10,.. origin}; //~ ERROR use of possibly uninitialized variable: `origin` + origin = point {x: 10,.. origin}; //~ ERROR use of possibly uninitialized variable: `origin.y` origin.clone(); } diff --git a/src/test/compile-fail/liveness-use-after-move.rs b/src/test/compile-fail/liveness-use-after-move.rs index 9927e2ea9da..4b578765f32 100644 --- a/src/test/compile-fail/liveness-use-after-move.rs +++ b/src/test/compile-fail/liveness-use-after-move.rs @@ -13,6 +13,6 @@ extern crate debug; fn main() { let x = box 5; let y = x; - println!("{:?}", *x); //~ ERROR use of moved value: `x` + println!("{:?}", *x); //~ ERROR use of partially moved value: `*x` y.clone(); } diff --git a/src/test/compile-fail/moves-based-on-type-access-to-field.rs b/src/test/compile-fail/moves-based-on-type-access-to-field.rs index 15cf176a0db..f09e80974bd 100644 --- a/src/test/compile-fail/moves-based-on-type-access-to-field.rs +++ b/src/test/compile-fail/moves-based-on-type-access-to-field.rs @@ -17,12 +17,6 @@ struct Foo { f: String, y: int } fn consume(_s: String) {} fn touch<A>(_a: &A) {} -fn f10() { - let x = Foo { f: "hi".to_string(), y: 3 }; - consume(x.f); - touch(&x.y); //~ ERROR use of partially moved value: `x` -} - fn f20() { let x = vec!("hi".to_string()); consume(x.move_iter().next().unwrap()); diff --git a/src/test/compile-fail/moves-sru-moved-field.rs b/src/test/compile-fail/moves-sru-moved-field.rs index 6f8353c6164..8b02740497d 100644 --- a/src/test/compile-fail/moves-sru-moved-field.rs +++ b/src/test/compile-fail/moves-sru-moved-field.rs @@ -26,13 +26,7 @@ fn test0(f: Foo, g: Noncopyable, h: Noncopyable) { fn test1(f: Foo, g: Noncopyable, h: Noncopyable) { // copying move-by-default fields from `f`, so move: let _b = Foo {noncopyable: g, ..f}; - let _c = Foo {noncopyable: h, ..f}; //~ ERROR use of partially moved value: `f` -} - -fn test2(f: Foo, g: Noncopyable) { - // move non-copyable field - let _b = Foo {copied: 22, moved: box 23, ..f}; - let _c = Foo {noncopyable: g, ..f}; //~ ERROR use of partially moved value: `f` + let _c = Foo {noncopyable: h, ..f}; //~ ERROR use of moved value: `f.moved` } fn main() {} diff --git a/src/test/compile-fail/use-after-move-self-based-on-type.rs b/src/test/compile-fail/use-after-move-self-based-on-type.rs index b93fc2680cd..b11650a6a4f 100644 --- a/src/test/compile-fail/use-after-move-self-based-on-type.rs +++ b/src/test/compile-fail/use-after-move-self-based-on-type.rs @@ -19,7 +19,7 @@ impl Drop for S { impl S { pub fn foo(self) -> int { self.bar(); - return self.x; //~ ERROR use of moved value: `self` + return self.x; //~ ERROR use of partially moved value: `self.x` } pub fn bar(self) {} diff --git a/src/test/compile-fail/use-after-move-self.rs b/src/test/compile-fail/use-after-move-self.rs index 8d1ab1bcd94..22c3ec7c341 100644 --- a/src/test/compile-fail/use-after-move-self.rs +++ b/src/test/compile-fail/use-after-move-self.rs @@ -16,7 +16,7 @@ struct S { impl S { pub fn foo(self) -> int { self.bar(); - return *self.x; //~ ERROR use of moved value: `self` + return *self.x; //~ ERROR use of partially moved value: `*self.x` } pub fn bar(self) {} |