diff --git a/src/librust/rust.rc b/src/librust/rust.rc index 36246b7a9a14c..00925a5700ada 100644 --- a/src/librust/rust.rc +++ b/src/librust/rust.rc @@ -152,15 +152,15 @@ fn cmd_help(args: &[~str]) -> ValidUsage { } match args { - [command_string] => print_usage(command_string), - _ => Invalid + [ref command_string] => print_usage(copy *command_string), + _ => Invalid } } fn cmd_test(args: &[~str]) -> ValidUsage { match args { - [filename] => { - let test_exec = Path(filename).filestem().unwrap() + "test~"; + [ref filename] => { + let test_exec = Path(*filename).filestem().unwrap() + "test~"; invoke("rustc", &[~"--test", filename.to_owned(), ~"-o", test_exec.to_owned()], rustc::main); let exit_code = run::process_status(~"./" + test_exec, []); @@ -172,8 +172,8 @@ fn cmd_test(args: &[~str]) -> ValidUsage { fn cmd_run(args: &[~str]) -> ValidUsage { match args { - [filename, ..prog_args] => { - let exec = Path(filename).filestem().unwrap() + "~"; + [ref filename, ..prog_args] => { + let exec = Path(*filename).filestem().unwrap() + "~"; invoke("rustc", &[filename.to_owned(), ~"-o", exec.to_owned()], rustc::main); let exit_code = run::process_status(~"./"+exec, prog_args); diff --git a/src/librustc/driver/driver.rs b/src/librustc/driver/driver.rs index 64d3b0e373cfd..65838f62498dc 100644 --- a/src/librustc/driver/driver.rs +++ b/src/librustc/driver/driver.rs @@ -263,8 +263,8 @@ pub fn compile_rest(sess: Session, time(time_passes, ~"loop checking", || middle::check_loop::check_crate(ty_cx, crate)); - let middle::moves::MoveMaps {moves_map, variable_moves_map, - moved_variables_set, capture_map} = + let middle::moves::MoveMaps {moves_map, moved_variables_set, + capture_map} = time(time_passes, ~"compute moves", || middle::moves::compute_moves(ty_cx, method_map, crate)); @@ -274,7 +274,6 @@ pub fn compile_rest(sess: Session, time(time_passes, ~"liveness checking", || middle::liveness::check_crate(ty_cx, method_map, - variable_moves_map, capture_map, crate)); let (root_map, write_guard_map) = diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 237a464dc9e96..183771956eae0 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -33,20 +33,23 @@ use syntax::codemap::span; struct CheckLoanCtxt<'self> { bccx: @BorrowckCtxt, - dfcx: &'self LoanDataFlow, + dfcx_loans: &'self LoanDataFlow, + move_data: move_data::FlowedMoveData, all_loans: &'self [Loan], reported: @mut HashSet<ast::node_id>, } pub fn check_loans(bccx: @BorrowckCtxt, - dfcx: &LoanDataFlow, + dfcx_loans: &LoanDataFlow, + move_data: move_data::FlowedMoveData, all_loans: &[Loan], body: &ast::blk) { debug!("check_loans(body id=%?)", body.node.id); let clcx = @mut CheckLoanCtxt { bccx: bccx, - dfcx: dfcx, + dfcx_loans: dfcx_loans, + move_data: move_data, all_loans: all_loans, reported: @mut HashSet::new(), }; @@ -62,8 +65,7 @@ pub fn check_loans(bccx: @BorrowckCtxt, enum MoveError { MoveOk, - MoveFromIllegalCmt(mc::cmt), - MoveWhileBorrowed(/*loan*/@LoanPath, /*loan*/span) + MoveWhileBorrowed(/*move*/@LoanPath, /*loan*/@LoanPath, /*loan*/span) } pub impl<'self> CheckLoanCtxt<'self> { @@ -79,7 +81,7 @@ pub impl<'self> CheckLoanCtxt<'self> { //! are issued for future scopes and thus they may have been //! *issued* but not yet be in effect. - for self.dfcx.each_bit_on_entry(scope_id) |loan_index| { + for self.dfcx_loans.each_bit_on_entry(scope_id) |loan_index| { let loan = &self.all_loans[loan_index]; if !op(loan) { return false; @@ -131,7 +133,7 @@ pub impl<'self> CheckLoanCtxt<'self> { //! we encounter `scope_id`. let mut result = ~[]; - for self.dfcx.each_gen_bit(scope_id) |loan_index| { + for self.dfcx_loans.each_gen_bit(scope_id) |loan_index| { result.push(loan_index); } return result; @@ -198,9 +200,9 @@ pub impl<'self> CheckLoanCtxt<'self> { loan1.repr(self.tcx()), loan2.repr(self.tcx())); - // Restrictions that would cause the new loan to be immutable: + // Restrictions that would cause the new loan to be illegal: let illegal_if = match loan2.mutbl { - m_mutbl => RESTR_ALIAS | RESTR_FREEZE | RESTR_MUTATE, + m_mutbl => RESTR_ALIAS | RESTR_FREEZE | RESTR_CLAIM, m_imm => RESTR_ALIAS | RESTR_FREEZE, m_const => RESTR_ALIAS, }; @@ -251,6 +253,29 @@ pub impl<'self> CheckLoanCtxt<'self> { } } + fn check_if_path_is_moved(&self, + id: ast::node_id, + span: span, + use_kind: MovedValueUseKind, + lp: @LoanPath) { + /*! + * Reports an error if `expr` (which should be a path) + * is using a moved/uninitialized value + */ + + debug!("check_if_path_is_moved(id=%?, use_kind=%?, lp=%s)", + id, use_kind, lp.repr(self.bccx.tcx)); + for self.move_data.each_move_of(id, lp) |move, moved_lp| { + self.bccx.report_use_of_moved_value( + span, + use_kind, + lp, + move, + moved_lp); + return; + } + } + fn check_assignment(&self, expr: @ast::expr) { // We don't use cat_expr() here because we don't want to treat // auto-ref'd parameters in overloaded operators as rvalues. @@ -261,48 +286,42 @@ pub impl<'self> CheckLoanCtxt<'self> { debug!("check_assignment(cmt=%s)", cmt.repr(self.tcx())); - // check that the value being assigned is declared as mutable - // and report an error otherwise. - match cmt.mutbl { - mc::McDeclared => { - // OK, but we have to mark arguments as requiring mut - // if they are assigned (other cases are handled by liveness, - // since we need to distinguish local variables assigned - // once vs those assigned multiple times) - match cmt.cat { - mc::cat_self(*) | - mc::cat_arg(*) => { - mark_variable_as_used_mut(self, cmt); - } - _ => {} + // Mutable values can be assigned, as long as they obey loans + // and aliasing restrictions: + if cmt.mutbl.is_mutable() { + if check_for_aliasable_mutable_writes(self, expr, cmt) { + if check_for_assignment_to_restricted_or_frozen_location( + self, expr, cmt) + { + // Safe, but record for lint pass later: + mark_variable_as_used_mut(self, cmt); } } - mc::McInherited => { - // OK, but we may have to add an entry to `used_mut_nodes` - mark_variable_as_used_mut(self, cmt); - } - mc::McReadOnly | mc::McImmutable => { - // Subtle: liveness guarantees that immutable local - // variables are only assigned once, so no need to - // report an error for an assignment to a local - // variable (note also that it is not legal to borrow - // for a local variable before it has been assigned - // for the first time). - if !self.is_local_variable(cmt) { - self.bccx.span_err( - expr.span, - fmt!("cannot assign to %s %s" - cmt.mutbl.to_user_str(), - self.bccx.cmt_to_str(cmt))); - } + return; + } + + // For immutable local variables, assignments are legal + // if they cannot already have been assigned + if self.is_local_variable(cmt) { + assert!(cmt.mutbl.is_immutable()); // no "const" locals + let lp = opt_loan_path(cmt).get(); + for self.move_data.each_assignment_of(expr.id, lp) |assign| { + self.bccx.report_reassigned_immutable_variable( + expr.span, + lp, + assign); return; } + return; } - if check_for_aliasable_mutable_writes(self, expr, cmt) { - check_for_assignment_to_restricted_or_frozen_location( - self, expr, cmt); - } + // Otherwise, just a plain error. + self.bccx.span_err( + expr.span, + fmt!("cannot assign to %s %s" + cmt.mutbl.to_user_str(), + self.bccx.cmt_to_str(cmt))); + return; fn mark_variable_as_used_mut(this: &CheckLoanCtxt, cmt: mc::cmt) { @@ -538,18 +557,12 @@ pub impl<'self> CheckLoanCtxt<'self> { let cmt = self.bccx.cat_expr(ex); match self.analyze_move_out_from_cmt(cmt) { MoveOk => {} - MoveFromIllegalCmt(_) => { - self.bccx.span_err( - cmt.span, - fmt!("cannot move out of %s", - self.bccx.cmt_to_str(cmt))); - } - MoveWhileBorrowed(loan_path, loan_span) => { + MoveWhileBorrowed(move_path, loan_path, loan_span) => { self.bccx.span_err( cmt.span, fmt!("cannot move out of `%s` \ because it is borrowed", - self.bccx.loan_path_to_str(loan_path))); + self.bccx.loan_path_to_str(move_path))); self.bccx.span_note( loan_span, fmt!("borrow of `%s` occurs here", @@ -561,29 +574,7 @@ pub impl<'self> CheckLoanCtxt<'self> { } fn analyze_move_out_from_cmt(&self, cmt: mc::cmt) -> MoveError { - debug!("check_move_out_from_cmt(cmt=%s)", cmt.repr(self.tcx())); - - match cmt.cat { - // Rvalues, locals, and arguments can be moved: - mc::cat_rvalue | mc::cat_local(_) | - mc::cat_arg(_) | mc::cat_self(_) => {} - - // It seems strange to allow a move out of a static item, - // but what happens in practice is that you have a - // reference to a constant with a type that should be - // moved, like `None::<~int>`. The type of this constant - // is technically `Option<~int>`, which moves, but we know - // that the content of static items will never actually - // contain allocated pointers, so we can just memcpy it. - mc::cat_static_item => {} - - mc::cat_deref(_, _, mc::unsafe_ptr(*)) => {} - - // Nothing else. - _ => { - return MoveFromIllegalCmt(cmt); - } - } + debug!("analyze_move_out_from_cmt(cmt=%s)", cmt.repr(self.tcx())); // FIXME(#4384) inadequare if/when we permit `move a.b` @@ -591,7 +582,7 @@ pub impl<'self> CheckLoanCtxt<'self> { for opt_loan_path(cmt).each |&lp| { for self.each_in_scope_restriction(cmt.id, lp) |loan, _| { // Any restriction prevents moves. - return MoveWhileBorrowed(loan.loan_path, loan.span); + return MoveWhileBorrowed(lp, loan.loan_path, loan.span); } } @@ -631,54 +622,53 @@ fn check_loans_in_fn<'a>(fk: &visit::fn_kind, visit::fk_anon(*) | visit::fk_fn_block(*) => { - let fty = ty::node_id_to_type(this.tcx(), id); - let fty_sigil = ty::ty_closure_sigil(fty); - check_moves_from_captured_variables(this, id, fty_sigil); + check_captured_variables(this, id, sp); } } visit::visit_fn(fk, decl, body, sp, id, this, visitor); - fn check_moves_from_captured_variables(this: @mut CheckLoanCtxt, - id: ast::node_id, - fty_sigil: ast::Sigil) { - match fty_sigil { - ast::ManagedSigil | ast::OwnedSigil => { - let cap_vars = this.bccx.capture_map.get(&id); - for cap_vars.each |cap_var| { - match cap_var.mode { - moves::CapRef | moves::CapCopy => { loop; } - moves::CapMove => { } - } - let def_id = ast_util::def_id_of_def(cap_var.def).node; - let ty = ty::node_id_to_type(this.tcx(), def_id); - let cmt = this.bccx.cat_def(id, cap_var.span, - ty, cap_var.def); - let move_err = this.analyze_move_out_from_cmt(cmt); - match move_err { - MoveOk => {} - MoveFromIllegalCmt(move_cmt) => { - this.bccx.span_err( - cap_var.span, - fmt!("illegal by-move capture of %s", - this.bccx.cmt_to_str(move_cmt))); - } - MoveWhileBorrowed(loan_path, loan_span) => { - this.bccx.span_err( - cap_var.span, - fmt!("cannot move `%s` into closure \ - because it is borrowed", - this.bccx.loan_path_to_str(loan_path))); - this.bccx.span_note( - loan_span, - fmt!("borrow of `%s` occurs here", - this.bccx.loan_path_to_str(loan_path))); - } - } + fn check_captured_variables(this: @mut CheckLoanCtxt, + closure_id: ast::node_id, + span: span) { + let cap_vars = this.bccx.capture_map.get(&closure_id); + for cap_vars.each |cap_var| { + match cap_var.mode { + moves::CapRef | moves::CapCopy => { + let var_id = ast_util::def_id_of_def(cap_var.def).node; + let lp = @LpVar(var_id); + this.check_if_path_is_moved(closure_id, span, + MovedInCapture, lp); + } + moves::CapMove => { + check_by_move_capture(this, closure_id, cap_var); + } + } + } + return; + + fn check_by_move_capture(this: @mut CheckLoanCtxt, + closure_id: ast::node_id, + cap_var: &moves::CaptureVar) { + let var_id = ast_util::def_id_of_def(cap_var.def).node; + let ty = ty::node_id_to_type(this.tcx(), var_id); + let cmt = this.bccx.cat_def(closure_id, cap_var.span, + ty, cap_var.def); + let move_err = this.analyze_move_out_from_cmt(cmt); + match move_err { + MoveOk => {} + MoveWhileBorrowed(move_path, loan_path, loan_span) => { + this.bccx.span_err( + cap_var.span, + fmt!("cannot move `%s` into closure \ + because it is borrowed", + this.bccx.loan_path_to_str(move_path))); + this.bccx.span_note( + loan_span, + fmt!("borrow of `%s` occurs here", + this.bccx.loan_path_to_str(loan_path))); } } - - ast::BorrowedSigil => {} } } } @@ -692,11 +682,11 @@ fn check_loans_in_local<'a>(local: @ast::local, fn check_loans_in_expr<'a>(expr: @ast::expr, this: @mut CheckLoanCtxt<'a>, vt: visit::vt<@mut CheckLoanCtxt<'a>>) { + visit::visit_expr(expr, this, vt); + debug!("check_loans_in_expr(expr=%s)", expr.repr(this.tcx())); - visit::visit_expr(expr, this, vt); - this.check_for_conflicting_loans(expr.id); if this.bccx.moves_map.contains(&expr.id) { @@ -704,6 +694,17 @@ fn check_loans_in_expr<'a>(expr: @ast::expr, } match expr.node { + ast::expr_self | + ast::expr_path(*) => { + if !this.move_data.is_assignee(expr.id) { + let cmt = this.bccx.cat_expr_unadjusted(expr); + debug!("path cmt=%s", cmt.repr(this.tcx())); + for opt_loan_path(cmt).each |&lp| { + this.check_if_path_is_moved(expr.id, expr.span, + MovedInUse, lp); + } + } + } ast::expr_assign(dest, _) | ast::expr_assign_op(_, dest, _) => { this.check_assignment(dest); diff --git a/src/librustc/middle/borrowck/doc.rs b/src/librustc/middle/borrowck/doc.rs index 1e09fbe71843c..cb3983117e97c 100644 --- a/src/librustc/middle/borrowck/doc.rs +++ b/src/librustc/middle/borrowck/doc.rs @@ -13,13 +13,48 @@ # The Borrow Checker This pass has the job of enforcing memory safety. This is a subtle -topic. The only way I know how to explain it is terms of a formal -model, so that's what I'll do. +topic. This docs aim to explain both the practice and the theory +behind the borrow checker. They start with a high-level overview of +how it works, and then proceed to dive into the theoretical +background. Finally, they go into detail on some of the more subtle +aspects. + +# Table of contents + +These docs are long. Search for the section you are interested in. + +- Overview +- Formal model +- Borrowing and loans +- Moves and initialization +- Future work + +# Overview + +The borrow checker checks one function at a time. It operates in two +passes. The first pass, called `gather_loans`, walks over the function +and identifies all of the places where borrows (e.g., `&` expressions +and `ref` bindings) and moves (copies or captures of a linear value) +occur. It also tracks initialization sites. For each borrow and move, +it checks various basic safety conditions at this time (for example, +that the lifetime of the borrow doesn't exceed the lifetime of the +value being borrowed, or that there is no move out of an `&T` +pointee). + +It then uses the dataflow module to propagate which of those borrows +may be in scope at each point in the procedure. A loan is considered +to come into scope at the expression that caused it and to go out of +scope when the lifetime of the resulting borrowed pointer expires. + +Once the in-scope loans are known for each point in the program, the +borrow checker walks the IR again in a second pass called +`check_loans`. This pass examines each statement and makes sure that +it is safe with respect to the in-scope loans. # Formal model -Let's consider a simple subset of Rust in which you can only borrow -from lvalues like so: +Throughout the docs we'll consider a simple subset of Rust in which +you can only borrow from lvalues, defined like so: LV = x | LV.f | *LV @@ -42,9 +77,11 @@ struct name and we assume structs are declared like so: SD = struct S<'LT...> { (f: TY)... } -# An intuitive explanation +# Borrowing and loans -## Issuing loans +## An intuitive explanation + +### Issuing loans Now, imagine we had a program like this: @@ -60,691 +97,766 @@ This is of course dangerous because mutating `x` will free the old value and hence invalidate `y`. The borrow checker aims to prevent this sort of thing. -### Loans +#### Loans and restrictions The way the borrow checker works is that it analyzes each borrow expression (in our simple model, that's stuff like `&LV`, though in real life there are a few other cases to consider). For each borrow -expression, it computes a vector of loans: - - LOAN = (LV, LT, PT, LK) - PT = Partial | Total - LK = MQ | RESERVE - -Each `LOAN` tuple indicates some sort of restriction on what can be -done to the lvalue `LV`; `LV` will always be a path owned by the -current stack frame. These restrictions are called "loans" because -they are always the result of a borrow expression. - -Every loan has a lifetime `LT` during which those restrictions are in -effect. The indicator `PT` distinguishes between *total* loans, in -which the LV itself was borrowed, and *partial* loans, which means -that some content ownwed by LV was borrowed. - -The final element in the loan tuple is the *loan kind* `LK`. There -are four kinds: mutable, immutable, const, and reserve: - -- A "mutable" loan means that LV may be written to through an alias, and - thus LV cannot be written to directly or immutably aliased (remember - that we preserve the invariant that any given value can only be - written to through one path at a time; hence if there is a mutable - alias to LV, then LV cannot be written directly until this alias is - out of scope). - -- An "immutable" loan means that LV must remain immutable. Hence it - cannot be written, but other immutable aliases are permitted. - -- A "const" loan means that an alias to LV exists. LV may still be - written or frozen. - -- A "reserve" loan is the strongest case. It prevents both mutation - and aliasing of any kind, including `&const` loans. Reserve loans - are a side-effect of borrowing an `&mut` loan. - -In addition to affecting mutability, a loan of any kind implies that -LV cannot be moved. - -### Example - -To give you a better feeling for what a loan is, let's look at three -loans that would be issued as a result of the borrow `&(*x).f` in the -example above: - - ((*x).f, Total, mut, 'a) - (*x, Partial, mut, 'a) - (x, Partial, mut, 'a) - -The first loan states that the expression `(*x).f` has been loaned -totally as mutable for the lifetime `'a`. This first loan would -prevent an assignment `(*x).f = ...` from occurring during the -lifetime `'a`. - -Now let's look at the second loan. You may have expected that each -borrow would result in only one loan. But this is not the case. -Instead, there will be loans for every path where mutation might -affect the validity of the borrowed pointer that is created (in some -cases, there can even be multiple loans per path, see the section on -"Borrowing in Calls" below for the gory details). The reason for this -is to prevent actions that would indirectly affect the borrowed path. -In this case, we wish to ensure that `(*x).f` is not mutated except -through the mutable alias `y`. Therefore, we must not only prevent an -assignment to `(*x).f` but also an assignment like `*x = Foo {...}`, -as this would also mutate the field `f`. To do so, we issue a -*partial* mutable loan for `*x` (the loan is partial because `*x` -itself was not borrowed). This partial loan will cause any attempt to -assign to `*x` to be flagged as an error. - -Because both partial and total loans prevent assignments, you may -wonder why we bother to distinguish between them. The reason for this -distinction has to do with preventing double borrows. In particular, -it is legal to borrow both `&mut x.f` and `&mut x.g` simultaneously, -but it is not legal to borrow `&mut x.f` twice. In the borrow checker, -the first case would result in two *partial* mutable loans of `x` -(along with one total mutable loan of `x.f` and one of `x.g) whereas -the second would result in two *total* mutable loans of `x.f` (along -with two partial mutable loans of `x`). Multiple *total mutable* loan -for the same path are not permitted, but multiple *partial* loans (of -any mutability) are permitted. - -Finally, we come to the third loan. This loan is a partial mutable -loan of `x`. This loan prevents us from reassigning `x`, which would -be bad for two reasons. First, it would change the value of `(*x).f` -but, even worse, it would cause the pointer `y` to become a dangling -pointer. Bad all around. - -## Checking for illegal assignments, moves, and reborrows +expression, it computes a `Loan`, which is a data structure that +records (1) the value being borrowed, (2) the mutability and scope of +the borrow, and (3) a set of restrictions. In the code, `Loan` is a +struct defined in `middle::borrowck`. Formally, we define `LOAN` as +follows: + + LOAN = (LV, LT, MQ, RESTRICTION*) + RESTRICTION = (LV, ACTION*) + ACTION = MUTATE | CLAIM | FREEZE | ALIAS + +Here the `LOAN` tuple defines the lvalue `LV` being borrowed; the +lifetime `LT` of that borrow; the mutability `MQ` of the borrow; and a +list of restrictions. The restrictions indicate actions which, if +taken, could invalidate the loan and lead to type safety violations. + +Each `RESTRICTION` is a pair of a restrictive lvalue `LV` (which will +either be the path that was borrowed or some prefix of the path that +was borrowed) and a set of restricted actions. There are three kinds +of actions that may be restricted for the path `LV`: + +- `MUTATE` means that `LV` cannot be assigned to; +- `CLAIM` means that the `LV` cannot be borrowed mutably; +- `FREEZE` means that the `LV` cannot be borrowed immutably; +- `ALIAS` means that `LV` cannot be aliased in any way (not even `&const`). + +Finally, it is never possible to move from an lvalue that appears in a +restriction. This implies that the "empty restriction" `(LV, [])`, +which contains an empty set of actions, still has a purpose---it +prevents moves from `LV`. I chose not to make `MOVE` a fourth kind of +action because that would imply that sometimes moves are permitted +from restrictived values, which is not the case. + +#### Example + +To give you a better feeling for what kind of restrictions derived +from a loan, let's look at the loan `L` that would be issued as a +result of the borrow `&mut (*x).f` in the example above: + + L = ((*x).f, 'a, mut, RS) where + RS = [((*x).f, [MUTATE, CLAIM, FREEZE]), + (*x, [MUTATE, CLAIM, FREEZE]), + (x, [MUTATE, CLAIM, FREEZE])] + +The loan states that the expression `(*x).f` has been loaned as +mutable for the lifetime `'a`. Because the loan is mutable, that means +that the value `(*x).f` may be mutated via the newly created borrowed +pointer (and *only* via that pointer). This is reflected in the +restrictions `RS` that accompany the loan. + +The first restriction `((*x).f, [MUTATE, CLAIM, FREEZE])` states that +the lender may not mutate nor freeze `(*x).f`. Mutation is illegal +because `(*x).f` is only supposed to be mutated via the new borrowed +pointer, not by mutating the original path `(*x).f`. Freezing is +illegal because the path now has an `&mut` alias; so even if we the +lender were to consider `(*x).f` to be immutable, it might be mutated +via this alias. Both of these restrictions are temporary. They will be +enforced for the lifetime `'a` of the loan. After the loan expires, +the restrictions no longer apply. + +The second restriction on `*x` is interesting because it does not +apply to the path that was lent (`(*x).f`) but rather to a prefix of +the borrowed path. This is due to the rules of inherited mutability: +if the user were to assign to (or freeze) `*x`, they would indirectly +overwrite (or freeze) `(*x).f`, and thus invalidate the borrowed +pointer that was created. In general it holds that when a path is +lent, restrictions are issued for all the owning prefixes of that +path. In this case, the path `*x` owns the path `(*x).f` and, +because `x` is an owned pointer, the path `x` owns the path `*x`. +Therefore, borrowing `(*x).f` yields restrictions on both +`*x` and `x`. + +### Checking for illegal assignments, moves, and reborrows Once we have computed the loans introduced by each borrow, the borrow -checker will determine the full set of loans in scope at each -expression and use that to decide whether that expression is legal. -Remember that the scope of loan is defined by its lifetime LT. We -sometimes say that a loan which is in-scope at a particular point is -an "outstanding loan". - -The kinds of expressions which in-scope loans can render illegal are -*assignments*, *moves*, and *borrows*. - -An assignments to an lvalue LV is illegal if there is in-scope mutable -or immutable loan for LV. Assignment with an outstanding mutable loan -is illegal because then the `&mut` pointer is supposed to be the only -way to mutate the value. Assignment with an outstanding immutable -loan is illegal because the value is supposed to be immutable at that -point. - -A move from an lvalue LV is illegal if there is any sort of -outstanding loan. - -A borrow expression may be illegal if any of the loans which it -produces conflict with other outstanding loans. Two loans are -considered compatible if one of the following conditions holds: - -- At least one loan is a const loan. -- Both loans are partial loans. -- Both loans are immutable. - -Any other combination of loans is illegal. - -# The set of loans that results from a borrow expression - -Here we'll define four functions---MUTATE, FREEZE, ALIAS, and -TAKE---which are all used to compute the set of LOANs that result -from a borrow expression. The first three functions each have -a similar type signature: - - MUTATE(LV, LT, PT) -> LOANS - FREEZE(LV, LT, PT) -> LOANS - ALIAS(LV, LT, PT) -> LOANS - -MUTATE, FREEZE, and ALIAS are used when computing the loans result -from mutable, immutable, and const loans respectively. For example, -the loans resulting from an expression like `&mut (*x).f` would be -computed by `MUTATE((*x).f, LT, Total)`, where `LT` is the lifetime of -the resulting pointer. Similarly the loans for `&(*x).f` and `&const -(*x).f` would be computed by `FREEZE((*x).f, LT, Total)` and -`ALIAS((*x).f, LT, Total)` respectively. (Actually this is a slight -simplification; see the section below on Borrows in Calls for the full -gory details) - -The names MUTATE, FREEZE, and ALIAS are intended to suggest the -semantics of `&mut`, `&`, and `&const` borrows respectively. `&mut`, -for example, creates a mutable alias of LV. `&` causes the borrowed -value to be frozen (immutable). `&const` does neither but does -introduce an alias to be the borrowed value. - -Each of these three functions is only defined for some inputs. That -is, it may occur that some particular borrow is not legal. For -example, it is illegal to make an `&mut` loan of immutable data. In -that case, the MUTATE() function is simply not defined (in the code, -it returns a Result<> condition to indicate when a loan would be -illegal). - -The final function, RESERVE, is used as part of borrowing an `&mut` -pointer. Due to the fact that it is used for one very particular -purpose, it has a rather simpler signature than the others: - - RESERVE(LV, LT) -> LOANS - -It is explained when we come to that case. - -## The function MUTATE() - -Here we use [inference rules][ir] to define the MUTATE() function. -We will go case by case for the various kinds of lvalues that -can be borrowed. - -[ir]: http://en.wikipedia.org/wiki/Rule_of_inference - -### Mutating local variables - -The rule for mutating local variables is as follows: - - Mutate-Variable: - LT <= Scope(x) - Mut(x) = Mut - -------------------------------------------------- - MUTATE(x, LT, PT) = (x, LT, PT, mut) - -Here `Scope(x)` is the lifetime of the block in which `x` was declared -and `Mut(x)` indicates the mutability with which `x` was declared. -This rule simply states that you can only create a mutable alias -to a variable if it is mutable, and that alias cannot outlive the -stack frame in which the variable is declared. - -### Mutating fields and owned pointers - -As it turns out, the rules for mutating fields and mutating owned -pointers turn out to be quite similar. The reason is that the -expressions `LV.f` and `*LV` are both owned by their base expression -`LV`. So basically the result of mutating `LV.f` or `*LV` is computed -by adding a loan for `LV.f` or `*LV` and then the loans for a partial -take of `LV`: - - Mutate-Field: - MUTATE(LV, LT, Partial) = LOANS - ------------------------------------------------------------ - MUTATE(LV.f, LT, PT) = LOANS, (LV.F, LT, PT, mut) - - Mutate-Owned-Ptr: - Type(LV) = ~Ty - MUTATE(LV, LT, Partial) = LOANS - ------------------------------------------------------------ - MUTATE(*LV, LT, PT) = LOANS, (*LV, LT, PT, mut) - -Note that while our micro-language only has fields, the slight -variations on the `Mutate-Field` rule are used for any interior content -that appears in the full Rust language, such as the contents of a -tuple, fields in a struct, or elements of a fixed-length vector. - -### Mutating dereferenced borrowed pointers - -The rule for borrowed pointers is by far the most complicated: - - Mutate-Mut-Borrowed-Ptr: - Type(LV) = <_P mut Ty // (1) - LT <= LT_P // (2) - RESERVE(LV, LT) = LOANS // (3) - ------------------------------------------------------------ - MUTATE(*LV, LT, PT) = LOANS, (*LV, LT, PT, Mut) - -Condition (1) states that only a mutable borrowed pointer can be -taken. Condition (2) states that the lifetime of the alias must be -less than the lifetime of the borrowed pointer being taken. - -Conditions (3) and (4) are where things get interesting. The intended -semantics of the borrow is that the new `&mut` pointer is the only one -which has the right to modify the data; the original `&mut` pointer -must not be used for mutation. Because borrowed pointers do not own -their content nor inherit mutability, we must be particularly cautious -of aliases, which could permit the original borrowed pointer to be -reached from another path and thus circumvent our loans. - -Here is one example of what could go wrong if we ignore clause (4): - - let x: &mut T; - ... - let y = &mut *x; // Only *y should be able to mutate... - let z = &const x; - **z = ...; // ...but here **z is still able to mutate! +checker uses a data flow propagation to compute the full set of loans +in scope at each expression and then uses that set to decide whether +that expression is legal. Remember that the scope of loan is defined +by its lifetime LT. We sometimes say that a loan which is in-scope at +a particular point is an "outstanding loan", aand the set of +restrictions included in those loans as the "outstanding +restrictions". + +The kinds of expressions which in-scope loans can render illegal are: +- *assignments* (`lv = v`): illegal if there is an in-scope restriction + against mutating `lv`; +- *moves*: illegal if there is any in-scope restriction on `lv` at all; +- *mutable borrows* (`&mut lv`): illegal there is an in-scope restriction + against mutating `lv` or aliasing `lv`; +- *immutable borrows* (`&lv`): illegal there is an in-scope restriction + against freezing `lv` or aliasing `lv`; +- *read-only borrows* (`&const lv`): illegal there is an in-scope restriction + against aliasing `lv`. -Another possible error could occur with moves: +## Formal rules - let x: &mut T; - ... - let y = &mut *x; // Issues loan: (*x, LT, Total, Mut) - let z = x; // moves from x - *z = ...; // Mutates *y indirectly! Bad. - -In both of these cases, the problem is that when creating the alias -`y` we would only issue a loan preventing assignment through `*x`. -But this loan can be easily circumvented by moving from `x` or -aliasing it. Note that, in the first example, the alias of `x` was -created using `&const`, which is a particularly weak form of alias. - -The danger of aliases can also occur when the `&mut` pointer itself -is already located in an alias location, as here: - - let x: @mut &mut T; // or &mut &mut T, &&mut T, - ... // &const &mut T, @&mut T, etc - let y = &mut **x; // Only *y should be able to mutate... - let z = x; - **z = ...; // ...but here **z is still able to mutate! - -When we cover the rules for RESERVE, we will see that it would -disallow this case, because MUTATE can only be applied to canonical -lvalues which are owned by the current stack frame. - -It might be the case that if `&const` and `@const` pointers were -removed, we could do away with RESERVE and simply use MUTATE instead. -But we have to be careful about the final example in particular, since -dynamic freezing would not be sufficient to prevent this example. -Perhaps a combination of MUTATE with a predicate OWNED(LV). - -One final detail: unlike every other case, when we calculate the loans -using RESERVE we do not use the original lifetime `LT` but rather -`GLB(Scope(LV), LT)`. What this says is: - -### Mutating dereferenced managed pointers - -Because the correctness of managed pointer loans is checked dynamically, -the rule is quite simple: - - Mutate-Mut-Managed-Ptr: - Type(LV) = @mut Ty - Add ROOT-FREEZE annotation for *LV with lifetime LT - ------------------------------------------------------------ - MUTATE(*LV, LT, Total) = [] - -No loans are issued. Instead, we add a side annotation that causes -`*LV` to be rooted and frozen on entry to LV. You could rephrase -these rules as having multiple returns values, or rephrase this as a -kind of loan, but whatever. - -One interesting point is that *partial takes* of `@mut` are forbidden. -This is not for any soundness reason but just because it is clearer -for users when `@mut` values are either lent completely or not at all. - -## The function FREEZE - -The rules for FREEZE are pretty similar to MUTATE. The first four -cases I'll just present without discussion, as the reasoning is -quite analogous to the MUTATE case: - - Freeze-Variable: - LT <= Scope(x) - -------------------------------------------------- - FREEZE(x, LT, PT) = (x, LT, PT, imm) - - Freeze-Field: - FREEZE(LV, LT, Partial) = LOANS - ------------------------------------------------------------ - FREEZE(LV.f, LT, PT) = LOANS, (LV.F, LT, PT, imm) - - Freeze-Owned-Ptr: - Type(LV) = ~Ty - FREEZE(LV, LT, Partial) = LOANS - ------------------------------------------------------------ - FREEZE(*LV, LT, PT) = LOANS, (*LV, LT, PT, imm) - - Freeze-Mut-Borrowed-Ptr: - Type(LV) = <_P mut Ty - LT <= LT_P - RESERVE(LV, LT) = LOANS - ------------------------------------------------------------ - FREEZE(*LV, LT, PT) = LOANS, (*LV, LT, PT, Imm) - - Freeze-Mut-Managed-Ptr: - Type(LV) = @mut Ty - Add ROOT-FREEZE annotation for *LV with lifetime LT - ------------------------------------------------------------ - Freeze(*LV, LT, Total) = [] - -The rule to "freeze" an immutable borrowed pointer is quite -simple, since the content is already immutable: - - Freeze-Imm-Borrowed-Ptr: - Type(LV) = <_P Ty // (1) - LT <= LT_P // (2) - ------------------------------------------------------------ - FREEZE(*LV, LT, PT) = LOANS, (*LV, LT, PT, Mut) - -The final two rules pertain to borrows of `@Ty`. There is a bit of -subtlety here. The main problem is that we must guarantee that the -managed box remains live for the entire borrow. We can either do this -dynamically, by rooting it, or (better) statically, and hence there -are two rules: - - Freeze-Imm-Managed-Ptr-1: - Type(LV) = @Ty - Add ROOT annotation for *LV - ------------------------------------------------------------ - FREEZE(*LV, LT, PT) = [] - - Freeze-Imm-Managed-Ptr-2: - Type(LV) = @Ty - LT <= Scope(LV) - Mut(LV) = imm - LV is not moved - ------------------------------------------------------------ - FREEZE(*LV, LT, PT) = [] - -The intention of the second rule is to avoid an extra root if LV -serves as a root. In that case, LV must (1) outlive the borrow; (2) -be immutable; and (3) not be moved. - -## The ALIAS function - -The function ALIAS is used for `&const` loans but also to handle one -corner case concerning function arguments (covered in the section -"Borrows in Calls" below). It computes the loans that result from -observing that there is a pointer to `LV` and thus that pointer must -remain valid. - -The first two rules are simple: - - Alias-Variable: - LT <= Scope(x) - -------------------------------------------------- - ALIAS(x, LT, PT) = (x, LT, PT, Const) - - Alias-Field: - ALIAS(LV, LT, Partial) = LOANS - ------------------------------------------------------------ - ALIAS(LV.f, LT, PT) = LOANS, (LV.F, LT, PT, Const) - -### Aliasing owned pointers - -The rule for owned pointers is somewhat interesting: - - Alias-Owned-Ptr: - Type(LV) = ~Ty - FREEZE(LV, LT, Partial) = LOANS - ------------------------------------------------------------ - ALIAS(*LV, LT, PT) = LOANS, (*LV, LT, PT, Const) - -Here we *freeze* the base `LV`. The reason is that if an owned -pointer is mutated it frees its content, which means that the alias to -`*LV` would become a dangling pointer. - -### Aliasing borrowed pointers - -The rule for borrowed pointers is quite simple, because borrowed -pointers do not own their content and thus do not play a role in -keeping it live: - - Alias-Borrowed-Ptr: - Type(LV) = <_P MQ Ty - LT <= LT_P - ------------------------------------------------------------ - ALIAS(*LV, LT, PT) = [] - -Basically, the existence of a borrowed pointer to some memory with -lifetime LT_P is proof that the memory can safely be aliased for any -lifetime LT <= LT_P. - -### Aliasing managed pointers - -The rules for aliasing managed pointers are similar to those -used with FREEZE, except that they apply to all manager pointers -regardles of mutability: - - Alias-Managed-Ptr-1: - Type(LV) = @MQ Ty - Add ROOT annotation for *LV - ------------------------------------------------------------ - ALIAS(*LV, LT, PT) = [] - - Alias-Managed-Ptr-2: - Type(LV) = @MQ Ty - LT <= Scope(LV) - Mut(LV) = imm - LV is not moved - ------------------------------------------------------------ - ALIAS(*LV, LT, PT) = [] - -## The RESERVE function - -The final function, RESERVE, is used for loans of `&mut` pointers. As -discussed in the section on the function MUTATE, we must be quite -careful when "re-borrowing" an `&mut` pointer to ensure that the original -`&mut` pointer can no longer be used to mutate. - -There are a couple of dangers to be aware of: - -- `&mut` pointers do not inherit mutability. Therefore, if you have - an lvalue LV with type `&mut T` and you freeze `LV`, you do *not* - freeze `*LV`. This is quite different from an `LV` with type `~T`. - -- Also, because they do not inherit mutability, if the `&mut` pointer - lives in an aliased location, then *any alias* can be used to write! - -As a consequence of these two rules, RESERVE can only be successfully -invoked on an lvalue LV that is *owned by the current stack frame*. -This ensures that there are no aliases that are not visible from the -outside. Moreover, Reserve loans are incompatible with all other -loans, even Const loans. This prevents any aliases from being created -within the current function. +Now that we hopefully have some kind of intuitive feeling for how the +borrow checker works, let's look a bit more closely now at the precise +conditions that it uses. For simplicity I will ignore const loans. -### Reserving local variables +I will present the rules in a modified form of standard inference +rules, which looks as as follows: -The rule for reserving a variable is generally straightforward but -with one interesting twist: + PREDICATE(X, Y, Z) // Rule-Name + Condition 1 + Condition 2 + Condition 3 + +The initial line states the predicate that is to be satisfied. The +indented lines indicate the conditions that must be met for the +predicate to be satisfied. The right-justified comment states the name +of this rule: there are comments in the borrowck source referencing +these names, so that you can cross reference to find the actual code +that corresponds to the formal rule. + +### The `gather_loans` pass - Reserve-Variable: - -------------------------------------------------- - RESERVE(x, LT) = (x, LT, Total, Reserve) +We start with the `gather_loans` pass, which walks the AST looking for +borrows. For each borrow, there are three bits of information: the +lvalue `LV` being borrowed and the mutability `MQ` and lifetime `LT` +of the resulting pointer. Given those, `gather_loans` applies three +validity tests: -The twist here is that the incoming lifetime is not required to -be a subset of the incoming variable, unlike every other case. To -see the reason for this, imagine the following function: +1. `MUTABILITY(LV, MQ)`: The mutability of the borrowed pointer is +compatible with the mutability of `LV` (i.e., not borrowing immutable +data as mutable). - struct Foo { count: uint } - fn count_field(x: &'a mut Foo) -> &'a mut count { - &mut (*x).count - } +2. `LIFETIME(LV, LT, MQ)`: The lifetime of the borrow does not exceed +the lifetime of the value being borrowed. This pass is also +responsible for inserting root annotations to keep managed values +alive and for dynamically freezing `@mut` boxes. -This function consumes one `&mut` pointer and returns another with the -same lifetime pointing at a particular field. The borrow for the -`&mut` expression will result in a call to `RESERVE(x, 'a)`, which is -intended to guarantee that `*x` is not later aliased or used to -mutate. But the lifetime of `x` is limited to the current function, -which is a sublifetime of the parameter `'a`, so the rules used for -MUTATE, FREEZE, and ALIAS (which require that the lifetime of the loan -not exceed the lifetime of the variable) would result in an error. +3. `RESTRICTIONS(LV, ACTIONS) = RS`: This pass checks and computes the +restrictions to maintain memory safety. These are the restrictions +that will go into the final loan. We'll discuss in more detail below. -Nonetheless this function is perfectly legitimate. After all, the -caller has moved in an `&mut` pointer with lifetime `'a`, and thus has -given up their right to mutate the value for the remainder of `'a`. -So it is fine for us to return a pointer with the same lifetime. +## Checking mutability -The reason that RESERVE differs from the other functions is that -RESERVE is not responsible for guaranteeing that the pointed-to data -will outlive the borrowed pointer being created. After all, `&mut` -values do not own the data they point at. +Checking mutability is fairly straightforward. We just want to prevent +immutable data from being borrowed as mutable. Note that it is ok to +borrow mutable data as immutable, since that is simply a +freeze. Formally we define a predicate `MUTABLE(LV, MQ)` which, if +defined, means that "borrowing `LV` with mutability `MQ` is ok. The +Rust code corresponding to this predicate is the function +`check_mutability` in `middle::borrowck::gather_loans`. -### Reserving owned content +### Checking mutability of variables -The rules for fields and owned pointers are very straightforward: +*Code pointer:* Function `check_mutability()` in `gather_loans/mod.rs`, +but also the code in `mem_categorization`. - Reserve-Field: - RESERVE(LV, LT) = LOANS - ------------------------------------------------------------ - RESERVE(LV.f, LT) = LOANS, (LV.F, LT, Total, Reserve) +Let's begin with the rules for variables, which state that if a +variable is declared as mutable, it may be borrowed any which way, but +otherwise the variable must be borrowed as immutable or const: - Reserve-Owned-Ptr: - Type(LV) = ~Ty - RESERVE(LV, LT) = LOANS - ------------------------------------------------------------ - RESERVE(*LV, LT) = LOANS, (*LV, LT, Total, Reserve) + MUTABILITY(X, MQ) // M-Var-Mut + DECL(X) = mut -### Reserving `&mut` borrowed pointers + MUTABILITY(X, MQ) // M-Var-Imm + DECL(X) = imm + MQ = imm | const -Unlike other borrowed pointers, `&mut` pointers are unaliasable, -so we can reserve them like everything else: +### Checking mutability of owned content - Reserve-Mut-Borrowed-Ptr: - Type(LV) = <_P mut Ty - RESERVE(LV, LT) = LOANS - ------------------------------------------------------------ - RESERVE(*LV, LT) = LOANS, (*LV, LT, Total, Reserve) +Fields and owned pointers inherit their mutability from +their base expressions, so both of their rules basically +delegate the check to the base expression `LV`: -## Borrows in calls + MUTABILITY(LV.f, MQ) // M-Field + MUTABILITY(LV, MQ) -Earlier we said that the MUTATE, FREEZE, and ALIAS functions were used -to compute the loans resulting from a borrow expression. But this is -not strictly correct, there is a slight complication that occurs with -calls by which additional loans may be necessary. We will explain -that here and give the full details. + MUTABILITY(*LV, MQ) // M-Deref-Unique + TYPE(LV) = ~Ty + MUTABILITY(LV, MQ) -Imagine a call expression `'a: E1(E2, E3)`, where `Ei` are some -expressions. If we break this down to something a bit lower-level, it -is kind of short for: +### Checking mutability of immutable pointer types - 'a: { - 'a_arg1: let temp1: ... = E1; - 'a_arg2: let temp2: ... = E2; - 'a_arg3: let temp3: ... = E3; - 'a_call: temp1(temp2, temp3) - } +Immutable pointer types like `&T` and `@T` can only +be borrowed if MQ is immutable or const: -Here the lifetime labels indicate the various lifetimes. As you can -see there are in fact four relevant lifetimes (only one of which was -named by the user): `'a` corresponds to the expression `E1(E2, E3)` as -a whole. `'a_arg1`, `'a_arg2`, and `'a_arg3` correspond to the -evaluations of `E1`, `E2`, and `E3` respectively. Finally, `'a_call` -corresponds to the *actual call*, which is the point where the values -of the parameters will be used. + MUTABILITY(*LV, MQ) // M-Deref-Borrowed-Imm + TYPE(LV) = &Ty + MQ == imm | const -Now, let's look at a (contrived, but representative) example to see -why all this matters: + MUTABILITY(*LV, MQ) // M-Deref-Managed-Imm + TYPE(LV) = @Ty + MQ == imm | const - struct Foo { f: uint, g: uint } - ... - fn add(p: &mut uint, v: uint) { - *p += v; - } - ... - fn inc(p: &mut uint) -> uint { - *p += 1; *p - } - fn weird() { - let mut x: ~Foo = ~Foo { ... }; - 'a: add(&mut (*x).f, - 'b: inc(&mut (*x).f)) // (*) - } +### Checking mutability of mutable pointer types -The important part is the line marked `(*)` which contains a call to -`add()`. The first argument is a mutable borrow of the field `f`. -The second argument *always borrows* the field `f`. Now, if these two -borrows overlapped in time, this would be illegal, because there would -be two `&mut` pointers pointing at `f`. And, in a way, they *do* -overlap in time, since the first argument will be evaluated first, -meaning that the pointer will exist when the second argument executes. -But in another important way they do not overlap in time. Let's -expand out that final call to `add()` as we did before: +`&mut T` and `@mut T` can be frozen, so it is acceptable to borrow +them as either imm or mut: - 'a: { - 'a_arg1: let a_temp1: ... = add; - 'a_arg2: let a_temp2: &'a_call mut uint = &'a_call mut (*x).f; - 'a_arg3_: let a_temp3: uint = { - let b_temp1: ... = inc; - let b_temp2: &'b_call = &'b_call mut (*x).f; - 'b_call: b_temp1(b_temp2) - }; - 'a_call: a_temp1(a_temp2, a_temp3) - } + MUTABILITY(*LV, MQ) // M-Deref-Borrowed-Mut + TYPE(LV) = &mut Ty -When it's written this way, we can see that although there are two -borrows, the first has lifetime `'a_call` and the second has lifetime -`'b_call` and in fact these lifetimes do not overlap. So everything -is fine. + MUTABILITY(*LV, MQ) // M-Deref-Managed-Mut + TYPE(LV) = @mut Ty -But this does not mean that there isn't reason for caution! Imagine a -devious program like *this* one: +## Checking lifetime - struct Foo { f: uint, g: uint } - ... - fn add(p: &mut uint, v: uint) { - *p += v; +These rules aim to ensure that no data is borrowed for a scope that +exceeds its lifetime. In addition, these rules manage the rooting and +dynamic freezing of `@` and `@mut` values. These two computations wind +up being intimately related. Formally, we define a predicate +`LIFETIME(LV, LT, MQ)`, which states that "the lvalue `LV` can be +safely borrowed for the lifetime `LT` with mutability `MQ`". The Rust +code corresponding to this predicate is the module +`middle::borrowck::gather_loans::lifetime`. + +### The Scope function + +Several of the rules refer to a helper function `SCOPE(LV)=LT`. The +`SCOPE(LV)` yields the lifetime `LT` for which the lvalue `LV` is +guaranteed to exist, presuming that no mutations occur. + +The scope of a local variable is the block where it is declared: + + SCOPE(X) = block where X is declared + +The scope of a field is the scope of the struct: + + SCOPE(LV.f) = SCOPE(LV) + +The scope of a unique pointee is the scope of the pointer, since +(barring mutation or moves) the pointer will not be freed until +the pointer itself `LV` goes out of scope: + + SCOPE(*LV) = SCOPE(LV) if LV has type ~T + +The scope of a managed pointee is also the scope of the pointer. This +is a conservative approximation, since there may be other aliases fo +that same managed box that would cause it to live longer: + + SCOPE(*LV) = SCOPE(LV) if LV has type @T or @mut T + +The scope of a borrowed pointee is the scope associated with the +pointer. This is a conservative approximation, since the data that +the pointer points at may actually live longer: + + SCOPE(*LV) = LT if LV has type &'LT T or &'LT mut T + +### Checking lifetime of variables + +The rule for variables states that a variable can only be borrowed a +lifetime `LT` that is a subregion of the variable's scope: + + LIFETIME(X, LT, MQ) // L-Local + LT <= SCOPE(X) + +### Checking lifetime for owned content + +The lifetime of a field or owned pointer is the same as the lifetime +of its owner: + + LIFETIME(LV.f, LT, MQ) // L-Field + LIFETIME(LV, LT, MQ) + + LIFETIME(*LV, LT, MQ) // L-Deref-Owned + TYPE(LV) = ~Ty + LIFETIME(LV, LT, MQ) + +### Checking lifetime for derefs of borrowed pointers + +Borrowed pointers have a lifetime `LT'` associated with them. The +data they point at has been guaranteed to be valid for at least this +lifetime. Therefore, the borrow is valid so long as the lifetime `LT` +of the borrow is shorter than the lifetime `LT'` of the pointer +itself: + + LIFETIME(*LV, LT, MQ) // L-Deref-Borrowed + TYPE(LV) = <' Ty OR <' mut Ty + LT <= LT' + +### Checking lifetime for derefs of managed, immutable pointers + +Managed pointers are valid so long as the data within them is +*rooted*. There are two ways that this can be achieved. The first is +when the user guarantees such a root will exist. For this to be true, +three conditions must be met: + + LIFETIME(*LV, LT, MQ) // L-Deref-Managed-Imm-User-Root + TYPE(LV) = @Ty + LT <= SCOPE(LV) // (1) + LV is immutable // (2) + LV is not moved or not movable // (3) + +Condition (1) guarantees that the managed box will be rooted for at +least the lifetime `LT` of the borrow, presuming that no mutation or +moves occur. Conditions (2) and (3) then serve to guarantee that the +value is not mutated or moved. Note that lvalues are either +(ultimately) owned by a local variable, in which case we can check +whether that local variable is ever moved in its scope, or they are +owned by the pointee of an (immutable, due to condition 2) managed or +borrowed pointer, in which case moves are not permitted because the +location is aliasable. + +If the conditions of `L-Deref-Managed-Imm-User-Root` are not met, then +there is a second alternative. The compiler can attempt to root the +managed pointer itself. This permits great flexibility, because the +location `LV` where the managed pointer is found does not matter, but +there are some limitations. The lifetime of the borrow can only extend +to the innermost enclosing loop or function body. This guarantees that +the compiler never requires an unbounded amount of stack space to +perform the rooting; if this condition were violated, the compiler +might have to accumulate a list of rooted objects, for example if the +borrow occurred inside the body of a loop but the scope of the borrow +extended outside the loop. More formally, the requirement is that +there is no path starting from the borrow that leads back to the +borrow without crossing the exit from the scope `LT`. + +The rule for compiler rooting is as follows: + + LIFETIME(*LV, LT, MQ) // L-Deref-Managed-Imm-Compiler-Root + TYPE(LV) = @Ty + LT <= innermost enclosing loop/func + ROOT LV at *LV for LT + +Here I have written `ROOT LV at *LV FOR LT` to indicate that the code +makes a note in a side-table that the box `LV` must be rooted into the +stack when `*LV` is evaluated, and that this root can be released when +the scope `LT` exits. + +### Checking lifetime for derefs of managed, mutable pointers + +Loans of the contents of mutable managed pointers are simpler in some +ways that loans of immutable managed pointers, because we can never +rely on the user to root them (since the contents are, after all, +mutable). This means that the burden always falls to the compiler, so +there is only one rule: + + LIFETIME(*LV, LT, MQ) // L-Deref-Managed-Mut-Compiler-Root + TYPE(LV) = @mut Ty + LT <= innermost enclosing loop/func + ROOT LV at *LV for LT + LOCK LV at *LV as MQ for LT + +Note that there is an additional clause this time `LOCK LV at *LV as +MQ for LT`. This clause states that in addition to rooting `LV`, the +compiler should also "lock" the box dynamically, meaning that we +register that the box has been borrowed as mutable or immutable, +depending on `MQ`. This lock will fail if the box has already been +borrowed and either the old loan or the new loan is a mutable loan +(multiple immutable loans are okay). The lock is released as we exit +the scope `LT`. + +## Computing the restrictions + +The final rules govern the computation of *restrictions*, meaning that +we compute the set of actions that will be illegal for the life of the +loan. The predicate is written `RESTRICTIONS(LV, ACTIONS) = +RESTRICTION*`, which can be read "in order to prevent `ACTIONS` from +occuring on `LV`, the restrictions `RESTRICTION*` must be respected +for the lifetime of the loan". + +Note that there is an initial set of restrictions: these restrictions +are computed based on the kind of borrow: + + &mut LV => RESTRICTIONS(LV, MUTATE|CLAIM|FREEZE) + &LV => RESTRICTIONS(LV, MUTATE|CLAIM) + &const LV => RESTRICTIONS(LV, []) + +The reasoning here is that a mutable borrow must be the only writer, +therefore it prevents other writes (`MUTATE`), mutable borrows +(`CLAIM`), and immutable borrows (`FREEZE`). An immutable borrow +permits other immutable borows but forbids writes and mutable borows. +Finally, a const borrow just wants to be sure that the value is not +moved out from under it, so no actions are forbidden. + +### Restrictions for loans of a local variable + +The simplest case is a borrow of a local variable `X`: + + RESTRICTIONS(X, ACTIONS) = (X, ACTIONS) // R-Variable + +In such cases we just record the actions that are not permitted. + +### Restrictions for loans of fields + +Restricting a field is the same as restricting the owner of that +field: + + RESTRICTIONS(LV.f, ACTIONS) = RS, (LV.f, ACTIONS) // R-Field + RESTRICTIONS(LV, ACTIONS) = RS + +The reasoning here is as follows. If the field must not be mutated, +then you must not mutate the owner of the field either, since that +would indirectly modify the field. Similarly, if the field cannot be +frozen or aliased, we cannot allow the owner to be frozen or aliased, +since doing so indirectly freezes/aliases the field. This is the +origin of inherited mutability. + +### Restrictions for loans of owned pointees + +Because the mutability of owned pointees is inherited, restricting an +owned pointee is similar to restricting a field, in that it implies +restrictions on the pointer. However, owned pointers have an important +twist: if the owner `LV` is mutated, that causes the owned pointee +`*LV` to be freed! So whenever an owned pointee `*LV` is borrowed, we +must prevent the owned pointer `LV` from being mutated, which means +that we always add `MUTATE` and `CLAIM` to the restriction set imposed +on `LV`: + + RESTRICTIONS(*LV, ACTIONS) = RS, (*LV, ACTIONS) // R-Deref-Owned-Pointer + TYPE(LV) = ~Ty + RESTRICTIONS(LV, ACTIONS|MUTATE|CLAIM) = RS + +### Restrictions for loans of immutable managed/borrowed pointees + +Immutable managed/borrowed pointees are freely aliasable, meaning that +the compiler does not prevent you from copying the pointer. This +implies that issuing restrictions is useless. We might prevent the +user from acting on `*LV` itself, but there could be another path +`*LV1` that refers to the exact same memory, and we would not be +restricting that path. Therefore, the rule for `&Ty` and `@Ty` +pointers always returns an empty set of restrictions, and it only +permits restricting `MUTATE` and `CLAIM` actions: + + RESTRICTIONS(*LV, ACTIONS) = [] // R-Deref-Imm-Borrowed + TYPE(LV) = &Ty or @Ty + ACTIONS subset of [MUTATE, CLAIM] + +The reason that we can restrict `MUTATE` and `CLAIM` actions even +without a restrictions list is that it is never legal to mutate nor to +borrow mutably the contents of a `&Ty` or `@Ty` pointer. In other +words, those restrictions are already inherent in the type. + +Typically, this limitation is not an issue, because restrictions other +than `MUTATE` or `CLAIM` typically arise due to `&mut` borrow, and as +we said, that is already illegal for `*LV`. However, there is one case +where we can be asked to enforce an `ALIAS` restriction on `*LV`, +which is when you have a type like `&&mut T`. In such cases we will +report an error because we cannot enforce a lack of aliases on a `&Ty` +or `@Ty` type. That case is described in more detail in the section on +mutable borrowed pointers. + +### Restrictions for loans of const aliasable pointees + +Const pointers are read-only. There may be `&mut` or `&` aliases, and +we can not prevent *anything* but moves in that case. So the +`RESTRICTIONS` function is only defined if `ACTIONS` is the empty set. +Because moves from a `&const` or `@const` lvalue are never legal, it +is not necessary to add any restrictions at all to the final +result. + + RESTRICTIONS(*LV, []) = [] // R-Deref-Const-Borrowed + TYPE(LV) = &const Ty or @const Ty + +### Restrictions for loans of mutable borrowed pointees + +Borrowing mutable borrowed pointees is a bit subtle because we permit +users to freeze or claim `&mut` pointees. To see what I mean, consider this +(perfectly safe) code example: + + fn foo(t0: &mut T, op: fn(&T)) { + let t1: &T = &*t0; // (1) + op(t1); } - ... - fn consume(x: ~Foo) -> uint { - x.f + x.g + +In the borrow marked `(1)`, the data at `*t0` is *frozen* as part of a +re-borrow. Therefore, for the lifetime of `t1`, `*t0` must not be +mutated. This is the same basic idea as when we freeze a mutable local +variable, but unlike in that case `t0` is a *pointer* to the data, and +thus we must enforce some subtle restrictions in order to guarantee +soundness. + +Intuitively, we must ensure that `*t0` is the only *mutable* path to +reach the memory that was frozen. The reason that we are so concerned +with *mutable* paths is that those are the paths through which the +user could mutate the data that was frozen and hence invalidate the +`t1` pointer. Note that const aliases to `*t0` are acceptable (and in +fact we can't prevent them without unacceptable performance cost, more +on that later) because + +There are two rules governing `&mut` pointers, but we'll begin with +the first. This rule governs cases where we are attempting to prevent +an `&mut` pointee from being mutated, claimed, or frozen, as occurs +whenever the `&mut` pointee `*LV` is reborrowed as mutable or +immutable: + + RESTRICTIONS(*LV, ACTIONS) = RS, (*LV, ACTIONS) // R-Deref-Mut-Borrowed-1 + TYPE(LV) = &mut Ty + RESTRICTIONS(LV, MUTATE|CLAIM|ALIAS) = RS + +The main interesting part of the rule is the final line, which +requires that the `&mut` *pointer* `LV` be restricted from being +mutated, claimed, or aliased. The goal of these restrictions is to +ensure that, not considering the pointer that will result from this +borrow, `LV` remains the *sole pointer with mutable access* to `*LV`. + +Restrictions against mutations and claims are necessary because if the +pointer in `LV` were to be somehow copied or moved to a different +location, then the restriction issued for `*LV` would not apply to the +new location. Note that because `&mut` values are non-copyable, a +simple attempt to move the base pointer will fail due to the +(implicit) restriction against moves: + + // src/test/compile-fail/borrowck-move-mut-base-ptr.rs + fn foo(t0: &mut int) { + let p: &int = &*t0; // Freezes `*t0` + let t1 = t0; //~ ERROR cannot move out of `t0` + *t1 = 22; } - fn weird() { - let mut x: ~Foo = ~Foo { ... }; - 'a: add(&mut (*x).f, consume(x)) // (*) + +However, the additional restrictions against mutation mean that even a +clever attempt to use a swap to circumvent the type system will +encounter an error: + + // src/test/compile-fail/borrowck-swap-mut-base-ptr.rs + fn foo<'a>(mut t0: &'a mut int, + mut t1: &'a mut int) { + let p: &int = &*t0; // Freezes `*t0` + swap(&mut t0, &mut t1); //~ ERROR cannot borrow `t0` + *t1 = 22; } -In this case, there is only one borrow, but the second argument is -`consume(x)` instead of a second borrow. Because `consume()` is -declared to take a `~Foo`, it will in fact free the pointer `x` when -it has finished executing. If it is not obvious why this is -troublesome, consider this expanded version of that call: +The restriction against *aliasing* (and, in turn, freezing) is +necessary because, if an alias were of `LV` were to be produced, then +`LV` would no longer be the sole path to access the `&mut` +pointee. Since we are only issuing restrictions against `*LV`, these +other aliases would be unrestricted, and the result would be +unsound. For example: + + // src/test/compile-fail/borrowck-alias-mut-base-ptr.rs + fn foo(t0: &mut int) { + let p: &int = &*t0; // Freezes `*t0` + let q: &const &mut int = &const t0; //~ ERROR cannot borrow `t0` + **q = 22; // (*) + } - 'a: { - 'a_arg1: let a_temp1: ... = add; - 'a_arg2: let a_temp2: &'a_call mut uint = &'a_call mut (*x).f; - 'a_arg3_: let a_temp3: uint = { - let b_temp1: ... = consume; - let b_temp2: ~Foo = x; - 'b_call: b_temp1(x) - }; - 'a_call: a_temp1(a_temp2, a_temp3) +Note that the current rules also report an error at the assignment in +`(*)`, because we only permit `&mut` poiners to be assigned if they +are located in a non-aliasable location. However, I do not believe +this restriction is strictly necessary. It was added, I believe, to +discourage `&mut` from being placed in aliasable locations in the +first place. One (desirable) side-effect of restricting aliasing on +`LV` is that borrowing an `&mut` pointee found inside an aliasable +pointee yields an error: + + // src/test/compile-fail/borrowck-borrow-mut-base-ptr-in-aliasable-loc: + fn foo(t0: & &mut int) { + let t1 = t0; + let p: &int = &**t0; //~ ERROR cannot borrow an `&mut` in a `&` pointer + **t1 = 22; // (*) } -In this example, we will have borrowed the first argument before `x` -is freed and then free `x` during evaluation of the second -argument. This causes `a_temp2` to be invalidated. +Here at the line `(*)` you will also see the error I referred to +above, which I do not believe is strictly necessary. + +The second rule for `&mut` handles the case where we are not adding +any restrictions (beyond the default of "no move"): -Of course the loans computed from the borrow expression are supposed -to prevent this situation. But if we just considered the loans from -`MUTATE((*x).f, 'a_call, Total)`, the resulting loans would be: + RESTRICTIONS(*LV, []) = [] // R-Deref-Mut-Borrowed-2 + TYPE(LV) = &mut Ty - ((*x).f, 'a_call, Total, Mut) - (*x, 'a_call, Partial, Mut) - (x, 'a_call, Partial, Mut) +Moving from an `&mut` pointee is never legal, so no special +restrictions are needed. -Because these loans are only in scope for `'a_call`, they do nothing -to prevent the move that occurs evaluating the second argument. +### Restrictions for loans of mutable managed pointees -The way that we solve this is to say that if you have a borrow -expression `&'LT_P mut LV` which itself occurs in the lifetime -`'LT_B`, then the resulting loans are: +With `@mut` pointees, we don't make any static guarantees. But as a +convenience, we still register a restriction against `*LV`, because +that way if we *can* find a simple static error, we will: - MUTATE(LV, LT_P, Total) + ALIAS(LV, LUB(LT_P, LT_B), Total) + RESTRICTIONS(*LV, ACTIONS) = [*LV, ACTIONS] // R-Deref-Managed-Borrowed + TYPE(LV) = @mut Ty -The call to MUTATE is what we've seen so far. The second part -expresses the idea that the expression LV will be evaluated starting -at LT_B until the end of LT_P. Now, in the normal case, LT_P >= LT_B, -and so the second set of loans that result from a ALIAS are basically -a no-op. However, in the case of an argument where the evaluation of -the borrow occurs before the interval where the resulting pointer will -be used, this ALIAS is important. +# Moves and initialization -In the case of our example, it would produce a set of loans like: +The borrow checker is also in charge of ensuring that: - ((*x).f, 'a, Total, Const) - (*x, 'a, Total, Const) - (x, 'a, Total, Imm) +- all memory which is accessed is initialized +- immutable local variables are assigned at most once. -The scope of these loans is `'a = LUB('a_arg2, 'a_call)`, and so they -encompass all subsequent arguments. The first set of loans are Const -loans, which basically just prevent moves. However, when we cross -over the dereference of the owned pointer `x`, the rule for ALIAS -specifies that `x` must be frozen, and hence the final loan is an Imm -loan. In any case the troublesome second argument would be flagged -as an error. +These are two separate dataflow analyses built on the same +framework. Let's look at checking that memory is initialized first; +the checking of immutable local variabe assignments works in a very +similar way. -# Maps that are created +To track the initialization of memory, we actually track all the +points in the program that *create uninitialized memory*, meaning +moves and the declaration of uninitialized variables. For each of +these points, we create a bit in the dataflow set. Assignments to a +variable `x` or path `a.b.c` kill the move/uninitialization bits for +those paths and any subpaths (e.g., `x`, `x.y`, `a.b.c`, `*a.b.c`). +The bits are also killed when the root variables (`x`, `a`) go out of +scope. Bits are unioned when two control-flow paths join. Thus, the +presence of a bit indicates that the move may have occurred without an +intervening assignment to the same memory. At each use of a variable, +we examine the bits in scope, and check that none of them are +moves/uninitializations of the variable that is being used. -Borrowck results in two maps. +Let's look at a simple example: + + fn foo(a: ~int) { + let b: ~int; // Gen bit 0. + + if cond { // Bits: 0 + use(&*a); + b = a; // Gen bit 1, kill bit 0. + use(&*b); + } else { + // Bits: 0 + } + // Bits: 0,1 + use(&*a); // Error. + use(&*b); // Error. + } -- `root_map`: identifies those expressions or patterns whose result - needs to be rooted. Conceptually the root_map maps from an - expression or pattern node to a `node_id` identifying the scope for - which the expression must be rooted (this `node_id` should identify - a block or call). The actual key to the map is not an expression id, - however, but a `root_map_key`, which combines an expression id with a - deref count and is used to cope with auto-deref. + fn use(a: &int) { } + +In this example, the variable `b` is created uninitialized. In one +branch of an `if`, we then move the variable `a` into `b`. Once we +exit the `if`, therefore, it is an error to use `a` or `b` since both +are only conditionally initialized. I have annotated the dataflow +state using comments. There are two dataflow bits, with bit 0 +corresponding to the creation of `b` without an initializer, and bit 1 +corresponding to the move of `a`. The assignment `b = a` both +generates bit 1, because it is a move of `a`, and kills bit 0, because +`b` is now initialized. On the else branch, though, `b` is never +initialized, and so bit 0 remains untouched. When the two flows of +control join, we union the bits from both sides, resulting in both +bits 0 and 1 being set. Thus any attempt to use `a` uncovers the bit 1 +from the "then" branch, showing that `a` may be moved, and any attempt +to use `b` uncovers bit 0, from the "else" branch, showing that `b` +may not be initialized. + +## Initialization of immutable variables + +Initialization of immutable variables works in a very similar way, +except that: + +1. we generate bits for each assignment to a variable; +2. the bits are never killed except when the variable goes out of scope. + +Thus the presence of an assignment bit indicates that the assignment +may have occurred. Note that assignments are only killed when the +variable goes out of scope, as it is not relevant whether or not there +has been a move in the meantime. Using these bits, we can declare that +an assignment to an immutable variable is legal iff there is no other +assignment bit to that same variable in scope. + +## Why is the design made this way? + +It may seem surprising that we assign dataflow bits to *each move* +rather than *each path being moved*. This is somewhat less efficient, +since on each use, we must iterate through all moves and check whether +any of them correspond to the path in question. Similar concerns apply +to the analysis for double assignments to immutable variables. The +main reason to do it this way is that it allows us to print better +error messages, because when a use occurs, we can print out the +precise move that may be in scope, rather than simply having to say +"the variable may not be initialized". + +## Data structures used in the move analysis + +The move analysis maintains several data structures that enable it to +cross-reference moves and assignments to determine when they may be +moving/assigning the same memory. These are all collected into the +`MoveData` and `FlowedMoveData` structs. The former represents the set +of move paths, moves, and assignments, and the latter adds in the +results of a dataflow computation. + +### Move paths + +The `MovePath` tree tracks every path that is moved or assigned to. +These paths have the same form as the `LoanPath` data structure, which +in turn is the "real world version of the lvalues `LV` that we +introduced earlier. The difference between a `MovePath` and a `LoanPath` +is that move paths are: + +1. Canonicalized, so that we have exactly one copy of each, and + we can refer to move paths by index; +2. Cross-referenced with other paths into a tree, so that given a move + path we can efficiently find all parent move paths and all + extensions (e.g., given the `a.b` move path, we can easily find the + move path `a` and also the move paths `a.b.c`) +3. Cross-referenced with moves and assignments, so that we can + easily find all moves and assignments to a given path. + +The mechanism that we use is to create a `MovePath` record for each +move path. These are arranged in an array and are referenced using +`MovePathIndex` values, which are newtype'd indices. The `MovePath` +structs are arranged into a tree, representing using the standard +Knuth representation where each node has a child 'pointer' and a "next +sibling" 'pointer'. In addition, each `MovePath` has a parent +'pointer'. In this case, the 'pointers' are just `MovePathIndex` +values. + +In this way, if we want to find all base paths of a given move path, +we can just iterate up the parent pointers (see `each_base_path()` in +the `move_data` module). If we want to find all extensions, we can +iterate through the subtree (see `each_extending_path()`). + +### Moves and assignments + +There are structs to represent moves (`Move`) and assignments +(`Assignment`), and these are also placed into arrays and referenced +by index. All moves of a particular path are arranged into a linked +lists, beginning with `MovePath.first_move` and continuing through +`Move.next_move`. + +We distinguish between "var" assignments, which are assignments to a +variable like `x = foo`, and "path" assignments (`x.f = foo`). This +is because we need to assign dataflows to the former, but not the +latter, so as to check for double initialization of immutable +variables. + +### Gathering and checking moves + +Like loans, we distinguish two phases. The first, gathering, is where +we uncover all the moves and assignments. As with loans, we do some +basic sanity checking in this phase, so we'll report errors if you +attempt to move out of a borrowed pointer etc. Then we do the dataflow +(see `FlowedMoveData::new`). Finally, in the `check_loans.rs` code, we +walk back over, identify all uses, assignments, and captures, and +check that they are legal given the set of dataflow bits we have +computed for that program point. + +# Future work + +While writing up these docs, I encountered some rules I believe to be +stricter than necessary: + +- I think the restriction against mutating `&mut` pointers found in an + aliasable location is unnecessary. They cannot be reborrowed, to be sure, + so it should be safe to mutate them. Lifting this might cause some common + cases (`&mut int`) to work just fine, but might lead to further confusion + in other cases, so maybe it's best to leave it as is. +- I think restricting the `&mut` LV against moves and `ALIAS` is sufficient, + `MUTATE` and `CLAIM` are overkill. `MUTATE` was necessary when swap was + a built-in operator, but as it is not, it is implied by `CLAIM`, + and `CLAIM` is implied by `ALIAS`. The only net effect of this is an + extra error message in some cases, though. +- I have not described how closures interact. Current code is unsound. + I am working on describing and implementing the fix. +- If we wish, we can easily extend the move checking to allow finer-grained + tracking of what is initialized and what is not, enabling code like + this: + + a = x.f.g; // x.f.g is now uninitialized + // here, x and x.f are not usable, but x.f.h *is* + x.f.g = b; // x.f.g is not initialized + // now x, x.f, x.f.g, x.f.h are all usable + + What needs to change here, most likely, is that the `moves` module + should record not only what paths are moved, but what expressions + are actual *uses*. For example, the reference to `x` in `x.f.g = b` + is not a true *use* in the sense that it requires `x` to be fully + initialized. This is in fact why the above code produces an error + today: the reference to `x` in `x.f.g = b` is considered illegal + because `x` is not fully initialized. + +There are also some possible refactorings: + +- It might be nice to replace all loan paths with the MovePath mechanism, + since they allow lightweight comparison using an integer. */ diff --git a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs new file mode 100644 index 0000000000000..d32c1873ba053 --- /dev/null +++ b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs @@ -0,0 +1,164 @@ +// Copyright 2012 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 <LICENSE-APACHE or +// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license +// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +/*! + * Computes moves. + */ + +use core::prelude::*; +use mc = middle::mem_categorization; +use middle::borrowck::*; +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}; + +pub fn gather_decl(bccx: @BorrowckCtxt, + move_data: &mut MoveData, + decl_id: ast::node_id, + _decl_span: span, + var_id: ast::node_id) { + let loan_path = @LpVar(var_id); + move_data.add_move(bccx.tcx, loan_path, decl_id, Declared); +} + +pub fn gather_move_from_expr(bccx: @BorrowckCtxt, + move_data: &mut MoveData, + move_expr: @ast::expr, + cmt: mc::cmt) { + gather_move_from_expr_or_pat(bccx, move_data, move_expr.id, + MoveExpr(move_expr), cmt); +} + +pub fn gather_move_from_pat(bccx: @BorrowckCtxt, + move_data: &mut MoveData, + move_pat: @ast::pat, + cmt: mc::cmt) { + gather_move_from_expr_or_pat(bccx, move_data, move_pat.id, + MovePat(move_pat), cmt); +} + +fn gather_move_from_expr_or_pat(bccx: @BorrowckCtxt, + move_data: &mut MoveData, + move_id: ast::node_id, + 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 + } + } +} + +pub fn gather_captures(bccx: @BorrowckCtxt, + move_data: &mut MoveData, + closure_expr: @ast::expr) { + let captured_vars = bccx.capture_map.get(&closure_expr.id); + for captured_vars.each |captured_var| { + 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(closure_expr)); + } + moves::CapCopy | moves::CapRef => {} + } + } +} + +pub fn gather_assignment(bccx: @BorrowckCtxt, + move_data: &mut MoveData, + assignment_id: ast::node_id, + assignment_span: span, + assignee_loan_path: @LoanPath, + assignee_id: ast::node_id) { + move_data.add_assignment(bccx.tcx, + assignee_loan_path, + assignment_id, + assignment_span, + assignee_id); +} + +fn check_is_legal_to_move_from(bccx: @BorrowckCtxt, + cmt0: mc::cmt, + cmt: mc::cmt) -> bool { + match cmt.cat { + mc::cat_stack_upvar(*) | + mc::cat_implicit_self(*) | + mc::cat_copied_upvar(*) | + mc::cat_deref(_, _, mc::region_ptr(*)) | + mc::cat_deref(_, _, mc::gc_ptr(*)) => { + bccx.span_err( + cmt0.span, + fmt!("cannot move out of %s", + bccx.cmt_to_str(cmt))); + false + } + + // It seems strange to allow a move out of a static item, + // but what happens in practice is that you have a + // reference to a constant with a type that should be + // moved, like `None::<~int>`. The type of this constant + // is technically `Option<~int>`, which moves, but we know + // that the content of static items will never actually + // contain allocated pointers, so we can just memcpy it. + // Since static items can never have allocated memory, + // this is ok. For now anyhow. + mc::cat_static_item => { + true + } + + mc::cat_rvalue(*) | + mc::cat_local(*) | + mc::cat_arg(*) | + mc::cat_self(*) | + mc::cat_deref(_, _, mc::unsafe_ptr(*)) => { + true + } + + mc::cat_downcast(b) | + mc::cat_interior(b, _) => { + match ty::get(b.ty).sty { + ty::ty_struct(did, _) | ty::ty_enum(did, _) => { + if ty::has_dtor(bccx.tcx, did) { + bccx.span_err( + cmt0.span, + fmt!("cannot move out of type `%s`, \ + which defines the `Drop` trait", + b.ty.user_string(bccx.tcx))); + false + } else { + check_is_legal_to_move_from(bccx, cmt0, b) + } + } + _ => { + check_is_legal_to_move_from(bccx, cmt0, b) + } + } + } + + mc::cat_deref(b, _, mc::uniq_ptr(*)) | + 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 f7b30e22f0110..9455340268eff 100644 --- a/src/librustc/middle/borrowck/gather_loans/lifetime.rs +++ b/src/librustc/middle/borrowck/gather_loans/lifetime.rs @@ -70,11 +70,11 @@ impl GuaranteeLifetimeContext { match cmt.cat { mc::cat_rvalue | mc::cat_implicit_self | - mc::cat_copied_upvar(*) | - mc::cat_local(*) | - mc::cat_arg(*) | - mc::cat_self(*) | - mc::cat_deref(_, _, mc::region_ptr(*)) | + mc::cat_copied_upvar(*) | // L-Local + mc::cat_local(*) | // L-Local + mc::cat_arg(*) | // L-Local + mc::cat_self(*) | // L-Local + mc::cat_deref(_, _, mc::region_ptr(*)) | // L-Deref-Borrowed mc::cat_deref(_, _, mc::unsafe_ptr) => { let scope = self.scope(cmt); self.check_scope(scope) @@ -90,7 +90,7 @@ impl GuaranteeLifetimeContext { mc::cat_deref(base, derefs, mc::gc_ptr(ptr_mutbl)) => { let base_scope = self.scope(base); - // See rule Freeze-Imm-Managed-Ptr-2 in doc.rs + // L-Deref-Managed-Imm-User-Root let omit_root = ( ptr_mutbl == m_imm && self.bccx.is_subregion_of(self.loan_region, base_scope) && @@ -99,6 +99,8 @@ impl GuaranteeLifetimeContext { ); if !omit_root { + // L-Deref-Managed-Imm-Compiler-Root + // L-Deref-Managed-Mut-Compiler-Root self.check_root(cmt, base, derefs, ptr_mutbl, discr_scope); } else { debug!("omitting root, base=%s, base_scope=%?", @@ -107,8 +109,8 @@ impl GuaranteeLifetimeContext { } mc::cat_downcast(base) | - mc::cat_deref(base, _, mc::uniq_ptr(*)) | - mc::cat_interior(base, _) => { + mc::cat_deref(base, _, mc::uniq_ptr(*)) | // L-Deref-Owned + mc::cat_interior(base, _) => { // L-Field self.check(base, discr_scope) } @@ -321,6 +323,8 @@ impl GuaranteeLifetimeContext { //! lvalue `cmt` is guaranteed to be valid without any //! rooting etc, and presuming `cmt` is not mutated. + // See the SCOPE(LV) function in doc.rs + match cmt.cat { mc::cat_rvalue => { ty::re_scope(self.bccx.tcx.region_maps.cleanup_scope(cmt.id)) diff --git a/src/librustc/middle/borrowck/gather_loans/mod.rs b/src/librustc/middle/borrowck/gather_loans/mod.rs index a422d99b6f5cf..c2ae364e54ce8 100644 --- a/src/librustc/middle/borrowck/gather_loans/mod.rs +++ b/src/librustc/middle/borrowck/gather_loans/mod.rs @@ -19,6 +19,7 @@ use core::prelude::*; use middle::borrowck::*; +use middle::borrowck::move_data::MoveData; use mc = middle::mem_categorization; use middle::pat_util; use middle::ty::{ty_region}; @@ -35,6 +36,7 @@ use syntax::visit; mod lifetime; mod restrictions; +mod gather_moves; /// Context used while gathering loans: /// @@ -65,28 +67,32 @@ mod restrictions; struct GatherLoanCtxt { bccx: @BorrowckCtxt, id_range: id_range, + move_data: @mut move_data::MoveData, all_loans: @mut ~[Loan], item_ub: ast::node_id, repeating_ids: ~[ast::node_id] } pub fn gather_loans(bccx: @BorrowckCtxt, - body: &ast::blk) -> (id_range, @mut ~[Loan]) { + body: &ast::blk) + -> (id_range, @mut ~[Loan], @mut move_data::MoveData) { let glcx = @mut GatherLoanCtxt { bccx: bccx, id_range: id_range::max(), all_loans: @mut ~[], item_ub: body.node.id, - repeating_ids: ~[body.node.id] + repeating_ids: ~[body.node.id], + move_data: @mut MoveData::new() }; let v = visit::mk_vt(@visit::Visitor {visit_expr: gather_loans_in_expr, visit_block: gather_loans_in_block, visit_fn: gather_loans_in_fn, visit_stmt: add_stmt_to_map, visit_pat: add_pat_to_id_range, + visit_local: gather_loans_in_local, .. *visit::default_visitor()}); (v.visit_block)(body, glcx, v); - return (glcx.id_range, glcx.all_loans); + return (glcx.id_range, glcx.all_loans, glcx.move_data); } fn add_pat_to_id_range(p: @ast::pat, @@ -130,6 +136,35 @@ fn gather_loans_in_block(blk: &ast::blk, visit::visit_block(blk, this, vt); } +fn gather_loans_in_local(local: @ast::local, + this: @mut GatherLoanCtxt, + vt: visit::vt<@mut GatherLoanCtxt>) { + if local.node.init.is_none() { + // Variable declarations without initializers are considered "moves": + let tcx = this.bccx.tcx; + do pat_util::pat_bindings(tcx.def_map, local.node.pat) |_, id, span, _| { + gather_moves::gather_decl(this.bccx, + this.move_data, + id, + span, + id); + } + } else { + // Variable declarations with initializers are considered "assigns": + let tcx = this.bccx.tcx; + do pat_util::pat_bindings(tcx.def_map, local.node.pat) |_, id, span, _| { + gather_moves::gather_assignment(this.bccx, + this.move_data, + id, + span, + @LpVar(id), + id); + } + } + + visit::visit_local(local, this, vt); +} + fn gather_loans_in_expr(ex: @ast::expr, this: @mut GatherLoanCtxt, vt: visit::vt<@mut GatherLoanCtxt>) { @@ -147,6 +182,13 @@ fn gather_loans_in_expr(ex: @ast::expr, this.guarantee_adjustments(ex, *adjustments); } + // If this expression is a move, gather it: + if this.bccx.is_move(ex.id) { + let cmt = this.bccx.cat_expr(ex); + gather_moves::gather_move_from_expr( + this.bccx, this.move_data, ex, cmt); + } + // Special checks for various kinds of expressions: match ex.node { ast::expr_addr_of(mutbl, base) => { @@ -159,6 +201,23 @@ fn gather_loans_in_expr(ex: @ast::expr, visit::visit_expr(ex, this, vt); } + ast::expr_assign(l, _) | ast::expr_assign_op(_, l, _) => { + let l_cmt = this.bccx.cat_expr(l); + match opt_loan_path(l_cmt) { + Some(l_lp) => { + gather_moves::gather_assignment(this.bccx, this.move_data, + ex.id, ex.span, + l_lp, l.id); + } + None => { + // This can occur with e.g. `*foo() = 5`. In such + // cases, there is no need to check for conflicts + // with moves etc, just ignore. + } + } + visit::visit_expr(ex, this, vt); + } + ast::expr_match(ex_v, ref arms) => { let cmt = this.bccx.cat_expr(ex_v); for arms.each |arm| { @@ -203,6 +262,11 @@ fn gather_loans_in_expr(ex: @ast::expr, this.pop_repeating_id(body.node.id); } + ast::expr_fn_block(*) => { + gather_moves::gather_captures(this.bccx, this.move_data, ex); + visit::visit_expr(ex, this, vt); + } + _ => { visit::visit_expr(ex, this, vt); } @@ -417,6 +481,8 @@ pub impl GatherLoanCtxt { borrow_span: span, cmt: mc::cmt, req_mutbl: ast::mutability) { + //! Implements the M-* rules in doc.rs. + match req_mutbl { m_const => { // Data of any mutability can be lent as const. @@ -451,8 +517,8 @@ pub impl GatherLoanCtxt { fn restriction_set(&self, req_mutbl: ast::mutability) -> RestrictionSet { match req_mutbl { m_const => RESTR_EMPTY, - m_imm => RESTR_EMPTY | RESTR_MUTATE, - m_mutbl => RESTR_EMPTY | RESTR_MUTATE | RESTR_FREEZE + m_imm => RESTR_EMPTY | RESTR_MUTATE | RESTR_CLAIM, + m_mutbl => RESTR_EMPTY | RESTR_MUTATE | RESTR_CLAIM | RESTR_FREEZE } } @@ -558,8 +624,11 @@ pub impl GatherLoanCtxt { } } ast::bind_by_copy | ast::bind_infer => { - // Nothing to do here; neither copies nor moves induce - // borrows. + // No borrows here, but there may be moves + if self.bccx.is_move(pat.id) { + gather_moves::gather_move_from_pat( + self.bccx, self.move_data, pat, cmt); + } } } } diff --git a/src/librustc/middle/borrowck/gather_loans/restrictions.rs b/src/librustc/middle/borrowck/gather_loans/restrictions.rs index 42b1c40a4b3c4..82a3b145e1d5d 100644 --- a/src/librustc/middle/borrowck/gather_loans/restrictions.rs +++ b/src/librustc/middle/borrowck/gather_loans/restrictions.rs @@ -33,7 +33,7 @@ pub fn compute_restrictions(bccx: @BorrowckCtxt, cmt_original: cmt }; - ctxt.compute(cmt, restr) + ctxt.restrict(cmt, restr) } /////////////////////////////////////////////////////////////////////////// @@ -50,9 +50,9 @@ impl RestrictionsContext { self.bccx.tcx } - fn compute(&self, - cmt: mc::cmt, - restrictions: RestrictionSet) -> RestrictionResult { + fn restrict(&self, + cmt: mc::cmt, + restrictions: RestrictionSet) -> RestrictionResult { // Check for those cases where we cannot control the aliasing // and make sure that we are not being asked to. @@ -76,6 +76,7 @@ impl RestrictionsContext { mc::cat_local(local_id) | mc::cat_arg(local_id) | mc::cat_self(local_id) => { + // R-Variable let lp = @LpVar(local_id); SafeIf(lp, ~[Restriction {loan_path: lp, set: restrictions}]) @@ -85,22 +86,30 @@ impl RestrictionsContext { // When we borrow the interior of an enum, we have to // ensure the enum itself is not mutated, because that // could cause the type of the memory to change. - self.compute(cmt_base, restrictions | RESTR_MUTATE) + self.restrict( + cmt_base, + restrictions | RESTR_MUTATE | RESTR_CLAIM) } mc::cat_interior(cmt_base, i) => { + // R-Field + // // Overwriting the base would not change the type of // the memory, so no additional restrictions are // needed. - let result = self.compute(cmt_base, restrictions); + let result = self.restrict(cmt_base, restrictions); self.extend(result, cmt.mutbl, LpInterior(i), restrictions) } mc::cat_deref(cmt_base, _, mc::uniq_ptr(*)) => { + // R-Deref-Owned-Pointer + // // When we borrow the interior of an owned pointer, we // cannot permit the base to be mutated, because that // would cause the unique pointer to be freed. - let result = self.compute(cmt_base, restrictions | RESTR_MUTATE); + let result = self.restrict( + cmt_base, + restrictions | RESTR_MUTATE | RESTR_CLAIM); self.extend(result, cmt.mutbl, LpDeref, restrictions) } @@ -109,16 +118,20 @@ impl RestrictionsContext { mc::cat_implicit_self(*) | mc::cat_deref(_, _, mc::region_ptr(m_imm, _)) | mc::cat_deref(_, _, mc::gc_ptr(m_imm)) => { + // R-Deref-Imm-Borrowed Safe } mc::cat_deref(_, _, mc::region_ptr(m_const, _)) | mc::cat_deref(_, _, mc::gc_ptr(m_const)) => { + // R-Deref-Const-Borrowed self.check_no_mutability_control(cmt, restrictions); Safe } mc::cat_deref(cmt_base, _, mc::gc_ptr(m_mutbl)) => { + // R-Deref-Managed-Borrowed + // // Technically, no restrictions are *necessary* here. // The validity of the borrow is guaranteed // dynamically. However, nonetheless we add a @@ -169,12 +182,15 @@ impl RestrictionsContext { // mutability, we can only prevent mutation or prevent // freezing if it is not aliased. Therefore, in such // cases we restrict aliasing on `cmt_base`. - if restrictions.intersects(RESTR_MUTATE | RESTR_FREEZE) { - let result = self.compute(cmt_base, restrictions | RESTR_ALIAS); + if restrictions != RESTR_EMPTY { + // R-Deref-Mut-Borrowed-1 + let result = self.restrict( + cmt_base, + RESTR_ALIAS | RESTR_MUTATE | RESTR_CLAIM); self.extend(result, cmt.mutbl, LpDeref, restrictions) } else { - let result = self.compute(cmt_base, restrictions); - self.extend(result, cmt.mutbl, LpDeref, restrictions) + // R-Deref-Mut-Borrowed-2 + Safe } } @@ -185,7 +201,7 @@ impl RestrictionsContext { mc::cat_stack_upvar(cmt_base) | mc::cat_discr(cmt_base, _) => { - self.compute(cmt_base, restrictions) + self.restrict(cmt_base, restrictions) } } } @@ -233,7 +249,7 @@ impl RestrictionsContext { fn check_no_mutability_control(&self, cmt: mc::cmt, restrictions: RestrictionSet) { - if restrictions.intersects(RESTR_MUTATE | RESTR_FREEZE) { + if restrictions.intersects(RESTR_MUTATE | RESTR_FREEZE | RESTR_CLAIM) { self.bccx.report(BckError {span: self.span, cmt: cmt, code: err_freeze_aliasable_const}); diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs index 39479e726f8b6..1babf08aa705c 100644 --- a/src/librustc/middle/borrowck/mod.rs +++ b/src/librustc/middle/borrowck/mod.rs @@ -19,7 +19,7 @@ use middle::moves; use middle::dataflow::DataFlowContext; use middle::dataflow::DataFlowOperator; use util::common::stmt_set; -use util::ppaux::{note_and_explain_region, Repr}; +use util::ppaux::{note_and_explain_region, Repr, UserString}; use core::hashmap::{HashSet, HashMap}; use core::io; @@ -46,6 +46,8 @@ pub mod check_loans; #[path="gather_loans/mod.rs"] pub mod gather_loans; +pub mod move_data; + pub struct LoanDataFlowOperator; pub type LoanDataFlow = DataFlowContext<LoanDataFlowOperator>; @@ -121,21 +123,28 @@ fn borrowck_fn(fk: &visit::fn_kind, debug!("borrowck_fn(id=%?)", id); // Check the body of fn items. - let (id_range, all_loans) = + let (id_range, all_loans, move_data) = gather_loans::gather_loans(this, body); - let all_loans: &~[Loan] = &*all_loans; // FIXME(#5074) - let mut dfcx = + let mut loan_dfcx = DataFlowContext::new(this.tcx, this.method_map, LoanDataFlowOperator, id_range, all_loans.len()); for all_loans.eachi |loan_idx, loan| { - dfcx.add_gen(loan.gen_scope, loan_idx); - dfcx.add_kill(loan.kill_scope, loan_idx); + loan_dfcx.add_gen(loan.gen_scope, loan_idx); + loan_dfcx.add_kill(loan.kill_scope, loan_idx); } - dfcx.propagate(body); - check_loans::check_loans(this, &dfcx, *all_loans, body); + 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, body); } } @@ -226,13 +235,13 @@ pub struct Loan { span: span, } -#[deriving(Eq)] +#[deriving(Eq, IterBytes)] pub enum LoanPath { LpVar(ast::node_id), // `x` in doc.rs LpExtend(@LoanPath, mc::MutabilityCategory, LoanPathElem) } -#[deriving(Eq)] +#[deriving(Eq, IterBytes)] pub enum LoanPathElem { LpDeref, // `*LV` in doc.rs LpInterior(mc::InteriorKind) // `LV.f` in doc.rs @@ -292,10 +301,10 @@ 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 and mutable pointers to -// the value cannot be created. -// - `RESTR_FREEZE`: Immutable pointers to the value cannot be created. -// - `RESTR_ALIAS`: The lvalue may not be aliased in any way. +// - `RESTR_MUTATE`: The lvalue may not be modified. +// - `RESTR_CLAIM`: `&mut` borrows of the lvalue are forbidden. +// - `RESTR_FREEZE`: `&` borrows of the lvalue are forbidden. +// - `RESTR_ALIAS`: All borrows of the lvalue are forbidden. // // In addition, no value which is restricted may be moved. Therefore, // restrictions are meaningful even if the RestrictionSet is empty, @@ -306,14 +315,16 @@ pub struct Restriction { set: RestrictionSet } +#[deriving(Eq)] pub struct RestrictionSet { bits: u32 } -pub static RESTR_EMPTY: RestrictionSet = RestrictionSet {bits: 0b000}; -pub static RESTR_MUTATE: RestrictionSet = RestrictionSet {bits: 0b001}; -pub static RESTR_FREEZE: RestrictionSet = RestrictionSet {bits: 0b010}; -pub static RESTR_ALIAS: RestrictionSet = RestrictionSet {bits: 0b100}; +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_ALIAS: RestrictionSet = RestrictionSet {bits: 0b1000}; pub impl RestrictionSet { fn intersects(&self, restr: RestrictionSet) -> bool { @@ -407,6 +418,11 @@ pub enum AliasableViolationKind { BorrowViolation } +pub enum MovedValueUseKind { + MovedInUse, + MovedInCapture, +} + /////////////////////////////////////////////////////////////////////////// // Misc @@ -419,6 +435,10 @@ pub impl BorrowckCtxt { self.tcx.region_maps.is_subscope_of(r_sub, r_sup) } + fn is_move(&self, id: ast::node_id) -> bool { + self.moves_map.contains(&id) + } + fn cat_expr(&self, expr: @ast::expr) -> mc::cmt { mc::cat_expr(self.tcx, self.method_map, expr) } @@ -478,6 +498,83 @@ pub impl BorrowckCtxt { self.note_and_explain_bckerr(err); } + fn report_use_of_moved_value(&self, + use_span: span, + use_kind: MovedValueUseKind, + lp: @LoanPath, + move: &move_data::Move, + moved_lp: @LoanPath) { + let verb = match use_kind { + MovedInUse => "use", + MovedInCapture => "capture", + }; + + match move.kind { + move_data::Declared => { + self.tcx.sess.span_err( + use_span, + fmt!("%s of possibly uninitialized value: `%s`", + verb, + self.loan_path_to_str(lp))); + } + _ => { + let partially = if lp == moved_lp {""} else {"partially "}; + self.tcx.sess.span_err( + use_span, + fmt!("%s of %smoved value: `%s`", + verb, + partially, + self.loan_path_to_str(lp))); + } + } + + match move.kind { + move_data::Declared => {} + + move_data::MoveExpr(expr) => { + let expr_ty = ty::expr_ty_adjusted(self.tcx, expr); + self.tcx.sess.span_note( + expr.span, + fmt!("`%s` moved here because it has type `%s`, \ + which is moved by default (use `copy` to override)", + self.loan_path_to_str(moved_lp), + expr_ty.user_string(self.tcx))); + } + + move_data::MovePat(pat) => { + let pat_ty = ty::node_id_to_type(self.tcx, pat.id); + self.tcx.sess.span_note( + pat.span, + fmt!("`%s` moved here because it has type `%s`, \ + which is moved by default (use `ref` to override)", + self.loan_path_to_str(moved_lp), + pat_ty.user_string(self.tcx))); + } + + move_data::Captured(expr) => { + self.tcx.sess.span_note( + expr.span, + fmt!("`%s` moved into closure environment here \ + because its type is moved by default \ + (make a copy and capture that instead to override)", + self.loan_path_to_str(moved_lp))); + } + } + } + + fn report_reassigned_immutable_variable(&self, + span: span, + lp: @LoanPath, + assign: &move_data::Assignment) { + self.tcx.sess.span_err( + span, + fmt!("re-assignment of immutable variable `%s`", + self.loan_path_to_str(lp))); + self.tcx.sess.span_note( + assign.span, + fmt!("prior assignment occurs here")); + } + fn span_err(&self, s: span, m: &str) { self.tcx.sess.span_err(s, m); } diff --git a/src/librustc/middle/borrowck/move_data.rs b/src/librustc/middle/borrowck/move_data.rs new file mode 100644 index 0000000000000..91962f17d596f --- /dev/null +++ b/src/librustc/middle/borrowck/move_data.rs @@ -0,0 +1,603 @@ +// Copyright 2012 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 <LICENSE-APACHE or +// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license +// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +/*! + +Data structures used for tracking moves. Please see the extensive +comments in the section "Moves and initialization" and in `doc.rs`. + +*/ + +use core::prelude::*; +use core::hashmap::{HashMap, HashSet}; +use middle::borrowck::*; +use middle::dataflow::DataFlowContext; +use middle::dataflow::DataFlowOperator; +use middle::ty; +use middle::typeck; +use syntax::ast; +use syntax::ast_util; +use syntax::codemap::span; +use syntax::opt_vec::OptVec; +use syntax::opt_vec; +use util::ppaux::Repr; + +pub struct MoveData { + /// Move paths. See section "Move paths" in `doc.rs`. + paths: ~[MovePath], + + /// Cache of loan path to move path index, for easy lookup. + path_map: HashMap<@LoanPath, MovePathIndex>, + + /// Each move or uninitialized variable gets an entry here. + moves: ~[Move], + + /// Assignments to a variable, like `x = foo`. These are assigned + /// bits for dataflow, since we must track them to ensure that + /// immutable variables are assigned at most once along each path. + var_assignments: ~[Assignment], + + /// Assignments to a path, like `x.f = foo`. These are not + /// assigned dataflow bits, but we track them because they still + /// kill move bits. + path_assignments: ~[Assignment], + assignee_ids: HashSet<ast::node_id>, +} + +pub struct FlowedMoveData { + move_data: @mut MoveData, + // ^~~~~~~~~~~~~ + // It makes me sad to use @mut here, except that due to + // the visitor design, this is what gather_loans + // must produce. + + dfcx_moves: MoveDataFlow, + + // We could (and maybe should, for efficiency) combine both move + // and assign data flow into one, but this way it's easier to + // distinguish the bits that correspond to moves and assignments. + dfcx_assign: AssignDataFlow +} + +/// Index into `MoveData.paths`, used like a pointer +#[deriving(Eq)] +pub struct MovePathIndex(uint); + +static InvalidMovePathIndex: MovePathIndex = + MovePathIndex(uint::max_value); + +/// Index into `MoveData.moves`, used like a pointer +#[deriving(Eq)] +pub struct MoveIndex(uint); + +static InvalidMoveIndex: MoveIndex = + MoveIndex(uint::max_value); + +pub struct MovePath { + /// Loan path corresponding to this move path + loan_path: @LoanPath, + + /// Parent pointer, `InvalidMovePathIndex` if root + parent: MovePathIndex, + + /// Head of linked list of moves to this path, + /// `InvalidMoveIndex` if not moved + first_move: MoveIndex, + + /// First node in linked list of children, `InvalidMovePathIndex` if leaf + first_child: MovePathIndex, + + /// Next node in linked list of parent's children (siblings), + /// `InvalidMovePathIndex` if none. + next_sibling: MovePathIndex, +} + +pub enum MoveKind { + Declared, // When declared, variables start out "moved". + MoveExpr(@ast::expr), // Expression or binding that moves a variable + MovePat(@ast::pat), // By-move binding + Captured(@ast::expr), // Closure creation that moves a value +} + +pub struct Move { + /// Path being moved. + path: MovePathIndex, + + /// id of node that is doing the move. + id: ast::node_id, + + /// Kind of move, for error messages. + kind: MoveKind, + + /// Next node in linked list of moves from `path`, or `InvalidMoveIndex` + next_move: MoveIndex, +} + +pub struct Assignment { + /// Path being assigned. + path: MovePathIndex, + + /// id where assignment occurs + id: ast::node_id, + + /// span of node where assignment occurs + span: span, +} + +pub struct MoveDataFlowOperator; +pub type MoveDataFlow = DataFlowContext<MoveDataFlowOperator>; + +pub struct AssignDataFlowOperator; +pub type AssignDataFlow = DataFlowContext<AssignDataFlowOperator>; + +impl MoveData { + pub fn new() -> MoveData { + MoveData { + paths: ~[], + path_map: HashMap::new(), + moves: ~[], + path_assignments: ~[], + var_assignments: ~[], + assignee_ids: HashSet::new(), + } + } + + fn path<'a>(&'a self, index: MovePathIndex) -> &'a MovePath { + //! Type safe indexing operator + &self.paths[*index] + } + + fn mut_path<'a>(&'a mut self, index: MovePathIndex) -> &'a mut MovePath { + //! Type safe indexing operator + &mut self.paths[*index] + } + + fn move<'a>(&'a self, index: MoveIndex) -> &'a Move { + //! Type safe indexing operator + &self.moves[*index] + } + + fn is_var_path(&self, index: MovePathIndex) -> bool { + //! True if `index` refers to a variable + self.path(index).parent == InvalidMovePathIndex + } + + pub fn move_path(&mut self, + tcx: ty::ctxt, + lp: @LoanPath) -> MovePathIndex { + /*! + * Returns the existing move path index for `lp`, if any, + * and otherwise adds a new index for `lp` and any of its + * base paths that do not yet have an index. + */ + + match self.path_map.find(&lp) { + Some(&index) => { + return index; + } + None => {} + } + + let index = match *lp { + LpVar(*) => { + let index = MovePathIndex(self.paths.len()); + + self.paths.push(MovePath { + loan_path: lp, + parent: InvalidMovePathIndex, + first_move: InvalidMoveIndex, + first_child: InvalidMovePathIndex, + next_sibling: InvalidMovePathIndex, + }); + + index + } + + LpExtend(base, _, _) => { + let parent_index = self.move_path(tcx, base); + let index = MovePathIndex(self.paths.len()); + + let next_sibling = self.path(parent_index).first_child; + self.mut_path(parent_index).first_child = index; + + self.paths.push(MovePath { + loan_path: lp, + parent: parent_index, + first_move: InvalidMoveIndex, + first_child: InvalidMovePathIndex, + next_sibling: next_sibling, + }); + + index + } + }; + + debug!("move_path(lp=%s, index=%?)", + lp.repr(tcx), + index); + + assert_eq!(*index, self.paths.len() - 1); + self.path_map.insert(lp, index); + return index; + } + + fn existing_move_path(&self, + lp: @LoanPath) + -> Option<MovePathIndex> { + self.path_map.find_copy(&lp) + } + + fn existing_base_paths(&self, + lp: @LoanPath) + -> OptVec<MovePathIndex> { + let mut result = opt_vec::Empty; + self.add_existing_base_paths(lp, &mut result); + result + } + + fn add_existing_base_paths(&self, + lp: @LoanPath, + result: &mut OptVec<MovePathIndex>) { + /*! + * Adds any existing move path indices for `lp` and any base + * paths of `lp` to `result`, but does not add new move paths + */ + + match self.path_map.find_copy(&lp) { + Some(index) => { + for self.each_base_path(index) |p| { + result.push(p); + } + } + None => { + match *lp { + LpVar(*) => { } + LpExtend(b, _, _) => { + self.add_existing_base_paths(b, result); + } + } + } + } + + } + + pub fn add_move(&mut self, + tcx: ty::ctxt, + lp: @LoanPath, + id: ast::node_id, + kind: MoveKind) { + /*! + * Adds a new move entry for a move of `lp` that occurs at + * location `id` with kind `kind`. + */ + + debug!("add_move(lp=%s, id=%?, kind=%?)", + lp.repr(tcx), + id, + kind); + + let path_index = self.move_path(tcx, lp); + let move_index = MoveIndex(self.moves.len()); + + let next_move = self.path(path_index).first_move; + self.mut_path(path_index).first_move = move_index; + + self.moves.push(Move { + path: path_index, + id: id, + kind: kind, + next_move: next_move + }); + } + + pub fn add_assignment(&mut self, + tcx: ty::ctxt, + lp: @LoanPath, + assign_id: ast::node_id, + span: span, + assignee_id: ast::node_id) { + /*! + * Adds a new record for an assignment to `lp` that occurs at + * location `id` with the given `span`. + */ + + debug!("add_assignment(lp=%s, assign_id=%?, assignee_id=%?", + lp.repr(tcx), assign_id, assignee_id); + + let path_index = self.move_path(tcx, lp); + + self.assignee_ids.insert(assignee_id); + + let assignment = Assignment { + path: path_index, + id: assign_id, + span: span, + }; + + if self.is_var_path(path_index) { + debug!("add_assignment[var](lp=%s, assignment=%u, path_index=%?)", + lp.repr(tcx), self.var_assignments.len(), path_index); + + self.var_assignments.push(assignment); + } else { + debug!("add_assignment[path](lp=%s, path_index=%?)", + lp.repr(tcx), path_index); + + self.path_assignments.push(assignment); + } + } + + fn add_gen_kills(&self, + tcx: ty::ctxt, + dfcx_moves: &mut MoveDataFlow, + dfcx_assign: &mut AssignDataFlow) { + /*! + * Adds the gen/kills for the various moves and + * assignments into the provided data flow contexts. + * Moves are generated by moves and killed by assignments and + * scoping. Assignments are generated by assignment to variables and + * killed by scoping. See `doc.rs` for more details. + */ + + for self.moves.eachi |i, move| { + dfcx_moves.add_gen(move.id, i); + } + + for self.var_assignments.eachi |i, assignment| { + dfcx_assign.add_gen(assignment.id, i); + self.kill_moves(assignment.path, assignment.id, dfcx_moves); + } + + for self.path_assignments.each |assignment| { + self.kill_moves(assignment.path, assignment.id, dfcx_moves); + } + + // Kill all moves related to a variable `x` when it goes out + // of scope: + for self.paths.each |path| { + match *path.loan_path { + LpVar(id) => { + let kill_id = tcx.region_maps.encl_scope(id); + let path = *self.path_map.get(&path.loan_path); + self.kill_moves(path, kill_id, dfcx_moves); + } + LpExtend(*) => {} + } + } + + // Kill all assignments when the variable goes out of scope: + for self.var_assignments.eachi |assignment_index, assignment| { + match *self.path(assignment.path).loan_path { + LpVar(id) => { + let kill_id = tcx.region_maps.encl_scope(id); + dfcx_assign.add_kill(kill_id, assignment_index); + } + LpExtend(*) => { + tcx.sess.bug("Var assignment for non var path"); + } + } + } + } + + fn each_base_path(&self, + index: MovePathIndex, + f: &fn(MovePathIndex) -> bool) + -> bool { + let mut p = index; + while p != InvalidMovePathIndex { + if !f(p) { + return false; + } + p = self.path(p).parent; + } + return true; + } + + fn each_extending_path(&self, + index: MovePathIndex, + f: &fn(MovePathIndex) -> bool) -> bool { + if !f(index) { + return false; + } + + let mut p = self.path(index).first_child; + while p != InvalidMovePathIndex { + if !self.each_extending_path(p, f) { + return false; + } + p = self.path(p).next_sibling; + } + + return true; + } + + fn each_applicable_move(&self, + index0: MovePathIndex, + f: &fn(MoveIndex) -> bool) -> bool { + for self.each_extending_path(index0) |index| { + let mut p = self.path(index).first_move; + while p != InvalidMoveIndex { + if !f(p) { + return false; + } + p = self.move(p).next_move; + } + } + return true; + } + + fn kill_moves(&self, + path: MovePathIndex, + kill_id: ast::node_id, + dfcx_moves: &mut MoveDataFlow) { + for self.each_applicable_move(path) |move_index| { + dfcx_moves.add_kill(kill_id, *move_index); + } + } +} + +impl FlowedMoveData { + pub fn new(move_data: @mut MoveData, + tcx: ty::ctxt, + method_map: typeck::method_map, + id_range: ast_util::id_range, + body: &ast::blk) + -> FlowedMoveData + { + let mut dfcx_moves = + DataFlowContext::new(tcx, + method_map, + MoveDataFlowOperator, + id_range, + move_data.moves.len()); + let mut dfcx_assign = + DataFlowContext::new(tcx, + method_map, + AssignDataFlowOperator, + id_range, + move_data.var_assignments.len()); + move_data.add_gen_kills(tcx, &mut dfcx_moves, &mut dfcx_assign); + dfcx_moves.propagate(body); + dfcx_assign.propagate(body); + FlowedMoveData { + move_data: move_data, + dfcx_moves: dfcx_moves, + dfcx_assign: dfcx_assign, + } + } + + pub fn each_move_of(&self, + id: ast::node_id, + loan_path: @LoanPath, + f: &fn(&Move, @LoanPath) -> bool) + -> bool { + /*! + * Iterates through each move of `loan_path` (or some base path + * of `loan_path`) that *may* have occurred on entry to `id` without + * an intervening assignment. In other words, any moves that + * would invalidate a reference to `loan_path` at location `id`. + */ + + // Bad scenarios: + // + // 1. Move of `a.b.c`, use of `a.b.c` + // 2. Move of `a.b.c`, use of `a.b.c.d` + // 3. Move of `a.b.c`, use of `a` or `a.b` + // + // OK scenario: + // + // 4. move of `a.b.c`, use of `a.b.d` + + let base_indices = self.move_data.existing_base_paths(loan_path); + if base_indices.is_empty() { + return true; + } + + let opt_loan_path_index = self.move_data.existing_move_path(loan_path); + + for self.dfcx_moves.each_bit_on_entry(id) |index| { + let move = &self.move_data.moves[index]; + let moved_path = move.path; + if base_indices.contains(&moved_path) { + // Scenario 1 or 2: `loan_path` or some base path of + // `loan_path` was moved. + if !f(move, self.move_data.path(moved_path).loan_path) { + return false; + } + loop; + } + + for opt_loan_path_index.each |&loan_path_index| { + for self.move_data.each_base_path(moved_path) |p| { + if p == loan_path_index { + // Scenario 3: some extension of `loan_path` + // was moved + if !f(move, self.move_data.path(moved_path).loan_path) { + return false; + } + } + } + } + } + return true; + } + + pub fn is_assignee(&self, + id: ast::node_id) + -> bool { + //! True if `id` is the id of the LHS of an assignment + + self.move_data.assignee_ids.contains(&id) + } + + pub fn each_assignment_of(&self, + id: ast::node_id, + loan_path: @LoanPath, + f: &fn(&Assignment) -> bool) + -> bool { + /*! + * Iterates through every assignment to `loan_path` that + * may have occurred on entry to `id`. `loan_path` must be + * a single variable. + */ + + let loan_path_index = { + match self.move_data.existing_move_path(loan_path) { + Some(i) => i, + None => { + // if there were any assignments, it'd have an index + return true; + } + } + }; + + for self.dfcx_assign.each_bit_on_entry(id) |index| { + let assignment = &self.move_data.var_assignments[index]; + if assignment.path == loan_path_index && !f(assignment) { + return false; + } + } + return true; + } +} + +impl DataFlowOperator for MoveDataFlowOperator { + #[inline(always)] + fn initial_value(&self) -> bool { + false // no loans in scope by default + } + + #[inline(always)] + fn join(&self, succ: uint, pred: uint) -> uint { + succ | pred // moves from both preds are in scope + } + + #[inline(always)] + fn walk_closures(&self) -> bool { + true + } +} + +impl DataFlowOperator for AssignDataFlowOperator { + #[inline(always)] + fn initial_value(&self) -> bool { + false // no assignments in scope by default + } + + #[inline(always)] + fn join(&self, succ: uint, pred: uint) -> uint { + succ | pred // moves from both preds are in scope + } + + #[inline(always)] + fn walk_closures(&self) -> bool { + true + } +} diff --git a/src/librustc/middle/check_match.rs b/src/librustc/middle/check_match.rs index 803fdc4ed5d70..748ae83a60c57 100644 --- a/src/librustc/middle/check_match.rs +++ b/src/librustc/middle/check_match.rs @@ -866,7 +866,7 @@ pub fn check_legality_of_move_bindings(cx: @MatchCheckCtxt, if !any_by_move { return; } // pointless micro-optimization for pats.each |pat| { - do walk_pat(*pat) |p| { + for walk_pat(*pat) |p| { if pat_is_binding(def_map, p) { match p.node { pat_ident(_, _, sub) => { @@ -884,66 +884,5 @@ pub fn check_legality_of_move_bindings(cx: @MatchCheckCtxt, } } } - - // Now check to ensure that any move binding is not behind an - // @ or &, or within a struct with a destructor. This is - // always illegal. - let vt = visit::mk_vt(@visit::Visitor { - visit_pat: |pat, (behind_bad_pointer, behind_dtor_struct): (bool, bool), v| { - match pat.node { - pat_ident(_, _, sub) => { - debug!("(check legality of move) checking pat \ - ident with behind_bad_pointer %? and behind_dtor_struct %?", - behind_bad_pointer, behind_dtor_struct); - - if behind_bad_pointer || behind_dtor_struct && - cx.moves_map.contains(&pat.id) - { - let msg = if behind_bad_pointer { - "by-move pattern bindings may not occur behind @ or & bindings" - } else { - "cannot bind by-move within struct (it has a destructor)" - }; - cx.tcx.sess.span_err(pat.span, msg); - } - - match sub { - None => {} - Some(subpat) => { - (v.visit_pat)(subpat, - (behind_bad_pointer, behind_dtor_struct), - v); - } - } - } - - pat_box(subpat) | pat_region(subpat) => { - (v.visit_pat)(subpat, (true, behind_dtor_struct), v); - } - - pat_struct(_, ref fields, _) => { - let behind_dtor_struct = behind_dtor_struct || - (match cx.tcx.def_map.find(&pat.id) { - Some(&def_struct(id)) => { - ty::has_dtor(cx.tcx, id) - } - _ => false - }); - debug!("(check legality of move) checking pat \ - struct with behind_bad_pointer %? and behind_dtor_struct %?", - behind_bad_pointer, behind_dtor_struct); - - for fields.each |fld| { - (v.visit_pat)(fld.pat, (behind_bad_pointer, - behind_dtor_struct), v) - } - } - - _ => visit::visit_pat(pat, (behind_bad_pointer, behind_dtor_struct), v) - } - }, - .. *visit::default_visitor::<(bool, bool)>() - }); - (vt.visit_pat)(*pat, (false, false), vt); } } diff --git a/src/librustc/middle/dataflow.rs b/src/librustc/middle/dataflow.rs index 5898b6a5e4d15..e0806359c5d09 100644 --- a/src/librustc/middle/dataflow.rs +++ b/src/librustc/middle/dataflow.rs @@ -651,10 +651,10 @@ impl<'self, O:DataFlowOperator> PropagationContext<'self, O> { } ast::expr_struct(_, ref fields, with_expr) => { - self.walk_opt_expr(with_expr, in_out, loop_scopes); for fields.each |field| { self.walk_expr(field.node.expr, in_out, loop_scopes); } + self.walk_opt_expr(with_expr, in_out, loop_scopes); } ast::expr_call(f, ref args, _) => { @@ -826,7 +826,7 @@ impl<'self, O:DataFlowOperator> PropagationContext<'self, O> { debug!("DataFlowContext::walk_pat(pat=%s, in_out=%s)", pat.repr(self.dfcx.tcx), bits_to_str(reslice(in_out))); - do ast_util::walk_pat(pat) |p| { + for ast_util::walk_pat(pat) |p| { debug!(" p.id=%? in_out=%s", p.id, bits_to_str(reslice(in_out))); self.merge_with_entry_set(p.id, in_out); self.dfcx.apply_gen_kill(p.id, in_out); diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index e4b93468c2938..4897d6c87dec1 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -109,7 +109,6 @@ use middle::pat_util; use middle::ty; use middle::typeck; use middle::moves; -use util::ppaux::ty_to_str; use core::cast::transmute; use core::hashmap::HashMap; @@ -146,7 +145,6 @@ fn live_node_kind_to_str(lnk: LiveNodeKind, cx: ty::ctxt) -> ~str { pub fn check_crate(tcx: ty::ctxt, method_map: typeck::method_map, - variable_moves_map: moves::VariableMovesMap, capture_map: moves::CaptureMap, crate: @crate) { let visitor = visit::mk_vt(@visit::Visitor { @@ -159,7 +157,6 @@ pub fn check_crate(tcx: ty::ctxt, let initial_maps = @mut IrMaps(tcx, method_map, - variable_moves_map, capture_map); visit::visit_crate(crate, initial_maps, visitor); tcx.sess.abort_if_errors(); @@ -229,7 +226,6 @@ enum VarKind { struct IrMaps { tcx: ty::ctxt, method_map: typeck::method_map, - variable_moves_map: moves::VariableMovesMap, capture_map: moves::CaptureMap, num_live_nodes: uint, @@ -243,13 +239,11 @@ struct IrMaps { fn IrMaps(tcx: ty::ctxt, method_map: typeck::method_map, - variable_moves_map: moves::VariableMovesMap, capture_map: moves::CaptureMap) -> IrMaps { IrMaps { tcx: tcx, method_map: method_map, - variable_moves_map: variable_moves_map, capture_map: capture_map, num_live_nodes: 0, num_vars: 0, @@ -349,7 +343,6 @@ fn visit_fn(fk: &visit::fn_kind, // swap in a new set of IR maps for this function body: let fn_maps = @mut IrMaps(this.tcx, this.method_map, - this.variable_moves_map, this.capture_map); unsafe { @@ -1399,11 +1392,7 @@ pub impl Liveness { fn check_local(local: @local, this: @Liveness, vt: vt<@Liveness>) { match local.node.init { Some(_) => { - - // Initializer: this.warn_about_unused_or_dead_vars_in_pat(local.node.pat); - this.check_for_reassignments_in_pat(local.node.pat, - local.node.is_mutbl); } None => { @@ -1438,35 +1427,6 @@ fn check_arm(arm: &arm, this: @Liveness, vt: vt<@Liveness>) { fn check_expr(expr: @expr, this: @Liveness, vt: vt<@Liveness>) { match expr.node { - expr_path(_) | expr_self => { - for this.variable_from_def_map(expr.id, expr.span).each |var| { - let ln = this.live_node(expr.id, expr.span); - - match this.ir.variable_moves_map.find(&expr.id) { - None => {} - Some(&entire_expr) => { - debug!("(checking expr) is a move: `%s`", - expr_to_str(expr, this.tcx.sess.intr())); - this.check_move_from_var(ln, *var, entire_expr); - } - } - } - - visit::visit_expr(expr, this, vt); - } - - expr_fn_block(*) => { - let caps = this.ir.captures(expr); - for caps.each |cap| { - let var = this.variable(cap.var_nid, expr.span); - if cap.is_move { - this.check_move_from_var(cap.ln, var, expr); - } - } - - visit::visit_expr(expr, this, vt); - } - expr_assign(l, r) => { this.check_lvalue(l, vt); (vt.visit_expr)(r, this, vt); @@ -1507,7 +1467,7 @@ fn check_expr(expr: @expr, this: @Liveness, vt: vt<@Liveness>) { expr_cast(*) | expr_unary(*) | expr_ret(*) | expr_break(*) | expr_again(*) | expr_lit(_) | expr_block(*) | expr_mac(*) | expr_addr_of(*) | expr_struct(*) | expr_repeat(*) | - expr_paren(*) => { + expr_paren(*) | expr_fn_block(*) | expr_path(*) | expr_self(*) => { visit::visit_expr(expr, this, vt); } } @@ -1547,43 +1507,17 @@ pub impl Liveness { } } - fn check_move_from_var(&self, - ln: LiveNode, - var: Variable, - move_expr: @expr) { - /*! - * Checks whether `var` is live on entry to any of the - * successors of `ln`. If it is, report an error. - * `move_expr` is the expression which caused the variable - * to be moved. - * - * Note that `move_expr` is not necessarily a reference to the - * variable. It might be an expression like `x.f` which could - * cause a move of the variable `x`, or a closure creation. - */ - - debug!("check_move_from_var(%s, %s)", - ln.to_str(), var.to_str()); - - match self.live_on_exit(ln, var) { - None => {} - Some(lnk) => self.report_illegal_move(lnk, var, move_expr) - } - } - fn check_lvalue(@self, expr: @expr, vt: vt<@Liveness>) { match expr.node { expr_path(_) => { match self.tcx.def_map.get_copy(&expr.id) { - def_local(nid, mutbl) => { + def_local(nid, _) => { // Assignment to an immutable variable or argument: only legal // if there is no later assignment. If this local is actually // mutable, then check for a reassignment to flag the mutability // as being used. let ln = self.live_node(expr.id, expr.span); let var = self.variable(nid, expr.span); - self.check_for_reassignment(ln, var, expr.span, - if mutbl {Some(nid)} else {None}); self.warn_about_dead_assign(expr.span, expr.id, ln, var); } def => { @@ -1607,118 +1541,6 @@ pub impl Liveness { } } - fn check_for_reassignments_in_pat(&self, pat: @pat, mutbl: bool) { - do self.pat_bindings(pat) |ln, var, sp, id| { - self.check_for_reassignment(ln, var, sp, - if mutbl {Some(id)} else {None}); - } - } - - fn check_for_reassignment(&self, ln: LiveNode, var: Variable, - orig_span: span, mutbl: Option<node_id>) { - match self.assigned_on_exit(ln, var) { - Some(ExprNode(span)) => { - match mutbl { - Some(id) => { self.tcx.used_mut_nodes.insert(id); } - None => { - self.tcx.sess.span_err( - span, - "re-assignment of immutable variable"); - self.tcx.sess.span_note( - orig_span, - "prior assignment occurs here"); - } - } - } - Some(lnk) => { - self.tcx.sess.span_bug( - orig_span, - fmt!("illegal writer: %?", lnk)); - } - None => {} - } - } - - fn report_illegal_move(&self, lnk: LiveNodeKind, - var: Variable, - move_expr: @expr) { - // the only time that it is possible to have a moved variable - // used by ExitNode would be arguments or fields in a ctor. - // we give a slightly different error message in those cases. - if lnk == ExitNode { - // FIXME #4715: this seems like it should be reported in the - // borrow checker - let vk = self.ir.var_kinds[*var]; - match vk { - Arg(_, name) => { - self.tcx.sess.span_err( - move_expr.span, - fmt!("illegal move from argument `%s`, which is not \ - copy or move mode", *self.tcx.sess.str_of(name))); - return; - } - Local(*) | ImplicitRet => { - self.tcx.sess.span_bug( - move_expr.span, - fmt!("illegal reader (%?) for `%?`", - lnk, vk)); - } - } - } - - match move_expr.node { - expr_fn_block(*) => { - self.report_illegal_read( - move_expr.span, lnk, var, MovedValue); - let name = self.ir.variable_name(var); - self.tcx.sess.span_note( - move_expr.span, - fmt!("`%s` moved into closure environment here \ - because its type is moved by default", - *name)); - } - expr_path(*) => { - self.report_illegal_read( - move_expr.span, lnk, var, MovedValue); - self.report_move_location( - move_expr, var, "", "it"); - } - expr_field(*) => { - self.report_illegal_read( - move_expr.span, lnk, var, PartiallyMovedValue); - self.report_move_location( - move_expr, var, "field of ", "the field"); - } - expr_index(*) => { - self.report_illegal_read( - move_expr.span, lnk, var, PartiallyMovedValue); - self.report_move_location( - move_expr, var, "element of ", "the element"); - } - _ => { - self.report_illegal_read( - move_expr.span, lnk, var, PartiallyMovedValue); - self.report_move_location( - move_expr, var, "subcomponent of ", "the subcomponent"); - } - }; - } - - fn report_move_location(&self, - move_expr: @expr, - var: Variable, - expr_descr: &str, - pronoun: &str) { - let move_expr_ty = ty::expr_ty(self.tcx, move_expr); - let name = self.ir.variable_name(var); - self.tcx.sess.span_note( - move_expr.span, - fmt!("%s`%s` moved here because %s has type %s, \ - which is moved by default (use `copy` to override)", - expr_descr, *name, pronoun, - ty_to_str(self.tcx, move_expr_ty))); - } - fn report_illegal_read(&self, chk_span: span, lnk: LiveNodeKind, diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index 52c7bf0a21e7d..0d33555974724 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -93,19 +93,26 @@ pub enum ptr_kind { // We use the term "interior" to mean "something reachable from the // base without a pointer dereference", e.g. a field -#[deriving(Eq)] +#[deriving(Eq, IterBytes)] pub enum InteriorKind { InteriorField(FieldName), - InteriorElement(ty::t), // ty::t is the type of the vec/str + InteriorElement(ElementKind), } -#[deriving(Eq)] +#[deriving(Eq, IterBytes)] pub enum FieldName { NamedField(ast::ident), PositionalField(uint) } -#[deriving(Eq)] +#[deriving(Eq, IterBytes)] +pub enum ElementKind { + VecElement, + StrElement, + OtherElement, +} + +#[deriving(Eq, IterBytes)] pub enum MutabilityCategory { McImmutable, // Immutable. McReadOnly, // Read-only (`const`) @@ -192,7 +199,7 @@ pub fn opt_deref_kind(t: ty::t) -> Option<deref_kind> { ty::ty_evec(_, ty::vstore_fixed(_)) | ty::ty_estr(ty::vstore_fixed(_)) => { - Some(deref_interior(InteriorElement(t))) + Some(deref_interior(InteriorElement(element_kind(t)))) } _ => None @@ -749,7 +756,7 @@ pub impl mem_categorization_ctxt { @cmt_ { id:elt.id(), span:elt.span(), - cat:cat_interior(of_cmt, InteriorElement(vec_ty)), + cat:cat_interior(of_cmt, InteriorElement(element_kind(vec_ty))), mutbl:mutbl, ty:mt.ty } @@ -993,12 +1000,14 @@ pub impl mem_categorization_ctxt { cat_interior(_, InteriorField(PositionalField(_))) => { ~"anonymous field" } - cat_interior(_, InteriorElement(t)) => { - match ty::get(t).sty { - ty::ty_evec(*) => ~"vec content", - ty::ty_estr(*) => ~"str content", - _ => ~"indexed content" - } + cat_interior(_, InteriorElement(VecElement)) => { + ~"vec content" + } + cat_interior(_, InteriorElement(StrElement)) => { + ~"str content" + } + cat_interior(_, InteriorElement(OtherElement)) => { + ~"indexed content" } cat_stack_upvar(_) => { ~"captured outer variable" @@ -1193,3 +1202,11 @@ impl Repr for InteriorKind { } } } + +fn element_kind(t: ty::t) -> ElementKind { + match ty::get(t).sty { + ty::ty_evec(*) => VecElement, + ty::ty_estr(*) => StrElement, + _ => OtherElement + } +} diff --git a/src/librustc/middle/moves.rs b/src/librustc/middle/moves.rs index 3b20344b3ead3..abec56d32d79e 100644 --- a/src/librustc/middle/moves.rs +++ b/src/librustc/middle/moves.rs @@ -88,112 +88,32 @@ Similar reasoning can be applied to `let` expressions: ## Output -The pass results in the struct `MoveMaps` which contains two sets, -`moves_map` and `variable_moves_map`, and one map, `capture_map`. - -`moves_map` is a set containing the id of every *outermost -expression* or *binding* that is moved. Note that `moves_map` only -contains the *outermost expressions* that are moved. Therefore, if -you have a use of `x.b`, as in the example `y` above, the -expression `x.b` would be in the `moves_map` but not `x`. The -reason for this is that, for most purposes, it's only the outermost -expression that is needed. The borrow checker and trans, for -example, only care about the outermost expressions that are moved. -It is more efficient therefore just to store those entries. - -In the case of the liveness pass, however, we need to know which -*variable references* are moved (see the Enforcement of Moves -section below for more details). That is, for the `x.b` -expression, liveness only cares about the `x`. For this purpose, -we have a second map, `variable_moves_map`, that contains the ids -of all variable references which is moved. - -The `capture_map` maps from the node_id of a closure expression to an -array of `CaptureVar` structs detailing which variables are captured -and how (by ref, by copy, by move). +The pass results in the struct `MoveMaps` which contains several +maps: + +`moves_map` is a set containing the id of every *outermost expression* or +*binding* that causes a move. Note that `moves_map` only contains the *outermost +expressions* that are moved. Therefore, if you have a use of `x.b`, +as in the example `y` above, the expression `x.b` would be in the +`moves_map` but not `x`. The reason for this is that, for most +purposes, it's only the outermost expression that is needed. The +borrow checker and trans, for example, only care about the outermost +expressions that are moved. It is more efficient therefore just to +store those entries. + +Sometimes though we want to know the variables that are moved (in +particular in the borrow checker). For these cases, the set +`moved_variables_set` just collects the ids of variables that are +moved. + +Finally, the `capture_map` maps from the node_id of a closure +expression to an array of `CaptureVar` structs detailing which +variables are captured and how (by ref, by copy, by move). ## Enforcement of Moves -The enforcement of moves is somewhat complicated because it is divided -amongst the liveness and borrowck modules. In general, the borrow -checker is responsible for guaranteeing that *only owned data is -moved*. The liveness checker, in contrast, is responsible for -checking that *no variable is used after it is moved*. - -To see the difference, let's look at a few examples. Here is a -program fragment where the error would be caught by liveness: - - struct Foo { a: int, b: ~int } - let x: Foo = ...; - let y = x.b; // (1) - let z = x; // (2) //~ ERROR use of moved value `x` - -Here the liveness checker will see the assignment to `y` moves -invalidates the variable `x` because it moves the expression `x.b`. -An error is resported because `x` is not dead at the point where it is -invalidated. - -In more concrete terms, the `moves_map` generated from this example -would contain both the expression `x.b` (1) and the expression `x` -(2). Note that it would not contain `x` (1), because `moves_map` only -contains the outermost expressions that are moved. However, -`moves_map` is not used by liveness. It uses the -`variable_moves_map`, which would contain both references to `x`: (1) -and (2). Therefore, after computing which variables are live where, -liveness will see that the reference (1) to `x` is both present in -`variable_moves_map` and that `x` is live and report an error. - -Now let's look at another illegal example, but one where liveness would -not catch the error: - - struct Foo { a: int, b: ~int } - let x: @Foo = ...; - let y = x.b; //~ ERROR move from managed (@) box - -This is an interesting example because the only change I've made is -to make `x` have type `@Foo` and not `Foo`. Thanks to auto-deref, -the expression `x.b` still works, but now it is short for `{x).b`, -and hence the move is actually moving out of the contents of a -managed box, which is illegal. However, liveness knows nothing of -this. It only tracks what variables are used where. The moves -pass (that is, this pass) is also ignorant of such details. From -the perspective of the moves pass, the `let y = x.b` line above -will be categorized as follows: - - let y = {(x{Move}) {Move}).b; {Move} - -Therefore, the reference to `x` will be present in -`variable_moves_map`, but liveness will not report an error because -there is no subsequent use. - -This is where the borrow checker comes in. When the borrow checker -runs, it will see that `x.b` is present in the `moves_map`. It will -use the `mem_categorization` module to determine where the result of -this expression resides in memory and see that it is owned by managed -data, and report an error. - -In principle, liveness could use the `mem_categorization` module -itself and check that moves always originate from owned data -(historically, of course, this was not the case; `mem_categorization` -used to be private to the borrow checker). However, there is another -kind of error which liveness could not possibly detect. Sometimes a -move is an error due to an outstanding loan, and it is borrow -checker's job to compute those loans. That is, consider *this* -example: - - struct Foo { a: int, b: ~int } - let x: Foo = ...; - let y = &x.b; //~ NOTE loan issued here - let z = x.b; //~ ERROR move with outstanding loan - -In this case, `y` is a pointer into `x`, so when `z` tries to move out -of `x`, we get an error. There is no way that liveness could compute -this information without redoing the efforts of the borrow checker. - -### Closures - -Liveness is somewhat complicated by having to deal with stack -closures. More information to come! +The enforcement of moves is done by the borrow checker. Please see +the section "Moves and initialization" in `middle/borrowck/doc.rs`. ## Distributive property @@ -213,6 +133,7 @@ use middle::freevars; use middle::ty; use middle::typeck::{method_map}; use util::ppaux; +use util::ppaux::Repr; use util::common::indenter; use core::hashmap::{HashSet, HashMap}; @@ -220,7 +141,6 @@ use syntax::ast::*; use syntax::ast_util; use syntax::visit; use syntax::visit::vt; -use syntax::print::pprust; use syntax::codemap::span; #[deriving(Encodable, Decodable)] @@ -241,11 +161,6 @@ pub type CaptureMap = @mut HashMap<node_id, @[CaptureVar]>; pub type MovesMap = @mut HashSet<node_id>; -/** - * For each variable which will be moved, links to the - * expression */ -pub type VariableMovesMap = @mut HashMap<node_id, @expr>; - /** * Set of variable node-ids that are moved. * @@ -257,7 +172,6 @@ pub type MovedVariablesSet = @mut HashSet<node_id>; /** See the section Output on the module comment for explanation. */ pub struct MoveMaps { moves_map: MovesMap, - variable_moves_map: VariableMovesMap, moved_variables_set: MovedVariablesSet, capture_map: CaptureMap } @@ -269,9 +183,8 @@ struct VisitContext { } enum UseMode { - MoveInWhole, // Move the entire value. - MoveInPart(@expr), // Some subcomponent will be moved - Read // Read no matter what the type. + Move, // This value or something owned by it is moved. + Read // Read no matter what the type. } pub fn compute_moves(tcx: ty::ctxt, @@ -287,7 +200,6 @@ pub fn compute_moves(tcx: ty::ctxt, method_map: method_map, move_maps: MoveMaps { moves_map: @mut HashSet::new(), - variable_moves_map: @mut HashMap::new(), capture_map: @mut HashMap::new(), moved_variables_set: @mut HashSet::new() } @@ -317,21 +229,6 @@ fn compute_modes_for_expr(expr: @expr, cx.consume_expr(expr, v); } -pub impl UseMode { - fn component_mode(&self, expr: @expr) -> UseMode { - /*! - * - * Assuming that `self` is the mode for an expression E, - * returns the appropriate mode to use for a subexpression of E. - */ - - match *self { - Read | MoveInPart(_) => *self, - MoveInWhole => MoveInPart(expr) - } - } -} - pub impl VisitContext { fn consume_exprs(&self, exprs: &[@expr], @@ -347,18 +244,20 @@ pub impl VisitContext { visitor: vt<VisitContext>) { /*! - * * Indicates that the value of `expr` will be consumed, * meaning either copied or moved depending on its type. */ - debug!("consume_expr(expr=%?/%s)", - expr.id, - pprust::expr_to_str(expr, self.tcx.sess.intr())); + debug!("consume_expr(expr=%s)", + expr.repr(self.tcx)); let expr_ty = ty::expr_ty_adjusted(self.tcx, expr); - let mode = self.consume_mode_for_ty(expr_ty); - self.use_expr(expr, mode, visitor); + if ty::type_moves_by_default(self.tcx, expr_ty) { + self.move_maps.moves_map.insert(expr.id); + self.use_expr(expr, Move, visitor); + } else { + self.use_expr(expr, Read, visitor); + }; } fn consume_block(&self, @@ -366,7 +265,6 @@ pub impl VisitContext { visitor: vt<VisitContext>) { /*! - * * Indicates that the value of `blk` will be consumed, * meaning either copied or moved depending on its type. */ @@ -382,46 +280,20 @@ pub impl VisitContext { } } - fn consume_mode_for_ty(&self, ty: ty::t) -> UseMode { - /*! - * - * Selects the appropriate `UseMode` to consume a value with - * the type `ty`. This will be `MoveEntireMode` if `ty` is - * not implicitly copyable. - */ - - let result = if ty::type_moves_by_default(self.tcx, ty) { - MoveInWhole - } else { - Read - }; - - debug!("consume_mode_for_ty(ty=%s) = %?", - ppaux::ty_to_str(self.tcx, ty), result); - - return result; - } - fn use_expr(&self, expr: @expr, expr_mode: UseMode, visitor: vt<VisitContext>) { /*! - * * Indicates that `expr` is used with a given mode. This will * in turn trigger calls to the subcomponents of `expr`. */ - debug!("use_expr(expr=%?/%s, mode=%?)", - expr.id, pprust::expr_to_str(expr, self.tcx.sess.intr()), + debug!("use_expr(expr=%s, mode=%?)", + expr.repr(self.tcx), expr_mode); - match expr_mode { - MoveInWhole => { self.move_maps.moves_map.insert(expr.id); } - MoveInPart(_) | Read => {} - } - // `expr_mode` refers to the post-adjustment value. If one of // those adjustments is to take a reference, then it's only // reading the underlying expression, not moving it. @@ -429,7 +301,7 @@ pub impl VisitContext { Some(&@ty::AutoDerefRef( ty::AutoDerefRef { autoref: Some(_), _})) => Read, - _ => expr_mode.component_mode(expr) + _ => expr_mode }; debug!("comp_mode = %?", comp_mode); @@ -437,21 +309,13 @@ pub impl VisitContext { match expr.node { expr_path(*) | expr_self => { match comp_mode { - MoveInPart(entire_expr) => { - self.move_maps.variable_moves_map.insert( - expr.id, entire_expr); - + Move => { let def = self.tcx.def_map.get_copy(&expr.id); for moved_variable_node_id_from_def(def).each |&id| { self.move_maps.moved_variables_set.insert(id); } } Read => {} - MoveInWhole => { - self.tcx.sess.span_bug( - expr.span, - "Component mode can never be MoveInWhole"); - } } } @@ -546,19 +410,10 @@ pub impl VisitContext { self.consume_arm(arm, visitor); } - let by_move_bindings_present = - self.arms_have_by_move_bindings( - self.move_maps.moves_map, *arms); - - if by_move_bindings_present { - // If one of the arms moves a value out of the - // discriminant, then the discriminant itself is - // moved. - self.consume_expr(discr, visitor); - } else { - // Otherwise, the discriminant is merely read. - self.use_expr(discr, Read, visitor); - } + // The discriminant may, in fact, be partially moved + // if there are by-move bindings, but borrowck deals + // with that itself. + self.use_expr(discr, Read, visitor); } expr_copy(base) => { @@ -719,18 +574,17 @@ pub impl VisitContext { */ do pat_bindings(self.tcx.def_map, pat) |bm, id, _span, _path| { - let mode = match bm { - bind_by_copy => Read, - bind_by_ref(_) => Read, + let binding_moves = match bm { + bind_by_copy => false, + bind_by_ref(_) => false, bind_infer => { let pat_ty = ty::node_id_to_type(self.tcx, id); - self.consume_mode_for_ty(pat_ty) + ty::type_moves_by_default(self.tcx, pat_ty) } }; - match mode { - MoveInWhole => { self.move_maps.moves_map.insert(id); } - MoveInPart(_) | Read => {} + if binding_moves { + self.move_maps.moves_map.insert(id); } } } @@ -759,20 +613,18 @@ pub impl VisitContext { fn arms_have_by_move_bindings(&self, moves_map: MovesMap, - arms: &[arm]) -> bool + arms: &[arm]) -> Option<@pat> { for arms.each |arm| { - for arm.pats.each |pat| { - let mut found = false; - do pat_bindings(self.tcx.def_map, *pat) |_, node_id, _, _| { - if moves_map.contains(&node_id) { - found = true; + for arm.pats.each |&pat| { + for ast_util::walk_pat(pat) |p| { + if moves_map.contains(&p.id) { + return Some(p); } } - if found { return true; } } } - return false; + return None; } fn compute_captures(&self, fn_expr_id: node_id) -> @[CaptureVar] { diff --git a/src/librustc/middle/pat_util.rs b/src/librustc/middle/pat_util.rs index 1fd6012143b57..1237e9fb4a26a 100644 --- a/src/librustc/middle/pat_util.rs +++ b/src/librustc/middle/pat_util.rs @@ -72,8 +72,8 @@ pub fn pat_is_binding_or_wild(dm: resolve::DefMap, pat: @pat) -> bool { } pub fn pat_bindings(dm: resolve::DefMap, pat: @pat, - it: &fn(binding_mode, node_id, span, @Path)) { - do walk_pat(pat) |p| { + it: &fn(binding_mode, node_id, span, @Path)) { + for walk_pat(pat) |p| { match p.node { pat_ident(binding_mode, pth, _) if pat_is_binding(dm, p) => { it(binding_mode, p.id, p.span, pth); diff --git a/src/librustc/middle/resolve.rs b/src/librustc/middle/resolve.rs index cda0dfd12a35e..979d04e18c340 100644 --- a/src/librustc/middle/resolve.rs +++ b/src/librustc/middle/resolve.rs @@ -4151,7 +4151,7 @@ pub impl Resolver { bindings_list: Option<@mut HashMap<ident,node_id>>, visitor: ResolveVisitor) { let pat_id = pattern.id; - do walk_pat(pattern) |pattern| { + for walk_pat(pattern) |pattern| { match pattern.node { pat_ident(binding_mode, path, _) if !path.global && path.idents.len() == 1 => { diff --git a/src/librustc/middle/typeck/check/_match.rs b/src/librustc/middle/typeck/check/_match.rs index 77b10663ac790..8edae63cea926 100644 --- a/src/librustc/middle/typeck/check/_match.rs +++ b/src/librustc/middle/typeck/check/_match.rs @@ -157,8 +157,8 @@ pub fn check_pat_variant(pcx: &pat_ctxt, pat: @ast::pat, path: @ast::Path, None); fcx.write_error(pat.id); kind_name = "[error]"; - arg_types = (copy subpats).get_or_default(~[]).map(|_| - ty::mk_err()); + arg_types = (copy *subpats).get_or_default(~[]).map(|_| + ty::mk_err()); } } } @@ -199,8 +199,8 @@ pub fn check_pat_variant(pcx: &pat_ctxt, pat: @ast::pat, path: @ast::Path, None); fcx.write_error(pat.id); kind_name = "[error]"; - arg_types = (copy subpats).get_or_default(~[]).map(|_| - ty::mk_err()); + arg_types = (copy *subpats).get_or_default(~[]).map(|_| + ty::mk_err()); } } diff --git a/src/librustc/util/ppaux.rs b/src/librustc/util/ppaux.rs index 9d74f6c7b0ed7..8694c0e356eb1 100644 --- a/src/librustc/util/ppaux.rs +++ b/src/librustc/util/ppaux.rs @@ -801,3 +801,9 @@ impl UserString for ty::TraitRef { } } } + +impl UserString for ty::t { + fn user_string(&self, tcx: ctxt) -> ~str { + ty_to_str(tcx, *self) + } +} diff --git a/src/librustdoc/markdown_index_pass.rs b/src/librustdoc/markdown_index_pass.rs index 8ff0aa2314647..289aa33f67c37 100644 --- a/src/librustdoc/markdown_index_pass.rs +++ b/src/librustdoc/markdown_index_pass.rs @@ -74,7 +74,7 @@ fn build_mod_index( ) -> doc::Index { doc::Index { entries: doc.items.map(|doc| { - item_to_entry(copy *doc, copy config) + item_to_entry(copy *doc, &config) }) } } @@ -85,14 +85,14 @@ fn build_nmod_index( ) -> doc::Index { doc::Index { entries: doc.fns.map(|doc| { - item_to_entry(doc::FnTag(copy *doc), copy config) + item_to_entry(doc::FnTag(copy *doc), &config) }) } } fn item_to_entry( doc: doc::ItemTag, - config: config::Config + config: &config::Config ) -> doc::IndexEntry { let link = match doc { doc::ModTag(_) | doc::NmodTag(_) @@ -222,13 +222,13 @@ mod test { config::DocPerCrate, ~"mod a { } fn b() { }" ); - assert!((&doc.cratemod().index).get().entries[0] == doc::IndexEntry { + assert!(doc.cratemod().index.get().entries[0] == doc::IndexEntry { kind: ~"Module", name: ~"a", brief: None, link: ~"#module-a" }); - assert!((&doc.cratemod().index).get().entries[1] == doc::IndexEntry { + assert!(doc.cratemod().index.get().entries[1] == doc::IndexEntry { kind: ~"Function", name: ~"b", brief: None, @@ -242,13 +242,13 @@ mod test { config::DocPerMod, ~"mod a { } fn b() { }" ); - assert!((&doc.cratemod().index).get().entries[0] == doc::IndexEntry { + assert!(doc.cratemod().index.get().entries[0] == doc::IndexEntry { kind: ~"Module", name: ~"a", brief: None, link: ~"a.html" }); - assert!((&doc.cratemod().index).get().entries[1] == doc::IndexEntry { + assert!(doc.cratemod().index.get().entries[1] == doc::IndexEntry { kind: ~"Function", name: ~"b", brief: None, @@ -262,7 +262,7 @@ mod test { config::DocPerMod, ~"#[doc = \"test\"] mod a { }" ); - assert!((&doc.cratemod().index).get().entries[0].brief + assert!(doc.cratemod().index.get().entries[0].brief == Some(~"test")); } @@ -272,7 +272,7 @@ mod test { config::DocPerCrate, ~"extern { fn b(); }" ); - assert!((&doc.cratemod().nmods()[0].index).get().entries[0] + assert!(doc.cratemod().nmods()[0].index.get().entries[0] == doc::IndexEntry { kind: ~"Function", name: ~"b", diff --git a/src/librustdoc/markdown_pass.rs b/src/librustdoc/markdown_pass.rs index ed882bc3434b7..a3ad8d8d04de3 100644 --- a/src/librustdoc/markdown_pass.rs +++ b/src/librustdoc/markdown_pass.rs @@ -181,12 +181,12 @@ pub fn header_name(doc: doc::ItemTag) -> ~str { } &doc::ImplTag(ref doc) => { assert!(doc.self_ty.is_some()); - let bounds = if (&doc.bounds_str).is_some() { - fmt!(" where %s", (&doc.bounds_str).get()) + let bounds = if doc.bounds_str.is_some() { + fmt!(" where %s", *doc.bounds_str.get_ref()) } else { ~"" }; - let self_ty = (&doc.self_ty).get(); + let self_ty = doc.self_ty.get_ref(); let mut trait_part = ~""; for doc.trait_types.eachi |i, trait_type| { if i == 0 { @@ -196,7 +196,7 @@ pub fn header_name(doc: doc::ItemTag) -> ~str { } trait_part += *trait_type; } - fmt!("%s for %s%s", trait_part, self_ty, bounds) + fmt!("%s for %s%s", trait_part, *self_ty, bounds) } _ => { doc.name() @@ -208,17 +208,17 @@ pub fn header_text(doc: doc::ItemTag) -> ~str { match &doc { &doc::ImplTag(ref ImplDoc) => { let header_kind = header_kind(copy doc); - let bounds = if (&ImplDoc.bounds_str).is_some() { - fmt!(" where `%s`", (&ImplDoc.bounds_str).get()) + let bounds = if ImplDoc.bounds_str.is_some() { + fmt!(" where `%s`", *ImplDoc.bounds_str.get_ref()) } else { ~"" }; let desc = if ImplDoc.trait_types.is_empty() { - fmt!("for `%s`%s", (&ImplDoc.self_ty).get(), bounds) + fmt!("for `%s`%s", *ImplDoc.self_ty.get_ref(), bounds) } else { fmt!("of `%s` for `%s`%s", ImplDoc.trait_types[0], - (&ImplDoc.self_ty).get(), + *ImplDoc.self_ty.get_ref(), bounds) }; return fmt!("%s %s", header_kind, desc); @@ -295,7 +295,7 @@ fn write_mod_contents( ) { write_common(ctxt, doc.desc(), doc.sections()); if doc.index.is_some() { - write_index(ctxt, (&doc.index).get()); + write_index(ctxt, doc.index.get_ref()); } for doc.items.each |itemTag| { @@ -340,7 +340,7 @@ fn item_header_lvl(doc: &doc::ItemTag) -> Hlvl { } } -fn write_index(ctxt: &Ctxt, index: doc::Index) { +fn write_index(ctxt: &Ctxt, index: &doc::Index) { if vec::is_empty(index.entries) { return; } @@ -353,7 +353,7 @@ fn write_index(ctxt: &Ctxt, index: doc::Index) { let id = copy entry.link; if entry.brief.is_some() { ctxt.w.put_line(fmt!("* [%s](%s) - %s", - header, id, (&entry.brief).get())); + header, id, *entry.brief.get_ref())); } else { ctxt.w.put_line(fmt!("* [%s](%s)", header, id)); } @@ -366,7 +366,7 @@ fn write_index(ctxt: &Ctxt, index: doc::Index) { fn write_nmod(ctxt: &Ctxt, doc: doc::NmodDoc) { write_common(ctxt, doc.desc(), doc.sections()); if doc.index.is_some() { - write_index(ctxt, (&doc.index).get()); + write_index(ctxt, doc.index.get_ref()); } for doc.fns.each |FnDoc| { @@ -450,17 +450,17 @@ fn write_variants( fn write_variant(ctxt: &Ctxt, doc: doc::VariantDoc) { assert!(doc.sig.is_some()); - let sig = (&doc.sig).get(); + let sig = doc.sig.get_ref(); // space out list items so they all end up within paragraph elements ctxt.w.put_line(~""); match copy doc.desc { Some(desc) => { - ctxt.w.put_line(list_item_indent(fmt!("* `%s` - %s", sig, desc))); + ctxt.w.put_line(list_item_indent(fmt!("* `%s` - %s", *sig, desc))); } None => { - ctxt.w.put_line(fmt!("* `%s`", sig)); + ctxt.w.put_line(fmt!("* `%s`", *sig)); } } } diff --git a/src/librustdoc/markdown_writer.rs b/src/librustdoc/markdown_writer.rs index b537dfdbd0bc1..973326b10dce8 100644 --- a/src/librustdoc/markdown_writer.rs +++ b/src/librustdoc/markdown_writer.rs @@ -59,20 +59,20 @@ pub fn make_writer_factory(config: config::Config) -> WriterFactory { fn markdown_writer_factory(config: config::Config) -> WriterFactory { let result: ~fn(page: doc::Page) -> Writer = |page| { - markdown_writer(copy config, page) + markdown_writer(&config, page) }; result } fn pandoc_writer_factory(config: config::Config) -> WriterFactory { let result: ~fn(doc::Page) -> Writer = |page| { - pandoc_writer(copy config, page) + pandoc_writer(&config, page) }; result } fn markdown_writer( - config: config::Config, + config: &config::Config, page: doc::Page ) -> Writer { let filename = make_local_filename(config, page); @@ -82,11 +82,11 @@ fn markdown_writer( } fn pandoc_writer( - config: config::Config, + config: &config::Config, page: doc::Page ) -> Writer { assert!(config.pandoc_cmd.is_some()); - let pandoc_cmd = (&config.pandoc_cmd).get(); + let pandoc_cmd = copy *config.pandoc_cmd.get_ref(); let filename = make_local_filename(config, page); let pandoc_args = ~[ @@ -136,15 +136,15 @@ fn generic_writer(process: ~fn(markdown: ~str)) -> Writer { } pub fn make_local_filename( - config: config::Config, + config: &config::Config, page: doc::Page ) -> Path { - let filename = make_filename(copy config, page); + let filename = make_filename(config, page); config.output_dir.push_rel(&filename) } pub fn make_filename( - config: config::Config, + config: &config::Config, page: doc::Page ) -> Path { let filename = { @@ -247,7 +247,7 @@ mod test { }; let doc = mk_doc(~"test", ~""); let page = doc::CratePage(doc.CrateDoc()); - let filename = make_local_filename(config, page); + let filename = make_local_filename(&config, page); assert_eq!(filename.to_str(), ~"output/dir/test.md"); } @@ -261,7 +261,7 @@ mod test { }; let doc = mk_doc(~"", ~""); let page = doc::CratePage(doc.CrateDoc()); - let filename = make_local_filename(config, page); + let filename = make_local_filename(&config, page); assert_eq!(filename.to_str(), ~"output/dir/index.html"); } @@ -276,7 +276,7 @@ mod test { let doc = mk_doc(~"", ~"mod a { mod b { } }"); let modb = copy doc.cratemod().mods()[0].mods()[0]; let page = doc::ItemPage(doc::ModTag(modb)); - let filename = make_local_filename(config, page); + let filename = make_local_filename(&config, page); assert_eq!(filename, Path("output/dir/a_b.html")); } } diff --git a/src/librustdoc/sectionalize_pass.rs b/src/librustdoc/sectionalize_pass.rs index 89aa09b42d510..ed069b5ed5603 100644 --- a/src/librustdoc/sectionalize_pass.rs +++ b/src/librustdoc/sectionalize_pass.rs @@ -113,7 +113,7 @@ fn sectionalize(desc: Option<~str>) -> (Option<~str>, ~[doc::Section]) { match parse_header(copy *line) { Some(header) => { if current_section.is_some() { - sections += [(¤t_section).get()]; + sections += [copy *current_section.get_ref()]; } current_section = Some(doc::Section { header: header, diff --git a/src/librustdoc/tystr_pass.rs b/src/librustdoc/tystr_pass.rs index 051825d46e1b3..716784c51c513 100644 --- a/src/librustdoc/tystr_pass.rs +++ b/src/librustdoc/tystr_pass.rs @@ -434,7 +434,7 @@ mod test { #[test] fn should_add_struct_defs() { let doc = mk_doc(~"struct S { field: () }"); - assert!((&doc.cratemod().structs()[0].sig).get().contains( + assert!(doc.cratemod().structs()[0].sig.get().contains( "struct S {")); } @@ -442,6 +442,6 @@ mod test { fn should_not_serialize_struct_attrs() { // All we care about are the fields let doc = mk_doc(~"#[wut] struct S { field: () }"); - assert!(!(&doc.cratemod().structs()[0].sig).get().contains("wut")); + assert!(!doc.cratemod().structs()[0].sig.get().contains("wut")); } } diff --git a/src/libsyntax/ast_util.rs b/src/libsyntax/ast_util.rs index 8d5af682d6205..ba56d54488068 100644 --- a/src/libsyntax/ast_util.rs +++ b/src/libsyntax/ast_util.rs @@ -527,36 +527,31 @@ pub fn is_item_impl(item: @ast::item) -> bool { } } -pub fn walk_pat(pat: @pat, it: &fn(@pat)) { - it(pat); +pub fn walk_pat(pat: @pat, it: &fn(@pat) -> bool) -> bool { + if !it(pat) { + return false; + } + match pat.node { pat_ident(_, _, Some(p)) => walk_pat(p, it), pat_struct(_, ref fields, _) => { - for fields.each |f| { - walk_pat(f.pat, it) - } + fields.each(|f| walk_pat(f.pat, it)) } pat_enum(_, Some(ref s)) | pat_tup(ref s) => { - for s.each |p| { - walk_pat(*p, it) - } + s.each(|&p| walk_pat(p, it)) } pat_box(s) | pat_uniq(s) | pat_region(s) => { walk_pat(s, it) } pat_vec(ref before, ref slice, ref after) => { - for before.each |p| { - walk_pat(*p, it) - } - for slice.each |p| { - walk_pat(*p, it) - } - for after.each |p| { - walk_pat(*p, it) - } + before.each(|&p| walk_pat(p, it)) && + slice.each(|&p| walk_pat(p, it)) && + after.each(|&p| walk_pat(p, it)) } pat_wild | pat_lit(_) | pat_range(_, _) | pat_ident(_, _, _) | - pat_enum(_, _) => { } + pat_enum(_, _) => { + true + } } } diff --git a/src/test/compile-fail/borrowck-alias-mut-base-ptr.rs b/src/test/compile-fail/borrowck-alias-mut-base-ptr.rs new file mode 100644 index 0000000000000..c51cf5b9538d9 --- /dev/null +++ b/src/test/compile-fail/borrowck-alias-mut-base-ptr.rs @@ -0,0 +1,15 @@ +// Test that attempt to alias `&mut` pointer while pointee is borrowed +// yields an error. +// +// Example from src/middle/borrowck/doc.rs + +use std::util::swap; + +fn foo(t0: &mut int) { + let p: &int = &*t0; // Freezes `*t0` + let q: &const &mut int = &const t0; //~ ERROR cannot borrow `t0` + **q = 22; //~ ERROR cannot assign to an `&mut` in a `&const` pointer +} + +fn main() { +} \ No newline at end of file diff --git a/src/test/compile-fail/borrowck-borrow-mut-base-ptr-in-aliasable-loc.rs b/src/test/compile-fail/borrowck-borrow-mut-base-ptr-in-aliasable-loc.rs new file mode 100644 index 0000000000000..7e9c298ba4732 --- /dev/null +++ b/src/test/compile-fail/borrowck-borrow-mut-base-ptr-in-aliasable-loc.rs @@ -0,0 +1,31 @@ +// Test that attempt to reborrow an `&mut` pointer in an aliasable +// location yields an error. +// +// Example from src/middle/borrowck/doc.rs + +use std::util::swap; + +fn foo(t0: & &mut int) { + let t1 = t0; + let p: &int = &**t0; //~ ERROR cannot borrow an `&mut` in a `&` pointer + **t1 = 22; //~ ERROR cannot assign +} + +fn foo2(t0: &const &mut int) { + // Note: reborrowing from an &const actually yields two errors, since it + // is unsafe in two ways: we can't control the aliasing, and we can't + // control the mutation. + let t1 = t0; + let p: &int = &**t0; //~ ERROR cannot borrow an `&mut` in a `&const` pointer + //~^ ERROR unsafe borrow of aliasable, const value + **t1 = 22; //~ ERROR cannot assign +} + +fn foo3(t0: &mut &mut int) { + let t1 = &mut *t0; + let p: &int = &**t0; //~ ERROR cannot borrow + **t1 = 22; +} + +fn main() { +} \ No newline at end of file diff --git a/src/test/compile-fail/borrowck-move-by-capture.rs b/src/test/compile-fail/borrowck-move-by-capture.rs index 4e977442e1586..0efde1df6c22b 100644 --- a/src/test/compile-fail/borrowck-move-by-capture.rs +++ b/src/test/compile-fail/borrowck-move-by-capture.rs @@ -7,8 +7,7 @@ fn main() { //~^ ERROR cannot move `foo` let bar = ~3; - let _g = || { + let _g = || { //~ ERROR capture of moved value let _h: @fn() -> int = || *bar; - //~^ ERROR illegal by-move capture }; } diff --git a/src/test/compile-fail/borrowck-move-mut-base-ptr.rs b/src/test/compile-fail/borrowck-move-mut-base-ptr.rs new file mode 100644 index 0000000000000..6a3832d2304cf --- /dev/null +++ b/src/test/compile-fail/borrowck-move-mut-base-ptr.rs @@ -0,0 +1,15 @@ +// Test that attempt to move `&mut` pointer while pointee is borrowed +// yields an error. +// +// Example from src/middle/borrowck/doc.rs + +use std::util::swap; + +fn foo(t0: &mut int) { + let p: &int = &*t0; // Freezes `*t0` + let t1 = t0; //~ ERROR cannot move out of `t0` + *t1 = 22; +} + +fn main() { +} \ No newline at end of file diff --git a/src/test/compile-fail/borrowck-move-out-of-vec-tail.rs b/src/test/compile-fail/borrowck-move-out-of-vec-tail.rs new file mode 100644 index 0000000000000..dec976e0a6068 --- /dev/null +++ b/src/test/compile-fail/borrowck-move-out-of-vec-tail.rs @@ -0,0 +1,31 @@ +// Test that we do not permit moves from &[] matched by a vec pattern. + +struct Foo { + string: ~str +} + +pub fn main() { + let x = [ + Foo { string: ~"foo" }, + Foo { string: ~"bar" }, + Foo { string: ~"baz" } + ]; + match x { + [first, ..tail] => { + match tail { + [Foo { string: a }, Foo { string: b }] => { + //~^ ERROR cannot move out of dereference of & pointer + //~^^ ERROR cannot move out of dereference of & pointer + } + _ => { + ::std::util::unreachable(); + } + } + let z = copy tail[0]; + debug!(fmt!("%?", z)); + } + _ => { + ::std::util::unreachable(); + } + } +} diff --git a/src/test/compile-fail/borrowck-swap-mut-base-ptr.rs b/src/test/compile-fail/borrowck-swap-mut-base-ptr.rs new file mode 100644 index 0000000000000..bea5f1f6ea765 --- /dev/null +++ b/src/test/compile-fail/borrowck-swap-mut-base-ptr.rs @@ -0,0 +1,16 @@ +// Test that attempt to swap `&mut` pointer while pointee is borrowed +// yields an error. +// +// Example from src/middle/borrowck/doc.rs + +use std::util::swap; + +fn foo<'a>(mut t0: &'a mut int, + mut t1: &'a mut int) { + let p: &int = &*t0; // Freezes `*t0` + swap(&mut t0, &mut t1); //~ ERROR cannot borrow `t0` + *t1 = 22; +} + +fn main() { +} \ No newline at end of file diff --git a/src/test/compile-fail/borrowck-unary-move.rs b/src/test/compile-fail/borrowck-unary-move.rs index cf7529865118a..a67a12f9d0f73 100644 --- a/src/test/compile-fail/borrowck-unary-move.rs +++ b/src/test/compile-fail/borrowck-unary-move.rs @@ -10,7 +10,7 @@ fn foo(x: ~int) -> int { let y = &*x; - free(x); //~ ERROR cannot move out of `*x` because it is borrowed + free(x); //~ ERROR cannot move out of `x` because it is borrowed *y } diff --git a/src/test/compile-fail/by-move-pattern-binding.rs b/src/test/compile-fail/by-move-pattern-binding.rs index 1efed154286ec..dc42e28ec2523 100644 --- a/src/test/compile-fail/by-move-pattern-binding.rs +++ b/src/test/compile-fail/by-move-pattern-binding.rs @@ -13,7 +13,7 @@ fn main() { let s = S { x: Bar(~"hello") }; match &s.x { &Foo => {} - &Bar(identifier) => f(copy identifier) //~ ERROR by-move pattern bindings may not occur + &Bar(identifier) => f(copy identifier) //~ ERROR cannot move }; match &s.x { &Foo => {} diff --git a/src/test/compile-fail/disallowed-deconstructing-destructing-struct-match.rs b/src/test/compile-fail/disallowed-deconstructing-destructing-struct-match.rs index 40305ba8b95c9..478a56c03010a 100644 --- a/src/test/compile-fail/disallowed-deconstructing-destructing-struct-match.rs +++ b/src/test/compile-fail/disallowed-deconstructing-destructing-struct-match.rs @@ -23,6 +23,6 @@ fn main() { match x { X { x: y } => error!("contents: %s", y) - //~^ ERROR cannot bind by-move within struct + //~^ ERROR cannot move out of type `X`, which defines the `Drop` trait } } diff --git a/src/test/compile-fail/issue-2590.rs b/src/test/compile-fail/issue-2590.rs index a0b967d59593a..92f2e5ea689c1 100644 --- a/src/test/compile-fail/issue-2590.rs +++ b/src/test/compile-fail/issue-2590.rs @@ -18,7 +18,7 @@ trait parse { impl parse for parser { fn parse(&self) -> ~[int] { - self.tokens //~ ERROR cannot move out of field + self.tokens //~ ERROR cannot move out of dereference of & pointer } } diff --git a/src/test/compile-fail/liveness-move-in-loop.rs b/src/test/compile-fail/liveness-move-in-loop.rs index d1663bc356b18..6fe59f0ca52d1 100644 --- a/src/test/compile-fail/liveness-move-in-loop.rs +++ b/src/test/compile-fail/liveness-move-in-loop.rs @@ -16,10 +16,7 @@ fn main() { loop { loop { loop { -// tjc: Not sure why it prints the same error twice x = y; //~ ERROR use of moved value - //~^ ERROR use of moved value - copy x; } } diff --git a/src/test/compile-fail/liveness-move-in-while.rs b/src/test/compile-fail/liveness-move-in-while.rs index 6b4147242d19b..26e82dd367343 100644 --- a/src/test/compile-fail/liveness-move-in-while.rs +++ b/src/test/compile-fail/liveness-move-in-while.rs @@ -13,10 +13,8 @@ fn main() { let y: ~int = ~42; let mut x: ~int; loop { - debug!(y); -// tjc: not sure why it prints the same error twice + debug!(y); //~ ERROR use of moved value: `y` while true { while true { while true { x = y; copy x; } } } //~^ ERROR use of moved value: `y` - //~^^ ERROR use of moved value: `y` } } 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 6cc19b18c20a6..1a2beedff9306 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 @@ -7,13 +7,13 @@ fn touch<A>(_a: &A) {} fn f10() { let x = Foo { f: ~"hi", y: 3 }; - consume(x.f); //~ NOTE field of `x` moved here + consume(x.f); //~ NOTE `x.f` moved here touch(&x.y); //~ ERROR use of partially moved value: `x` } fn f20() { let x = ~[~"hi"]; - consume(x[0]); //~ NOTE element of `x` moved here + consume(x[0]); //~ NOTE `(*x)[]` moved here touch(&x[0]); //~ ERROR use of partially moved value: `x` } diff --git a/src/test/compile-fail/moves-based-on-type-block-bad.rs b/src/test/compile-fail/moves-based-on-type-block-bad.rs index 76d50710bb8c1..ca58097b555e1 100644 --- a/src/test/compile-fail/moves-based-on-type-block-bad.rs +++ b/src/test/compile-fail/moves-based-on-type-block-bad.rs @@ -16,9 +16,9 @@ fn main() { let s = S { x: ~Bar(~42) }; loop { do f(&s) |hellothere| { - match hellothere.x { //~ ERROR cannot move out + match hellothere.x { ~Foo(_) => {} - ~Bar(x) => io::println(x.to_str()), + ~Bar(x) => io::println(x.to_str()), //~ ERROR cannot move out ~Baz => {} } } diff --git a/src/test/compile-fail/moves-based-on-type-exprs.rs b/src/test/compile-fail/moves-based-on-type-exprs.rs index 5b733129ee5dc..40ee37fae78a8 100644 --- a/src/test/compile-fail/moves-based-on-type-exprs.rs +++ b/src/test/compile-fail/moves-based-on-type-exprs.rs @@ -86,7 +86,7 @@ fn f110() { } fn f120() { - let x = ~[~"hi", ~"ho"]; + let mut x = ~[~"hi", ~"ho"]; vec::swap(x, 0, 1); touch(&x[0]); touch(&x[1]); diff --git a/src/test/compile-fail/moves-based-on-type-match-bindings.rs b/src/test/compile-fail/moves-based-on-type-match-bindings.rs new file mode 100644 index 0000000000000..42944a206b360 --- /dev/null +++ b/src/test/compile-fail/moves-based-on-type-match-bindings.rs @@ -0,0 +1,19 @@ +// Tests that bindings to move-by-default values trigger moves of the +// discriminant. Also tests that the compiler explains the move in +// terms of the binding, not the discriminant. + +struct Foo<A> { f: A } +fn guard(_s: ~str) -> bool {fail!()} +fn touch<A>(_a: &A) {} + +fn f10() { + let x = Foo {f: ~"hi"}; + + let y = match x { + Foo {f} => {} //~ NOTE moved here + }; + + touch(&x); //~ ERROR use of partially moved value: `x` +} + +fn main() {} diff --git a/src/test/compile-fail/no-reuse-move-arc.rs b/src/test/compile-fail/no-reuse-move-arc.rs index 607127f2fee74..c9e5144557acc 100644 --- a/src/test/compile-fail/no-reuse-move-arc.rs +++ b/src/test/compile-fail/no-reuse-move-arc.rs @@ -15,12 +15,12 @@ fn main() { let v = ~[1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; let arc_v = arc::ARC(v); - do task::spawn() { //~ NOTE `arc_v` moved into closure environment here + do task::spawn() { let v = arc_v.get(); assert_eq!(v[3], 4); }; assert_eq!((arc_v.get())[2], 3); //~ ERROR use of moved value: `arc_v` - info!(arc_v); + info!(arc_v); //~ ERROR use of moved value: `arc_v` } 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 d19b4dfbd57d0..da8e0c9f2b697 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 @@ -9,7 +9,7 @@ impl Drop for S { pub impl S { fn foo(self) -> int { self.bar(); - return self.x; //~ ERROR use of partially moved value + return self.x; //~ ERROR use of moved value: `self` } 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 b2eaffdd06605..37db40d14365e 100644 --- a/src/test/compile-fail/use-after-move-self.rs +++ b/src/test/compile-fail/use-after-move-self.rs @@ -5,7 +5,7 @@ struct S { pub impl S { fn foo(self) -> int { self.bar(); - return *self.x; //~ ERROR use of partially moved value + return *self.x; //~ ERROR use of moved value: `self` } fn bar(self) {} diff --git a/src/test/compile-fail/borrowck-unary-move-2.rs b/src/test/run-pass/borrowck-unary-move-2.rs similarity index 94% rename from src/test/compile-fail/borrowck-unary-move-2.rs rename to src/test/run-pass/borrowck-unary-move-2.rs index 898830bbe55ba..c74fd4a68e719 100644 --- a/src/test/compile-fail/borrowck-unary-move-2.rs +++ b/src/test/run-pass/borrowck-unary-move-2.rs @@ -28,5 +28,5 @@ struct wrapper(noncopyable); fn main() { let x1 = wrapper(noncopyable()); - let _x2 = *x1; //~ ERROR cannot move out + let _x2 = *x1; } diff --git a/src/test/run-pass/move-out-of-field.rs b/src/test/run-pass/move-out-of-field.rs new file mode 100644 index 0000000000000..b6485348a5184 --- /dev/null +++ b/src/test/run-pass/move-out-of-field.rs @@ -0,0 +1,23 @@ +use std::str; + +struct StringBuffer { + s: ~str +} + +impl StringBuffer { + pub fn append(&mut self, v: &str) { + str::push_str(&mut self.s, v); + } +} + +fn to_str(sb: StringBuffer) -> ~str { + sb.s +} + +fn main() { + let mut sb = StringBuffer {s: ~""}; + sb.append("Hello, "); + sb.append("World!"); + let str = to_str(sb); + assert_eq!(str, ~"Hello, World!"); +} \ No newline at end of file diff --git a/src/test/run-pass/vec-matching-fold.rs b/src/test/run-pass/vec-matching-fold.rs index 7dcea2d30b7df..05a6dee06cc87 100644 --- a/src/test/run-pass/vec-matching-fold.rs +++ b/src/test/run-pass/vec-matching-fold.rs @@ -4,8 +4,8 @@ fn foldl<T, U: Copy+Clone>( function: &fn(partial: U, element: &T) -> U ) -> U { match values { - [head, ..tail] => - foldl(tail, function(initial, &head), function), + [ref head, ..tail] => + foldl(tail, function(initial, head), function), [] => initial.clone() } } @@ -16,8 +16,8 @@ fn foldr<T, U: Copy+Clone>( function: &fn(element: &T, partial: U) -> U ) -> U { match values { - [..head, tail] => - foldr(head, function(&tail, initial), function), + [..head, ref tail] => + foldr(head, function(tail, initial), function), [] => initial.clone() } } diff --git a/src/test/run-pass/vec-tail-matching.rs b/src/test/run-pass/vec-tail-matching.rs index cf4aebbd08270..6e1a47ad2dfd3 100644 --- a/src/test/run-pass/vec-tail-matching.rs +++ b/src/test/run-pass/vec-tail-matching.rs @@ -19,9 +19,9 @@ pub fn main() { [Foo { _ }, _, Foo { _ }, ..tail] => { ::std::util::unreachable(); } - [Foo { string: a }, Foo { string: b }] => { - assert_eq!(a, ~"bar"); - assert_eq!(b, ~"baz"); + [Foo { string: ref a }, Foo { string: ref b }] => { + assert_eq!("bar", a.slice(0, a.len())); + assert_eq!("baz", b.slice(0, b.len())); } _ => { ::std::util::unreachable();