Skip to content

Commit

Permalink
Move checking for moves and initialization of local variables and pat…
Browse files Browse the repository at this point in the history 
…terns into

borrow checker and generalize what moves are allowed. Fixes a nasty
bug or two in the pattern move checking code. Unifies dataflow code
used for initialization and other things. First step towards
once fns. Everybody wins.

Fixes rust-lang#4384. Fixes rust-lang#4715. cc once fns (rust-lang#2202), optimizing local moves (rust-lang#5016).
  • Loading branch information
@nikomatsakis
nikomatsakis committed May 29, 2013
1 parent 5676056 commit 5851d32
Show file tree
Hide file tree
Showing 41 changed files with 1,282 additions and 614 deletions.
12 changes: 6 additions & 6 deletions src/librust/rust.rc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, []);
Expand All @@ -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);
Expand Down
5 changes: 2 additions & 3 deletions src/librustc/driver/driver.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand All @@ -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) =
Expand Down
229 changes: 115 additions & 114 deletions src/librustc/middle/borrowck/check_loans.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
};
Expand All @@ -62,7 +65,6 @@ pub fn check_loans(bccx: @BorrowckCtxt,

enum MoveError {
MoveOk,
MoveFromIllegalCmt(mc::cmt),
MoveWhileBorrowed(/*loan*/@LoanPath, /*loan*/span)
}

Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -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`

Expand Down Expand Up @@ -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 => {}
}
}
}
Expand All @@ -692,18 +682,29 @@ 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) {
this.check_move_out_from_expr(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);
Expand Down
Loading

0 comments on commit 5851d32

Please sign in to comment.