From abfde39b0e904cf39d3fbf392d747f4f168017ec Mon Sep 17 00:00:00 2001 From: Edward Wang Date: Thu, 6 Mar 2014 22:58:34 +0800 Subject: [PATCH] borrowck: classify expressions as assignees, uses or both - Repurposes `MoveData.assignee_ids` to mean only `=` but not `+=`, so that borrowck effectively classifies all expressions into assignees, uses or both. - Removes two `span_err` in liveness analysis, which are now borrowck's responsibilities. Closes #12527. --- .../borrowck/gather_loans/gather_moves.rs | 17 ++++- .../middle/borrowck/gather_loans/mod.rs | 54 ++++++++-------- src/librustc/middle/borrowck/mod.rs | 2 +- src/librustc/middle/borrowck/move_data.rs | 15 +++-- src/librustc/middle/liveness.rs | 62 +++---------------- src/test/compile-fail/asm-out-read-uninit.rs | 2 +- ...eness-and-init.rs => borrowck-and-init.rs} | 0 ...block-unint.rs => borrowck-block-unint.rs} | 4 +- ...uninit-2.rs => borrowck-break-uninit-2.rs} | 0 ...eak-uninit.rs => borrowck-break-uninit.rs} | 0 ...s-if-no-else.rs => borrowck-if-no-else.rs} | 0 ...-with-else.rs => borrowck-if-with-else.rs} | 0 ....rs => borrowck-init-in-called-fn-expr.rs} | 0 ...fn-expr.rs => borrowck-init-in-fn-expr.rs} | 0 ...init-in-fru.rs => borrowck-init-in-fru.rs} | 0 ...-op-equal.rs => borrowck-init-op-equal.rs} | 0 ...s-equal.rs => borrowck-init-plus-equal.rs} | 0 ...iveness-or-init.rs => borrowck-or-init.rs} | 0 ...{liveness-return.rs => borrowck-return.rs} | 0 ...-item.rs => borrowck-uninit-after-item.rs} | 0 .../borrowck-uninit-in-assignop.rs | 44 +++++++++++++ ...{liveness-uninit.rs => borrowck-uninit.rs} | 0 ...lue.rs => borrowck-use-in-index-lvalue.rs} | 4 ++ ...while-break.rs => borrowck-while-break.rs} | 0 ...s-while-cond.rs => borrowck-while-cond.rs} | 0 .../{liveness-while.rs => borrowck-while.rs} | 0 src/test/compile-fail/liveness-unused.rs | 5 ++ 27 files changed, 119 insertions(+), 90 deletions(-) rename src/test/compile-fail/{liveness-and-init.rs => borrowck-and-init.rs} (100%) rename src/test/compile-fail/{liveness-block-unint.rs => borrowck-block-unint.rs} (84%) rename src/test/compile-fail/{liveness-break-uninit-2.rs => borrowck-break-uninit-2.rs} (100%) rename src/test/compile-fail/{liveness-break-uninit.rs => borrowck-break-uninit.rs} (100%) rename src/test/compile-fail/{liveness-if-no-else.rs => borrowck-if-no-else.rs} (100%) rename src/test/compile-fail/{liveness-if-with-else.rs => borrowck-if-with-else.rs} (100%) rename src/test/compile-fail/{liveness-init-in-called-fn-expr.rs => borrowck-init-in-called-fn-expr.rs} (100%) rename src/test/compile-fail/{liveness-init-in-fn-expr.rs => borrowck-init-in-fn-expr.rs} (100%) rename src/test/compile-fail/{liveness-init-in-fru.rs => borrowck-init-in-fru.rs} (100%) rename src/test/compile-fail/{liveness-init-op-equal.rs => borrowck-init-op-equal.rs} (100%) rename src/test/compile-fail/{liveness-init-plus-equal.rs => borrowck-init-plus-equal.rs} (100%) rename src/test/compile-fail/{liveness-or-init.rs => borrowck-or-init.rs} (100%) rename src/test/compile-fail/{liveness-return.rs => borrowck-return.rs} (100%) rename src/test/compile-fail/{liveness-uninit-after-item.rs => borrowck-uninit-after-item.rs} (100%) create mode 100644 src/test/compile-fail/borrowck-uninit-in-assignop.rs rename src/test/compile-fail/{liveness-uninit.rs => borrowck-uninit.rs} (100%) rename src/test/compile-fail/{liveness-use-in-index-lvalue.rs => borrowck-use-in-index-lvalue.rs} (78%) rename src/test/compile-fail/{liveness-while-break.rs => borrowck-while-break.rs} (100%) rename src/test/compile-fail/{liveness-while-cond.rs => borrowck-while-cond.rs} (100%) rename src/test/compile-fail/{liveness-while.rs => borrowck-while.rs} (100%) diff --git a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs index 7ec9f1f8f4795..6dd7ae31c9d38 100644 --- a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs +++ b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs @@ -94,7 +94,22 @@ pub fn gather_assignment(bccx: &BorrowckCtxt, assignee_loan_path, assignment_id, assignment_span, - assignee_id); + assignee_id, + false); +} + +pub fn gather_move_and_assignment(bccx: &BorrowckCtxt, + move_data: &MoveData, + assignment_id: ast::NodeId, + assignment_span: Span, + assignee_loan_path: @LoanPath, + assignee_id: ast::NodeId) { + move_data.add_assignment(bccx.tcx, + assignee_loan_path, + assignment_id, + assignment_span, + assignee_id, + true); } fn check_is_legal_to_move_from(bccx: &BorrowckCtxt, diff --git a/src/librustc/middle/borrowck/gather_loans/mod.rs b/src/librustc/middle/borrowck/gather_loans/mod.rs index d6666eb629310..e50d6da378af1 100644 --- a/src/librustc/middle/borrowck/gather_loans/mod.rs +++ b/src/librustc/middle/borrowck/gather_loans/mod.rs @@ -214,20 +214,19 @@ fn gather_loans_in_expr(this: &mut GatherLoanCtxt, visit::walk_expr(this, ex, ()); } - ast::ExprAssign(l, _) | ast::ExprAssignOp(_, 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. - } - } + ast::ExprAssign(l, _) => { + with_assignee_loan_path( + this.bccx, l, + |lp| gather_moves::gather_assignment(this.bccx, &this.move_data, + ex.id, ex.span, lp, l.id)); + visit::walk_expr(this, ex, ()); + } + + ast::ExprAssignOp(_, l, _) => { + with_assignee_loan_path( + this.bccx, l, + |lp| gather_moves::gather_move_and_assignment(this.bccx, &this.move_data, + ex.id, ex.span, lp, l.id)); visit::walk_expr(this, ex, ()); } @@ -288,17 +287,10 @@ fn gather_loans_in_expr(this: &mut GatherLoanCtxt, ast::ExprInlineAsm(ref ia) => { for &(_, out) in ia.outputs.iter() { - let out_cmt = this.bccx.cat_expr(out); - match opt_loan_path(out_cmt) { - Some(out_lp) => { - gather_moves::gather_assignment(this.bccx, &this.move_data, - ex.id, ex.span, - out_lp, out.id); - } - None => { - // See the comment for ExprAssign. - } - } + with_assignee_loan_path( + this.bccx, out, + |lp| gather_moves::gather_assignment(this.bccx, &this.move_data, + ex.id, ex.span, lp, out.id)); } visit::walk_expr(this, ex, ()); } @@ -309,6 +301,18 @@ fn gather_loans_in_expr(this: &mut GatherLoanCtxt, } } +fn with_assignee_loan_path(bccx: &BorrowckCtxt, expr: &ast::Expr, op: |@LoanPath|) { + let cmt = bccx.cat_expr(expr); + match opt_loan_path(cmt) { + Some(lp) => op(lp), + 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. + } + } +} + impl<'a> GatherLoanCtxt<'a> { pub fn tcx(&self) -> ty::ctxt { self.bccx.tcx } diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs index 9767b50ae9603..654203bf0776b 100644 --- a/src/librustc/middle/borrowck/mod.rs +++ b/src/librustc/middle/borrowck/mod.rs @@ -535,7 +535,7 @@ impl BorrowckCtxt { move_data::Declared => { self.tcx.sess.span_err( use_span, - format!("{} of possibly uninitialized value: `{}`", + format!("{} of possibly uninitialized variable: `{}`", verb, self.loan_path_to_str(lp))); } diff --git a/src/librustc/middle/borrowck/move_data.rs b/src/librustc/middle/borrowck/move_data.rs index e1434a8ac4e97..5f4d5b43231c8 100644 --- a/src/librustc/middle/borrowck/move_data.rs +++ b/src/librustc/middle/borrowck/move_data.rs @@ -33,23 +33,25 @@ use util::ppaux::Repr; pub struct MoveData { /// Move paths. See section "Move paths" in `doc.rs`. - paths: RefCell >, + paths: RefCell>, /// Cache of loan path to move path index, for easy lookup. path_map: RefCell>, /// Each move or uninitialized variable gets an entry here. - moves: RefCell >, + moves: RefCell>, /// Assignments to a variable, like `x = foo`. These are assigned /// bits for dataflow, since we must track them to ensure that /// immutable variables are assigned at most once along each path. - var_assignments: RefCell >, + var_assignments: RefCell>, /// Assignments to a path, like `x.f = foo`. These are not /// assigned dataflow bits, but we track them because they still /// kill move bits. - path_assignments: RefCell >, + path_assignments: RefCell>, + + /// Assignments to a variable or path, like `x = foo`, but not `x += foo`. assignee_ids: RefCell>, } @@ -392,7 +394,8 @@ impl MoveData { lp: @LoanPath, assign_id: ast::NodeId, span: Span, - assignee_id: ast::NodeId) { + assignee_id: ast::NodeId, + is_also_move: bool) { /*! * Adds a new record for an assignment to `lp` that occurs at * location `id` with the given `span`. @@ -403,7 +406,7 @@ impl MoveData { let path_index = self.move_path(tcx, lp); - { + if !is_also_move { let mut assignee_ids = self.assignee_ids.borrow_mut(); assignee_ids.get().insert(assignee_id); } diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 04f2b2f282319..02a947a0ddc7a 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -1468,28 +1468,14 @@ impl Liveness { fn check_local(this: &mut Liveness, local: &Local) { match local.init { - Some(_) => { - this.warn_about_unused_or_dead_vars_in_pat(local.pat); - } - None => { - - // No initializer: the variable might be unused; if not, it - // should not be live at this point. - - debug!("check_local() with no initializer"); - this.pat_bindings(local.pat, |ln, var, sp, id| { - if !this.warn_about_unused(sp, id, ln, var) { - match this.live_on_exit(ln, var) { - None => { /* not live: good */ } - Some(lnk) => { - this.report_illegal_read( - local.span, lnk, var, - PossiblyUninitializedVariable); - } - } - } - }) - } + Some(_) => { + this.warn_about_unused_or_dead_vars_in_pat(local.pat); + }, + None => { + this.pat_bindings(local.pat, |ln, var, sp, id| { + this.warn_about_unused(sp, id, ln, var); + }) + } } visit::walk_local(this, local, ()); @@ -1644,38 +1630,6 @@ impl Liveness { } } - pub fn report_illegal_read(&self, - chk_span: Span, - lnk: LiveNodeKind, - var: Variable, - rk: ReadKind) { - let msg = match rk { - PossiblyUninitializedVariable => "possibly uninitialized \ - variable", - PossiblyUninitializedField => "possibly uninitialized field", - MovedValue => "moved value", - PartiallyMovedValue => "partially moved value" - }; - let name = self.ir.variable_name(var); - match lnk { - FreeVarNode(span) => { - self.tcx.sess.span_err( - span, - format!("capture of {}: `{}`", msg, name)); - } - ExprNode(span) => { - self.tcx.sess.span_err( - span, - format!("use of {}: `{}`", msg, name)); - } - ExitNode | VarDefNode(_) => { - self.tcx.sess.span_bug( - chk_span, - format!("illegal reader: {:?}", lnk)); - } - } - } - pub fn should_warn(&self, var: Variable) -> Option<~str> { let name = self.ir.variable_name(var); if name.len() == 0 || name[0] == ('_' as u8) { None } else { Some(name) } diff --git a/src/test/compile-fail/asm-out-read-uninit.rs b/src/test/compile-fail/asm-out-read-uninit.rs index b63865921c653..65750eb926b45 100644 --- a/src/test/compile-fail/asm-out-read-uninit.rs +++ b/src/test/compile-fail/asm-out-read-uninit.rs @@ -19,7 +19,7 @@ fn foo(x: int) { info!("{}", x); } pub fn main() { let x: int; unsafe { - asm!("mov $1, $0" : "=r"(x) : "r"(x)); //~ ERROR use of possibly uninitialized value: `x` + asm!("mov $1, $0" : "=r"(x) : "r"(x)); //~ ERROR use of possibly uninitialized variable: `x` } foo(x); } diff --git a/src/test/compile-fail/liveness-and-init.rs b/src/test/compile-fail/borrowck-and-init.rs similarity index 100% rename from src/test/compile-fail/liveness-and-init.rs rename to src/test/compile-fail/borrowck-and-init.rs diff --git a/src/test/compile-fail/liveness-block-unint.rs b/src/test/compile-fail/borrowck-block-unint.rs similarity index 84% rename from src/test/compile-fail/liveness-block-unint.rs rename to src/test/compile-fail/borrowck-block-unint.rs index c46b9013a068d..fc865e271e39c 100644 --- a/src/test/compile-fail/liveness-block-unint.rs +++ b/src/test/compile-fail/borrowck-block-unint.rs @@ -11,7 +11,7 @@ fn force(f: ||) { f(); } fn main() { let x: int; - force(|| { - info!("{}", x); //~ ERROR capture of possibly uninitialized variable: `x` + force(|| { //~ ERROR capture of possibly uninitialized variable: `x` + info!("{}", x); }); } diff --git a/src/test/compile-fail/liveness-break-uninit-2.rs b/src/test/compile-fail/borrowck-break-uninit-2.rs similarity index 100% rename from src/test/compile-fail/liveness-break-uninit-2.rs rename to src/test/compile-fail/borrowck-break-uninit-2.rs diff --git a/src/test/compile-fail/liveness-break-uninit.rs b/src/test/compile-fail/borrowck-break-uninit.rs similarity index 100% rename from src/test/compile-fail/liveness-break-uninit.rs rename to src/test/compile-fail/borrowck-break-uninit.rs diff --git a/src/test/compile-fail/liveness-if-no-else.rs b/src/test/compile-fail/borrowck-if-no-else.rs similarity index 100% rename from src/test/compile-fail/liveness-if-no-else.rs rename to src/test/compile-fail/borrowck-if-no-else.rs diff --git a/src/test/compile-fail/liveness-if-with-else.rs b/src/test/compile-fail/borrowck-if-with-else.rs similarity index 100% rename from src/test/compile-fail/liveness-if-with-else.rs rename to src/test/compile-fail/borrowck-if-with-else.rs diff --git a/src/test/compile-fail/liveness-init-in-called-fn-expr.rs b/src/test/compile-fail/borrowck-init-in-called-fn-expr.rs similarity index 100% rename from src/test/compile-fail/liveness-init-in-called-fn-expr.rs rename to src/test/compile-fail/borrowck-init-in-called-fn-expr.rs diff --git a/src/test/compile-fail/liveness-init-in-fn-expr.rs b/src/test/compile-fail/borrowck-init-in-fn-expr.rs similarity index 100% rename from src/test/compile-fail/liveness-init-in-fn-expr.rs rename to src/test/compile-fail/borrowck-init-in-fn-expr.rs diff --git a/src/test/compile-fail/liveness-init-in-fru.rs b/src/test/compile-fail/borrowck-init-in-fru.rs similarity index 100% rename from src/test/compile-fail/liveness-init-in-fru.rs rename to src/test/compile-fail/borrowck-init-in-fru.rs diff --git a/src/test/compile-fail/liveness-init-op-equal.rs b/src/test/compile-fail/borrowck-init-op-equal.rs similarity index 100% rename from src/test/compile-fail/liveness-init-op-equal.rs rename to src/test/compile-fail/borrowck-init-op-equal.rs diff --git a/src/test/compile-fail/liveness-init-plus-equal.rs b/src/test/compile-fail/borrowck-init-plus-equal.rs similarity index 100% rename from src/test/compile-fail/liveness-init-plus-equal.rs rename to src/test/compile-fail/borrowck-init-plus-equal.rs diff --git a/src/test/compile-fail/liveness-or-init.rs b/src/test/compile-fail/borrowck-or-init.rs similarity index 100% rename from src/test/compile-fail/liveness-or-init.rs rename to src/test/compile-fail/borrowck-or-init.rs diff --git a/src/test/compile-fail/liveness-return.rs b/src/test/compile-fail/borrowck-return.rs similarity index 100% rename from src/test/compile-fail/liveness-return.rs rename to src/test/compile-fail/borrowck-return.rs diff --git a/src/test/compile-fail/liveness-uninit-after-item.rs b/src/test/compile-fail/borrowck-uninit-after-item.rs similarity index 100% rename from src/test/compile-fail/liveness-uninit-after-item.rs rename to src/test/compile-fail/borrowck-uninit-after-item.rs diff --git a/src/test/compile-fail/borrowck-uninit-in-assignop.rs b/src/test/compile-fail/borrowck-uninit-in-assignop.rs new file mode 100644 index 0000000000000..b5e462e592a76 --- /dev/null +++ b/src/test/compile-fail/borrowck-uninit-in-assignop.rs @@ -0,0 +1,44 @@ +// Copyright 2012-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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Tests that the use of uninitialized variable in assignment operator +// expression is detected. + +pub fn main() { + let x: int; + x += 1; //~ ERROR use of possibly uninitialized variable: `x` + + let x: int; + x -= 1; //~ ERROR use of possibly uninitialized variable: `x` + + let x: int; + x *= 1; //~ ERROR use of possibly uninitialized variable: `x` + + let x: int; + x /= 1; //~ ERROR use of possibly uninitialized variable: `x` + + let x: int; + x %= 1; //~ ERROR use of possibly uninitialized variable: `x` + + let x: int; + x ^= 1; //~ ERROR use of possibly uninitialized variable: `x` + + let x: int; + x &= 1; //~ ERROR use of possibly uninitialized variable: `x` + + let x: int; + x |= 1; //~ ERROR use of possibly uninitialized variable: `x` + + let x: int; + x <<= 1; //~ ERROR use of possibly uninitialized variable: `x` + + let x: int; + x >>= 1; //~ ERROR use of possibly uninitialized variable: `x` +} diff --git a/src/test/compile-fail/liveness-uninit.rs b/src/test/compile-fail/borrowck-uninit.rs similarity index 100% rename from src/test/compile-fail/liveness-uninit.rs rename to src/test/compile-fail/borrowck-uninit.rs diff --git a/src/test/compile-fail/liveness-use-in-index-lvalue.rs b/src/test/compile-fail/borrowck-use-in-index-lvalue.rs similarity index 78% rename from src/test/compile-fail/liveness-use-in-index-lvalue.rs rename to src/test/compile-fail/borrowck-use-in-index-lvalue.rs index 7ec0607fc971e..3ced98592400e 100644 --- a/src/test/compile-fail/liveness-use-in-index-lvalue.rs +++ b/src/test/compile-fail/borrowck-use-in-index-lvalue.rs @@ -10,6 +10,10 @@ fn test() { let w: ~[int]; + w[5] = 0; //~ ERROR use of possibly uninitialized variable: `w` + //~^ ERROR cannot assign to immutable vec content `w[..]` + + let mut w: ~[int]; w[5] = 0; //~ ERROR use of possibly uninitialized variable: `w` } diff --git a/src/test/compile-fail/liveness-while-break.rs b/src/test/compile-fail/borrowck-while-break.rs similarity index 100% rename from src/test/compile-fail/liveness-while-break.rs rename to src/test/compile-fail/borrowck-while-break.rs diff --git a/src/test/compile-fail/liveness-while-cond.rs b/src/test/compile-fail/borrowck-while-cond.rs similarity index 100% rename from src/test/compile-fail/liveness-while-cond.rs rename to src/test/compile-fail/borrowck-while-cond.rs diff --git a/src/test/compile-fail/liveness-while.rs b/src/test/compile-fail/borrowck-while.rs similarity index 100% rename from src/test/compile-fail/liveness-while.rs rename to src/test/compile-fail/borrowck-while.rs diff --git a/src/test/compile-fail/liveness-unused.rs b/src/test/compile-fail/liveness-unused.rs index 83c911d916e1e..33fc094abbe75 100644 --- a/src/test/compile-fail/liveness-unused.rs +++ b/src/test/compile-fail/liveness-unused.rs @@ -23,6 +23,11 @@ fn f1b(x: &mut int) { #[allow(unused_variable)] fn f1c(x: int) {} +fn f1d() { + let x: int; + //~^ ERROR unused variable: `x` +} + fn f2() { let x = 3; //~^ ERROR unused variable: `x`