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..f2bba4f694a90 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, } 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,7 +65,6 @@ pub fn check_loans(bccx: @BorrowckCtxt, enum MoveError { MoveOk, - MoveFromIllegalCmt(mc::cmt), MoveWhileBorrowed(/*loan*/@LoanPath, /*loan*/span) } @@ -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; @@ -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,12 +557,6 @@ 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) => { self.bccx.span_err( cmt.span, @@ -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` @@ -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(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))); } } - - 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/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 or the MIT license +// , 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/mod.rs b/src/librustc/middle/borrowck/gather_loans/mod.rs index a422d99b6f5cf..893365bdec195 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); } @@ -558,8 +622,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/mod.rs b/src/librustc/middle/borrowck/mod.rs index 39479e726f8b6..4554fde15fad5 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; @@ -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 @@ -407,6 +416,11 @@ pub enum AliasableViolationKind { BorrowViolation } +pub enum MovedValueUseKind { + MovedInUse, + MovedInCapture, +} + /////////////////////////////////////////////////////////////////////////// // Misc @@ -419,6 +433,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 +496,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..84bd7ecb1a167 --- /dev/null +++ b/src/librustc/middle/borrowck/move_data.rs @@ -0,0 +1,568 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +/*! + +Data structures used for tracking moves. + +*/ + +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 { + paths: ~[MovePath], + path_map: HashMap<@LoanPath, MovePathIndex>, + moves: ~[Move], + path_assignments: ~[Assignment], + var_assignments: ~[Assignment], + assignee_ids: HashSet, +} + +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, + dfcx_assign: AssignDataFlow +} + +#[deriving(Eq)] +pub struct MovePathIndex(uint); + +static InvalidMovePathIndex: MovePathIndex = + MovePathIndex(uint::max_value); + +#[deriving(Eq)] +pub struct MoveIndex(uint); + +static InvalidMoveIndex: MoveIndex = + MoveIndex(uint::max_value); + +#[deriving(Eq)] +pub struct VarAssignmentIndex(uint); + +static InvalidVarAssignmentIndex: VarAssignmentIndex = + VarAssignmentIndex(uint::max_value); + +pub struct MovePath { + index: MovePathIndex, + loan_path: @LoanPath, + parent: MovePathIndex, + first_move: MoveIndex, + first_child: MovePathIndex, + 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: MovePathIndex, + id: ast::node_id, + kind: MoveKind, + next_move: MoveIndex, +} + +pub struct Assignment { + path: MovePathIndex, + id: ast::node_id, + span: span, +} + +pub struct MoveDataFlowOperator; +pub type MoveDataFlow = DataFlowContext; + +pub struct AssignDataFlowOperator; +pub type AssignDataFlow = DataFlowContext; + +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 { + index: index, + 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 { + index: index, + 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 { + self.path_map.find_copy(&lp) + } + + fn existing_base_paths(&self, + lp: @LoanPath) + -> OptVec { + 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) { + /*! + * 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) { - 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 { 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..159f7707dd383 100644 --- a/src/librustc/middle/moves.rs +++ b/src/librustc/middle/moves.rs @@ -88,32 +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 +FIXME out of date + 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 @@ -136,12 +136,8 @@ 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. +contains the outermost expressions that are moved. However, the id of +`x` would be present in the `moved_variables_set`. Now let's look at another illegal example, but one where liveness would not catch the error: @@ -213,6 +209,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 +217,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 +237,6 @@ pub type CaptureMap = @mut HashMap; pub type MovesMap = @mut HashSet; -/** - * For each variable which will be moved, links to the - * expression */ -pub type VariableMovesMap = @mut HashMap; - /** * Set of variable node-ids that are moved. * @@ -257,7 +248,6 @@ pub type MovedVariablesSet = @mut HashSet; /** 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 +259,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 +276,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 +305,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 +320,20 @@ pub impl VisitContext { visitor: vt) { /*! - * * 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 +341,6 @@ pub impl VisitContext { visitor: vt) { /*! - * * Indicates that the value of `blk` will be consumed, * meaning either copied or moved depending on its type. */ @@ -382,46 +356,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) { /*! - * * 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 +377,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 +385,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 +486,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 +650,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 +689,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>, 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-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-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-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) {} 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 { f: A } +fn guard(_s: ~str) -> bool {fail!()} +fn touch(_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( 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( 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();