Skip to content

Change check_loans to use ExprUseVisitor #14318

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jun 6, 2014
481 changes: 248 additions & 233 deletions src/librustc/middle/borrowck/check_loans.rs

Large diffs are not rendered by default.

9 changes: 7 additions & 2 deletions src/librustc/middle/borrowck/gather_loans/gather_moves.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,15 @@ pub fn gather_move_from_expr(bccx: &BorrowckCtxt,
move_data: &MoveData,
move_error_collector: &MoveErrorCollector,
move_expr_id: ast::NodeId,
cmt: mc::cmt) {
cmt: mc::cmt,
move_reason: euv::MoveReason) {
let kind = match move_reason {
euv::DirectRefMove | euv::PatBindingMove => MoveExpr,
euv::CaptureMove => Captured
};
let move_info = GatherMoveInfo {
id: move_expr_id,
kind: MoveExpr,
kind: kind,
cmt: cmt,
span_path_opt: None,
};
Expand Down
14 changes: 7 additions & 7 deletions src/librustc/middle/borrowck/gather_loans/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,13 @@ impl<'a> euv::Delegate for GatherLoanCtxt<'a> {
consume_id, cmt.repr(self.tcx()), mode);

match mode {
euv::Copy => { return; }
euv::Move => { }
euv::Move(move_reason) => {
gather_moves::gather_move_from_expr(
self.bccx, &self.move_data, &self.move_error_collector,
consume_id, cmt, move_reason);
}
euv::Copy => { }
}

gather_moves::gather_move_from_expr(
self.bccx, &self.move_data, &self.move_error_collector,
consume_id, cmt);
}

