Skip to content

Commit

Permalink
borrowck: classify expressions as assignees, uses or both
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
edwardw committed Mar 9, 2014
1 parent 62f1d68 commit abfde39
Show file tree
Hide file tree
Showing 27 changed files with 119 additions and 90 deletions.
17 changes: 16 additions & 1 deletion src/librustc/middle/borrowck/gather_loans/gather_moves.rs
Expand Up @@ -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,
Expand Down
54 changes: 29 additions & 25 deletions src/librustc/middle/borrowck/gather_loans/mod.rs
Expand Up @@ -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, ());
}

Expand Down Expand Up @@ -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, ());
}
Expand All @@ -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 }

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/borrowck/mod.rs
Expand Up @@ -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)));
}
Expand Down
15 changes: 9 additions & 6 deletions src/librustc/middle/borrowck/move_data.rs
Expand Up @@ -33,23 +33,25 @@ use util::ppaux::Repr;

pub struct MoveData {
/// Move paths. See section "Move paths" in `doc.rs`.
paths: RefCell<Vec<MovePath> >,
paths: RefCell<Vec<MovePath>>,

/// Cache of loan path to move path index, for easy lookup.
path_map: RefCell<HashMap<@LoanPath, MovePathIndex>>,

/// Each move or uninitialized variable gets an entry here.
moves: RefCell<Vec<Move> >,
moves: RefCell<Vec<Move>>,

/// 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<Vec<Assignment> >,
var_assignments: RefCell<Vec<Assignment>>,

/// 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<Vec<Assignment> >,
path_assignments: RefCell<Vec<Assignment>>,

/// Assignments to a variable or path, like `x = foo`, but not `x += foo`.
assignee_ids: RefCell<HashSet<ast::NodeId>>,
}

Expand Down Expand Up @@ -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`.
Expand All @@ -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);
}
Expand Down
62 changes: 8 additions & 54 deletions src/librustc/middle/liveness.rs
Expand Up @@ -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, ());
Expand Down Expand Up @@ -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) }
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/asm-out-read-uninit.rs
Expand Up @@ -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);
}
Expand Down
File renamed without changes.
Expand Up @@ -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);
});
}
File renamed without changes.
File renamed without changes.
44 changes: 44 additions & 0 deletions 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 <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.

// 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`
}
File renamed without changes.
Expand Up @@ -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`
}

Expand Down
File renamed without changes.
5 changes: 5 additions & 0 deletions src/test/compile-fail/liveness-unused.rs
Expand Up @@ -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`
Expand Down

9 comments on commit abfde39

@bors
Copy link
Contributor

@bors bors commented on abfde39 Mar 9, 2014

Choose a reason for hiding this comment

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

saw approval from nikomatsakis
at edwardw@abfde39

@bors
Copy link
Contributor

@bors bors commented on abfde39 Mar 9, 2014

Choose a reason for hiding this comment

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

merging edwardw/rust/rw-liveness = abfde39 into auto

@bors
Copy link
Contributor

@bors bors commented on abfde39 Mar 9, 2014

Choose a reason for hiding this comment

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

edwardw/rust/rw-liveness = abfde39 merged ok, testing candidate = 41e880e

@bors
Copy link
Contributor

@bors bors commented on abfde39 Mar 10, 2014

Choose a reason for hiding this comment

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

@bors
Copy link
Contributor

@bors bors commented on abfde39 Mar 10, 2014

Choose a reason for hiding this comment

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

saw approval from nikomatsakis
at edwardw@abfde39

@bors
Copy link
Contributor

@bors bors commented on abfde39 Mar 10, 2014

Choose a reason for hiding this comment

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

merging edwardw/rust/rw-liveness = abfde39 into auto

@bors
Copy link
Contributor

@bors bors commented on abfde39 Mar 10, 2014

Choose a reason for hiding this comment

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

edwardw/rust/rw-liveness = abfde39 merged ok, testing candidate = cad7d24

@bors
Copy link
Contributor

@bors bors commented on abfde39 Mar 10, 2014

Choose a reason for hiding this comment

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

@bors
Copy link
Contributor

@bors bors commented on abfde39 Mar 10, 2014

Choose a reason for hiding this comment

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

fast-forwarding master to auto = cad7d24

Please sign in to comment.