Change check_loans to use ExprUseVisitor.

This commit is contained in:
Cameron Zwarich 2014-06-06 11:59:33 -07:00
parent 78934b03e3
commit c53d296e12
9 changed files with 226 additions and 233 deletions

View file

@ -20,15 +20,10 @@
use middle::borrowck::*;
use euv = middle::expr_use_visitor;
use middle::freevars;
use mc = middle::mem_categorization;
use middle::ty;
use middle::typeck::MethodCall;
use syntax::ast;
use syntax::ast_util;
use syntax::codemap::Span;
use syntax::visit::Visitor;
use syntax::visit;
use util::ppaux::Repr;
use std::rc::Rc;
@ -40,35 +35,104 @@ struct CheckLoanCtxt<'a> {
all_loans: &'a [Loan],
}
impl<'a> Visitor<()> for CheckLoanCtxt<'a> {
impl<'a> euv::Delegate for CheckLoanCtxt<'a> {
fn consume(&mut self,
consume_id: ast::NodeId,
consume_span: Span,
cmt: mc::cmt,
mode: euv::ConsumeMode) {
debug!("consume(consume_id={}, cmt={}, mode={})",
consume_id, cmt.repr(self.tcx()), mode);
fn visit_expr(&mut self, ex: &ast::Expr, _: ()) {
check_loans_in_expr(self, ex);
}
fn visit_local(&mut self, l: &ast::Local, _: ()) {
check_loans_in_local(self, l);
}
fn visit_block(&mut self, b: &ast::Block, _: ()) {
check_loans_in_block(self, b);
}
fn visit_pat(&mut self, p: &ast::Pat, _: ()) {
check_loans_in_pat(self, p);
}
fn visit_fn(&mut self, _fk: &visit::FnKind, _fd: &ast::FnDecl,
_b: &ast::Block, _s: Span, _n: ast::NodeId, _: ()) {
// Don't process nested items or closures here,
// the outer loop will take care of it.
return;
self.consume_common(consume_id, consume_span, cmt, mode);
}
// FIXME(#10894) should continue recursing
fn visit_ty(&mut self, _t: &ast::Ty, _: ()) {}
fn consume_pat(&mut self,
consume_pat: &ast::Pat,
cmt: mc::cmt,
mode: euv::ConsumeMode) {
debug!("consume_pat(consume_pat={}, cmt={}, mode={})",
consume_pat.repr(self.tcx()),
cmt.repr(self.tcx()),
mode);
self.consume_common(consume_pat.id, consume_pat.span, cmt, mode);
}
fn borrow(&mut self,
borrow_id: ast::NodeId,
borrow_span: Span,
cmt: mc::cmt,
loan_region: ty::Region,
bk: ty::BorrowKind,
loan_cause: euv::LoanCause)
{
debug!("borrow(borrow_id={}, cmt={}, loan_region={}, \
bk={}, loan_cause={:?})",
borrow_id, cmt.repr(self.tcx()), loan_region,
bk, loan_cause);
match opt_loan_path(&cmt) {
Some(lp) => {
let moved_value_use_kind = match loan_cause {
euv::ClosureCapture(_) => MovedInCapture,
_ => MovedInUse,
};
self.check_if_path_is_moved(borrow_id, borrow_span, moved_value_use_kind, &lp);
}
None => { }
}
self.check_for_conflicting_loans(borrow_id);
}
fn mutate(&mut self,
assignment_id: ast::NodeId,
assignment_span: Span,
assignee_cmt: mc::cmt,
mode: euv::MutateMode)
{
debug!("mutate(assignment_id={}, assignee_cmt={})",
assignment_id, assignee_cmt.repr(self.tcx()));
match opt_loan_path(&assignee_cmt) {
Some(lp) => {
match mode {
euv::Init | euv::JustWrite => {
// In a case like `path = 1`, then path does not
// have to be *FULLY* initialized, but we still
// must be careful lest it contains derefs of
// pointers.
self.check_if_assigned_path_is_moved(assignee_cmt.id,
assignment_span,
MovedInUse,
&lp);
}
euv::WriteAndRead => {
// In a case like `path += 1`, then path must be
// fully initialized, since we will read it before
// we write it.
self.check_if_path_is_moved(assignee_cmt.id,
assignment_span,
MovedInUse,
&lp);
}
}
}
None => { }
}
self.check_assignment(assignment_id, assignment_span, assignee_cmt, mode);
}
fn decl_without_init(&mut self, _id: ast::NodeId, _span: Span) { }
}
pub fn check_loans(bccx: &BorrowckCtxt,
dfcx_loans: &LoanDataFlow,
move_data: move_data::FlowedMoveData,
all_loans: &[Loan],
decl: &ast::FnDecl,
body: &ast::Block) {
debug!("check_loans(body id={:?})", body.id);
@ -79,7 +143,10 @@ pub fn check_loans(bccx: &BorrowckCtxt,
all_loans: all_loans,
};
clcx.visit_block(body, ());
{
let mut euv = euv::ExprUseVisitor::new(&mut clcx, bccx.tcx);
euv.walk_fn(decl, body);
}
}
#[deriving(PartialEq)]
@ -355,18 +422,130 @@ impl<'a> CheckLoanCtxt<'a> {
true
}
pub fn is_local_variable(&self, cmt: mc::cmt) -> bool {
pub fn is_local_variable_or_arg(&self, cmt: mc::cmt) -> bool {
match cmt.cat {
mc::cat_local(_) => true,
mc::cat_local(_) | mc::cat_arg(_) => true,
_ => false
}
}
pub fn check_if_path_is_moved(&self,
id: ast::NodeId,
span: Span,
use_kind: MovedValueUseKind,
lp: &Rc<LoanPath>) {
fn consume_common(&self,
id: ast::NodeId,
span: Span,
cmt: mc::cmt,
mode: euv::ConsumeMode) {
match opt_loan_path(&cmt) {
Some(lp) => {
let moved_value_use_kind = match mode {
euv::Copy => {
// FIXME(#12624) -- If we are copying the value,
// we don't care if it's borrowed.
MovedInUse
}
euv::Move(_) => {
match self.move_data.kind_of_move_of_path(id, &lp) {
None => {
// Sometimes moves don't have a move kind;
// this either means that the original move
// was from something illegal to move,
// or was moved from referent of an unsafe
// pointer or something like that.
MovedInUse
}
Some(move_kind) => {
self.check_for_move_of_borrowed_path(id, span,
&lp, move_kind);
if move_kind == move_data::Captured {
MovedInCapture
} else {
MovedInUse
}
}
}
}
};
self.check_if_path_is_moved(id, span, moved_value_use_kind, &lp);
}
None => { }
}
}
fn check_for_move_of_borrowed_path(&self,
id: ast::NodeId,
span: Span,
move_path: &Rc<LoanPath>,
move_kind: move_data::MoveKind) {
match self.analyze_move_out_from(id, &**move_path) {
MoveOk => { }
MoveWhileBorrowed(loan_path, loan_span) => {
let err_message = match move_kind {
move_data::Captured =>
format!("cannot move `{}` into closure because it is borrowed",
self.bccx.loan_path_to_str(&**move_path).as_slice()),
move_data::Declared |
move_data::MoveExpr |
move_data::MovePat =>
format!("cannot move out of `{}` because it is borrowed",
self.bccx.loan_path_to_str(&**move_path).as_slice())
};
self.bccx.span_err(span, err_message.as_slice());
self.bccx.span_note(
loan_span,
format!("borrow of `{}` occurs here",
self.bccx.loan_path_to_str(&*loan_path).as_slice())
.as_slice());
}
}
}
fn check_if_assigned_path_is_moved(&self,
id: ast::NodeId,
span: Span,
use_kind: MovedValueUseKind,
lp: &Rc<LoanPath>)
{
/*!
* Reports an error if assigning to `lp` will use a
* moved/uninitialized value. Mainly this is concerned with
* detecting derefs of uninitialized pointers.
*
* For example:
*
* let a: int;
* a = 10; // ok, even though a is uninitialized
*
* struct Point { x: uint, y: uint }
* let p: Point;
* p.x = 22; // ok, even though `p` is uninitialized
*
* let p: ~Point;
* (*p).x = 22; // not ok, p is uninitialized, can't deref
*/
match **lp {
LpVar(_) => {
// assigning to `x` does not require that `x` is initialized
}
LpExtend(ref lp_base, _, LpInterior(_)) => {
// assigning to `P.f` is ok if assigning to `P` is ok
self.check_if_assigned_path_is_moved(id, span,
use_kind, lp_base);
}
LpExtend(ref lp_base, _, LpDeref(_)) => {
// assigning to `(*P)` requires that `P` be initialized
self.check_if_path_is_moved(id, span,
use_kind, lp_base);
}
}
}
fn check_if_path_is_moved(&self,
id: ast::NodeId,
span: Span,
use_kind: MovedValueUseKind,
lp: &Rc<LoanPath>) {
/*!
* Reports an error if `expr` (which should be a path)
* is using a moved/uninitialized value
@ -385,32 +564,21 @@ impl<'a> CheckLoanCtxt<'a> {
});
}
pub fn check_assignment_expr(&self, expr: &ast::Expr) {
let assignment_id = expr.id;
let assignment_span = expr.span;
// We don't use cat_expr() here because we don't want to treat
// auto-ref'd parameters in overloaded operators as rvalues.
let assignee_cmt = match self.bccx.tcx.adjustments.borrow().find(&assignment_id) {
None => self.bccx.cat_expr_unadjusted(expr),
Some(adj) => self.bccx.cat_expr_autoderefd(expr, adj)
};
self.check_assignment(assignment_id, assignment_span, assignee_cmt);
}
fn check_assignment(&self,
assignment_id: ast::NodeId,
assignment_span: Span,
assignee_cmt: mc::cmt) {
assignee_cmt: mc::cmt,
mode: euv::MutateMode) {
debug!("check_assignment(assignee_cmt={})", assignee_cmt.repr(self.tcx()));
// Mutable values can be assigned, as long as they obey loans
// and aliasing restrictions:
if assignee_cmt.mutbl.is_mutable() {
if check_for_aliasable_mutable_writes(self, assignment_span, assignee_cmt.clone()) {
if check_for_assignment_to_restricted_or_frozen_location(
self, assignment_id, assignment_span, assignee_cmt.clone()) {
if mode != euv::Init &&
check_for_assignment_to_restricted_or_frozen_location(
self, assignment_id, assignment_span, assignee_cmt.clone())
{
// Safe, but record for lint pass later:
mark_variable_as_used_mut(self, assignee_cmt);
}
@ -420,7 +588,7 @@ impl<'a> CheckLoanCtxt<'a> {
// For immutable local variables, assignments are legal
// if they cannot already have been assigned
if self.is_local_variable(assignee_cmt.clone()) {
if self.is_local_variable_or_arg(assignee_cmt.clone()) {
assert!(assignee_cmt.mutbl.is_immutable()); // no "const" locals
let lp = opt_loan_path(&assignee_cmt).unwrap();
self.move_data.each_assignment_of(assignment_id, &lp, |assign| {
@ -695,86 +863,6 @@ impl<'a> CheckLoanCtxt<'a> {
self.bccx.loan_path_to_str(loan_path)).as_slice());
}
fn check_move_out_from_expr(&self, expr: &ast::Expr) {
match expr.node {
ast::ExprFnBlock(..) | ast::ExprProc(..) => {
// Moves due to captures are checked in
// check_captured_variables() because it allows
// us to give a more precise error message with
// a more precise span.
}
_ => {
self.check_move_out_from_id(expr.id, expr.span)
}
}
}
fn check_move_out_from_id(&self, id: ast::NodeId, span: Span) {
self.move_data.each_path_moved_by(id, |_, move_path| {
match self.analyze_move_out_from(id, move_path) {
MoveOk => {}
MoveWhileBorrowed(loan_path, loan_span) => {
self.bccx.span_err(
span,
format!("cannot move out of `{}` \
because it is borrowed",
self.bccx.loan_path_to_str(
move_path)).as_slice());
self.bccx.span_note(
loan_span,
format!("borrow of `{}` occurs here",
self.bccx.loan_path_to_str(
&*loan_path)).as_slice());
}
}
true
});
}
fn check_captured_variables(&self,
closure_id: ast::NodeId,
span: Span) {
let freevar_mode = freevars::get_capture_mode(self.tcx(), closure_id);
freevars::with_freevars(self.tcx(), closure_id, |freevars| {
for freevar in freevars.iter() {
let var_id = ast_util::def_id_of_def(freevar.def).node;
let var_path = Rc::new(LpVar(var_id));
self.check_if_path_is_moved(closure_id, span,
MovedInCapture, &var_path);
match freevar_mode {
freevars::CaptureByRef => { }
freevars::CaptureByValue => {
check_by_move_capture(self, closure_id, freevar.span, &*var_path);
}
}
}
});
return;
fn check_by_move_capture(this: &CheckLoanCtxt,
closure_id: ast::NodeId,
freevar_span: Span,
move_path: &LoanPath) {
let move_err = this.analyze_move_out_from(closure_id, move_path);
match move_err {
MoveOk => {}
MoveWhileBorrowed(loan_path, loan_span) => {
this.bccx.span_err(
freevar_span,
format!("cannot move `{}` into closure \
because it is borrowed",
this.bccx.loan_path_to_str(
move_path)).as_slice());
this.bccx.span_note(
loan_span,
format!("borrow of `{}` occurs here",
this.bccx.loan_path_to_str(
&*loan_path)).as_slice());
}
}
}
}
pub fn analyze_move_out_from(&self,
expr_id: ast::NodeId,
move_path: &LoanPath)
@ -805,89 +893,5 @@ impl<'a> CheckLoanCtxt<'a> {
}
}
}
pub fn check_call(&self,
_expr: &ast::Expr,
_callee: Option<@ast::Expr>,
_callee_span: Span,
_args: &[@ast::Expr]) {
// NB: This call to check for conflicting loans is not truly
// necessary, because the callee_id never issues new loans.
// However, I added it for consistency and lest the system
// should change in the future.
//
// FIXME(#6268) nested method calls
// self.check_for_conflicting_loans(callee_id);
}
}
fn check_loans_in_local<'a>(this: &mut CheckLoanCtxt<'a>,
local: &ast::Local) {
visit::walk_local(this, local, ());
}
fn check_loans_in_expr<'a>(this: &mut CheckLoanCtxt<'a>,
expr: &ast::Expr) {
visit::walk_expr(this, expr, ());
debug!("check_loans_in_expr(expr={})",
expr.repr(this.tcx()));
this.check_for_conflicting_loans(expr.id);
this.check_move_out_from_expr(expr);
let method_map = this.bccx.tcx.method_map.borrow();
match expr.node {
ast::ExprPath(..) => {
if !this.move_data.is_assignee(expr.id) {
let cmt = this.bccx.cat_expr_unadjusted(expr);
debug!("path cmt={}", cmt.repr(this.tcx()));
for lp in opt_loan_path(&cmt).iter() {
this.check_if_path_is_moved(expr.id, expr.span, MovedInUse, lp);
}
}
}
ast::ExprFnBlock(..) | ast::ExprProc(..) => {
this.check_captured_variables(expr.id, expr.span)
}
ast::ExprAssign(dest, _) |
ast::ExprAssignOp(_, dest, _) => {
this.check_assignment_expr(dest);
}
ast::ExprCall(f, ref args) => {
this.check_call(expr, Some(f), f.span, args.as_slice());
}
ast::ExprMethodCall(_, _, ref args) => {
this.check_call(expr, None, expr.span, args.as_slice());
}
ast::ExprIndex(_, rval) | ast::ExprBinary(_, _, rval)
if method_map.contains_key(&MethodCall::expr(expr.id)) => {
this.check_call(expr, None, expr.span, [rval]);
}
ast::ExprUnary(_, _) | ast::ExprIndex(_, _)
if method_map.contains_key(&MethodCall::expr(expr.id)) => {
this.check_call(expr, None, expr.span, []);
}
ast::ExprInlineAsm(ref ia) => {
for &(_, out) in ia.outputs.iter() {
this.check_assignment_expr(out);
}
}
_ => {}
}
}
fn check_loans_in_pat<'a>(this: &mut CheckLoanCtxt<'a>,
pat: &ast::Pat)
{
this.check_for_conflicting_loans(pat.id);
this.check_move_out_from_id(pat.id, pat.span);
visit::walk_pat(this, pat, ());
}
fn check_loans_in_block<'a>(this: &mut CheckLoanCtxt<'a>,
blk: &ast::Block)
{
visit::walk_block(this, blk, ());
this.check_for_conflicting_loans(blk.id);
}

View file

@ -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, ());
}

View file

@ -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

View file

@ -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();
}

View file

@ -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();
}

View file

@ -17,12 +17,6 @@ struct Foo { f: String, y: int }
fn consume(_s: String) {}
fn touch<A>(_a: &A) {}
fn f10() {
let x = Foo { f: "hi".to_string(), y: 3 };
consume(x.f);
touch(&x.y); //~ ERROR use of partially moved value: `x`
}
fn f20() {
let x = vec!("hi".to_string());
consume(x.move_iter().next().unwrap());

View file

@ -26,13 +26,7 @@ fn test0(f: Foo, g: Noncopyable, h: Noncopyable) {
fn test1(f: Foo, g: Noncopyable, h: Noncopyable) {
// copying move-by-default fields from `f`, so move:
let _b = Foo {noncopyable: g, ..f};
let _c = Foo {noncopyable: h, ..f}; //~ ERROR use of partially moved value: `f`
}
fn test2(f: Foo, g: Noncopyable) {
// move non-copyable field
let _b = Foo {copied: 22, moved: box 23, ..f};
let _c = Foo {noncopyable: g, ..f}; //~ ERROR use of partially moved value: `f`
let _c = Foo {noncopyable: h, ..f}; //~ ERROR use of moved value: `f.moved`
}
fn main() {}

View file

@ -19,7 +19,7 @@ impl Drop for S {
impl S {
pub fn foo(self) -> int {
self.bar();
return self.x; //~ ERROR use of moved value: `self`
return self.x; //~ ERROR use of partially moved value: `self.x`
}
pub fn bar(self) {}

View file

@ -16,7 +16,7 @@ struct S {
impl S {
pub fn foo(self) -> int {
self.bar();
return *self.x; //~ ERROR use of moved value: `self`
return *self.x; //~ ERROR use of partially moved value: `*self.x`
}
pub fn bar(self) {}