fn consume_pat(&mut self,
Expand All @@ -95,7 +95,7 @@ impl<'a> euv::Delegate for GatherLoanCtxt<'a> {

match mode {
euv::Copy => { return; }
euv::Move => { }
euv::Move(_) => { }
}

gather_moves::gather_move_from_pat(
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/borrowck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ fn borrowck_fn(this: &mut BorrowckCtxt,
body);

check_loans::check_loans(this, &loan_dfcx, flowed_moves,
all_loans.as_slice(), body);
all_loans.as_slice(), decl, body);

visit::walk_fn(this, fk, decl, body, sp, ());
}
Expand Down
25 changes: 24 additions & 1 deletion src/librustc/middle/borrowck/move_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ pub struct MovePath {
pub next_sibling: MovePathIndex,
}

#[deriving(PartialEq)]
pub enum MoveKind {
Declared, // When declared, variables start out "moved".
MoveExpr, // Expression or binding that moves a variable
Expand Down Expand Up @@ -356,7 +357,7 @@ impl MoveData {
let path_index = self.move_path(tcx, lp.clone());

match mode {
euv::JustWrite => {
euv::Init | euv::JustWrite => {
self.assignee_ids.borrow_mut().insert(assignee_id);
}
euv::WriteAndRead => { }
Expand Down Expand Up @@ -537,6 +538,28 @@ impl<'a> FlowedMoveData<'a> {
})
}

pub fn kind_of_move_of_path(&self,
id: ast::NodeId,
loan_path: &Rc<LoanPath>)
-> Option<MoveKind> {
//! Returns the kind of a move of `loan_path` by `id`, if one exists.

let mut ret = None;
for loan_path_index in self.move_data.path_map.borrow().find(&*loan_path).iter() {
self.dfcx_moves.each_gen_bit_frozen(id, |move_index| {
let move = self.move_data.moves.borrow();
let move = move.get(move_index);
if move.path == **loan_path_index {
ret = Some(move.kind);
false
} else {
true
}
});
}
ret
}

pub fn each_move_of(&self,
id: ast::NodeId,
loan_path: &Rc<LoanPath>,
Expand Down
25 changes: 17 additions & 8 deletions src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,20 @@ pub enum LoanCause {

#[deriving(PartialEq,Show)]
pub enum ConsumeMode {
Copy, // reference to x where x has a type that copies
Move, // reference to x where x has a type that moves
Copy, // reference to x where x has a type that copies
Move(MoveReason), // reference to x where x has a type that moves
}

#[deriving(PartialEq,Show)]
pub enum MoveReason {
DirectRefMove,
PatBindingMove,
CaptureMove,
}

#[deriving(PartialEq,Show)]
pub enum MutateMode {
Init,
JustWrite, // x = y
WriteAndRead, // x += y
}
Expand Down Expand Up @@ -160,7 +168,7 @@ impl<'d,'t,TYPER:mc::Typer> ExprUseVisitor<'d,'t,TYPER> {
consume_id: ast::NodeId,
consume_span: Span,
cmt: mc::cmt) {
let mode = copy_or_move(self.tcx(), cmt.ty);
let mode = copy_or_move(self.tcx(), cmt.ty, DirectRefMove);
self.delegate.consume(consume_id, consume_span, cmt, mode);
}

Expand Down Expand Up @@ -712,7 +720,7 @@ impl<'d,'t,TYPER:mc::Typer> ExprUseVisitor<'d,'t,TYPER> {
let def = def_map.borrow().get_copy(&pat.id);
match mc.cat_def(pat.id, pat.span, pat_ty, def) {
Ok(binding_cmt) => {
delegate.mutate(pat.id, pat.span, binding_cmt, JustWrite);
delegate.mutate(pat.id, pat.span, binding_cmt, Init);
}
Err(_) => { }
}
Expand All @@ -728,7 +736,7 @@ impl<'d,'t,TYPER:mc::Typer> ExprUseVisitor<'d,'t,TYPER> {
r, bk, RefBinding);
}
ast::PatIdent(ast::BindByValue(_), _, _) => {
let mode = copy_or_move(typer.tcx(), cmt_pat.ty);
let mode = copy_or_move(typer.tcx(), cmt_pat.ty, PatBindingMove);
delegate.consume_pat(pat, cmt_pat, mode);
}
_ => {
Expand Down Expand Up @@ -834,7 +842,8 @@ impl<'d,'t,TYPER:mc::Typer> ExprUseVisitor<'d,'t,TYPER> {
let cmt_var = return_if_err!(self.cat_captured_var(closure_expr.id,
closure_expr.span,
freevar.def));
self.delegate_consume(closure_expr.id, freevar.span, cmt_var);
let mode = copy_or_move(self.tcx(), cmt_var.ty, CaptureMove);
self.delegate.consume(closure_expr.id, freevar.span, cmt_var, mode);
}
}

Expand All @@ -851,7 +860,7 @@ impl<'d,'t,TYPER:mc::Typer> ExprUseVisitor<'d,'t,TYPER> {
}
}

fn copy_or_move(tcx: &ty::ctxt, ty: ty::t) -> ConsumeMode {
if ty::type_moves_by_default(tcx, ty) { Move } else { Copy }
fn copy_or_move(tcx: &ty::ctxt, ty: ty::t, move_reason: MoveReason) -> ConsumeMode {
if ty::type_moves_by_default(tcx, ty) { Move(move_reason) } else { Copy }
}

8 changes: 3 additions & 5 deletions src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,12 +390,10 @@ impl<'t,TYPER:Typer> MemCategorizationContext<'t,TYPER> {
Some(adjustment) => {
match *adjustment {
ty::AutoObject(..) => {
// Implicity casts a concrete object to trait object
// so just patch up the type
// Implicity cast a concrete object to trait object.
// Result is an rvalue.
let expr_ty = if_ok!(self.expr_ty_adjusted(expr));
let mut expr_cmt = (*if_ok!(self.cat_expr_unadjusted(expr))).clone();
expr_cmt.ty = expr_ty;
Ok(Rc::new(expr_cmt))
Ok(self.cat_rvalue_node(expr.id(), expr.span(), expr_ty))
}

ty::AutoAddEnv(..) => {
Expand Down
124 changes: 124 additions & 0 deletions src/test/compile-fail/borrowck-field-sensitivity.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

struct A { a: int, b: Box<int> }

fn borrow<T>(_: &T) { }

fn use_after_move() {
let x = A { a: 1, b: box 2 };
drop(x.b);
drop(*x.b); //~ ERROR use of partially moved value: `*x.b`
}

fn use_after_fu_move() {
let x = A { a: 1, b: box 2 };
let y = A { a: 3, .. x };
drop(*x.b); //~ ERROR use of partially moved value: `*x.b`
}

fn borrow_after_move() {
let x = A { a: 1, b: box 2 };
drop(x.b);
borrow(&x.b); //~ ERROR use of moved value: `x.b`
}

fn borrow_after_fu_move() {
let x = A { a: 1, b: box 2 };
let _y = A { a: 3, .. x };
borrow(&x.b); //~ ERROR use of moved value: `x.b`
}

fn move_after_borrow() {
let x = A { a: 1, b: box 2 };
let y = &x.b;
drop(x.b); //~ ERROR cannot move out of `x.b` because it is borrowed
borrow(&*y);
}

fn fu_move_after_borrow() {
let x = A { a: 1, b: box 2 };
let y = &x.b;
let _z = A { a: 3, .. x }; //~ ERROR cannot move out of `x.b` because it is borrowed
borrow(&*y);
}

fn mut_borrow_after_mut_borrow() {
let mut x = A { a: 1, b: box 2 };
let y = &mut x.a;
let z = &mut x.a; //~ ERROR cannot borrow `x.a` as mutable more than once at a time
drop(*y);
drop(*z);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am surprised that we can drop *y and *z, as that is borrowed content. Is this simply an artifact of us not reporting an error for some other reason?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine because *y and *z are integers, and they seem to get copied rather than moved. This code compiles fine:

fn foo() {
    let mut x = A { a: 1, b: box 2 };
    let y = &mut x.a;
    drop(*y);
}

but if I use the x.b field, which is a Box<int>, like this:

fn bar() {
    let mut x = A { a: 1, b: box 2 };
    let y = &mut x.b;
    drop(*y);
}

then I get an error:

test.rs:24:10: 24:12 error: cannot move out of dereference of `&mut`-pointer
test.rs:24     drop(*y);
                    ^~

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Wed, May 28, 2014 at 12:46:04PM -0700, Cameron Zwarich wrote:

It's fine because *y and *z are integers, and they seem to get copied rather than moved.

Indeed. I overlooked that.

}

fn move_after_move() {
let x = A { a: 1, b: box 2 };
drop(x.b);
drop(x.b); //~ ERROR use of moved value: `x.b`
}

fn move_after_fu_move() {
let x = A { a: 1, b: box 2 };
let _y = A { a: 3, .. x };
drop(x.b); //~ ERROR use of moved value: `x.b`
}

fn fu_move_after_move() {
let x = A { a: 1, b: box 2 };
drop(x.b);
let _z = A { a: 3, .. x }; //~ ERROR use of moved value: `x.b`
}

fn fu_move_after_fu_move() {
let x = A { a: 1, b: box 2 };
let _y = A { a: 3, .. x };
let _z = A { a: 4, .. x }; //~ ERROR use of moved value: `x.b`
}

// The following functions aren't yet accepted, but they should be.

fn use_after_field_assign_after_uninit() {
let mut x: A;
x.a = 1;
drop(x.a); //~ ERROR use of possibly uninitialized variable: `x.a`
}

fn borrow_after_field_assign_after_uninit() {
let mut x: A;
x.a = 1;
borrow(&x.a); //~ ERROR use of possibly uninitialized variable: `x.a`
}

fn move_after_field_assign_after_uninit() {
let mut x: A;
x.b = box 1;
drop(x.b); //~ ERROR use of possibly uninitialized variable: `x.b`
}

fn main() {
use_after_move();
use_after_fu_move();

borrow_after_move();
borrow_after_fu_move();
move_after_borrow();
fu_move_after_borrow();
mut_borrow_after_mut_borrow();

move_after_move();
move_after_fu_move();
fu_move_after_move();
fu_move_after_fu_move();

use_after_field_assign_after_uninit();
borrow_after_field_assign_after_uninit();
move_after_field_assign_after_uninit();
}

2 changes: 1 addition & 1 deletion src/test/compile-fail/borrowck-init-in-fru.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ struct point {

fn main() {
let mut origin: point;
origin = point {x: 10,.. origin}; //~ ERROR use of possibly uninitialized variable: `origin`
origin = point {x: 10,.. origin}; //~ ERROR use of possibly uninitialized variable: `origin.y`
origin.clone();
}
64 changes: 64 additions & 0 deletions src/test/compile-fail/borrowck-multiple-captures.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::task;

fn borrow<T>(_: &T) { }

fn different_vars_after_borrows() {
let x1 = box 1;
let p1 = &x1;
let x2 = box 2;
let p2 = &x2;
task::spawn(proc() {
drop(x1); //~ ERROR cannot move `x1` into closure because it is borrowed
drop(x2); //~ ERROR cannot move `x2` into closure because it is borrowed
});
borrow(&*p1);
borrow(&*p2);
}

fn different_vars_after_moves() {
let x1 = box 1;
drop(x1);
let x2 = box 2;
drop(x2);
task::spawn(proc() {
drop(x1); //~ ERROR capture of moved value: `x1`
drop(x2); //~ ERROR capture of moved value: `x2`
});
}

fn same_var_after_borrow() {
let x = box 1;
let p = &x;
task::spawn(proc() {
drop(x); //~ ERROR cannot move `x` into closure because it is borrowed
drop(x); //~ ERROR use of moved value: `x`
});
borrow(&*p);
}

fn same_var_after_move() {
let x = box 1;
drop(x);
task::spawn(proc() {
drop(x); //~ ERROR capture of moved value: `x`
drop(x); //~ ERROR use of moved value: `x`
});
}

fn main() {
different_vars_after_borrows();
different_vars_after_moves();
same_var_after_borrow();
same_var_after_move();
}

2 changes: 1 addition & 1 deletion src/test/compile-fail/liveness-use-after-move.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ extern crate debug;
fn main() {
let x = box 5;
let y = x;
println!("{:?}", *x); //~ ERROR use of moved value: `x`
println!("{:?}", *x); //~ ERROR use of partially moved value: `*x`
y.clone();
}
Loading