auto merge of #12733 : edwardw/rust/rw-liveness, r=nikomatsakis

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

Author: bors

Diff for: 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 for: 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 for: 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 for: 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<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>>,
 }
 
@@ -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 for: 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 for: 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 for: src/test/compile-fail/liveness-and-init.rs renamed to src/test/compile-fail/borrowck-and-init.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 for: src/test/compile-fail/liveness-block-unint.rs renamed to src/test/compile-fail/borrowck-block-unint.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`
+}

Diff for: src/test/compile-fail/liveness-break-uninit-2.rs renamed to src/test/compile-fail/borrowck-break-uninit-2.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 for: src/test/compile-fail/liveness-break-uninit.rs renamed to src/test/compile-fail/borrowck-break-uninit.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`