auto merge of #7262 : nikomatsakis/rust/ref-bindings-in-irrefut-patterns, r=catamorphism

Correct treatment of irrefutable patterns. The old code was wrong in many, many ways. `ref` bindings didn't work, it sometimes copied when it should have moved, the borrow checker didn't even look at such patterns at all, we weren't consistent about preventing values with destructors from being pulled apart, etc.

Fixes #3224.
Fixes #3225.
Fixes #3255.
Fixes #6225.
Fixes #6386.

r? @catamorphism

Author: bors

src/libextra/fileinput.rs

@@ -418,8 +418,8 @@ mod test {
     fn make_file(path : &Path, contents: &[~str]) {
         let file = io::file_writer(path, [io::Create, io::Truncate]).get();
 
-        for contents.iter().advance |&str| {
-            file.write_str(str);
+        for contents.iter().advance |str| {
+            file.write_str(*str);
             file.write_char('\n');
         }
     }
@@ -445,7 +445,7 @@ mod test {
             |i| fmt!("tmp/lib-fileinput-test-fileinput-read-byte-%u.tmp", i)), true);
 
         // 3 files containing 0\n, 1\n, and 2\n respectively
-        for filenames.iter().enumerate().advance |(i, &filename)| {
+        for filenames.iter().enumerate().advance |(i, filename)| {
             make_file(filename.get_ref(), [fmt!("%u", i)]);
         }
 
@@ -475,7 +475,7 @@ mod test {
             |i| fmt!("tmp/lib-fileinput-test-fileinput-read-%u.tmp", i)), true);
 
         // 3 files containing 1\n, 2\n, and 3\n respectively
-        for filenames.iter().enumerate().advance |(i, &filename)| {
+        for filenames.iter().enumerate().advance |(i, filename)| {
             make_file(filename.get_ref(), [fmt!("%u", i)]);
         }
 
@@ -495,10 +495,11 @@ mod test {
             3,
             |i| fmt!("tmp/lib-fileinput-test-input-vec-%u.tmp", i)), true);
 
-        for filenames.iter().enumerate().advance |(i, &filename)| {
+        for filenames.iter().enumerate().advance |(i, filename)| {
             let contents =
                 vec::from_fn(3, |j| fmt!("%u %u", i, j));
             make_file(filename.get_ref(), contents);
+            debug!("contents=%?", contents);
             all_lines.push_all(contents);
         }
 
@@ -515,7 +516,7 @@ mod test {
             3,
             |i| fmt!("tmp/lib-fileinput-test-input-vec-state-%u.tmp", i)),true);
 
-        for filenames.iter().enumerate().advance |(i, &filename)| {
+        for filenames.iter().enumerate().advance |(i, filename)| {
             let contents =
                 vec::from_fn(3, |j| fmt!("%u %u", i, j + 1));
             make_file(filename.get_ref(), contents);
@@ -579,10 +580,10 @@ mod test {
             3,
             |i| fmt!("tmp/lib-fileinput-test-next-file-%u.tmp", i)),true);
 
-        for filenames.iter().enumerate().advance |(i, &filename)| {
+        for filenames.iter().enumerate().advance |(i, filename)| {
             let contents =
                 vec::from_fn(3, |j| fmt!("%u %u", i, j + 1));
-            make_file(&filename.get(), contents);
+            make_file(filename.get_ref(), contents);
         }
 
         let in = FileInput::from_vec(filenames);

src/libextra/num/bigint.rs

@@ -1571,10 +1571,10 @@ mod biguint_tests {
     fn test_to_str_radix() {
         let r = to_str_pairs();
         for r.iter().advance |num_pair| {
-            let &(n, rs) = num_pair;
+            let &(ref n, ref rs) = num_pair;
             for rs.iter().advance |str_pair| {
-                let &(radix, str) = str_pair;
-                assert_eq!(n.to_str_radix(radix), str);
+                let &(ref radix, ref str) = str_pair;
+                assert_eq!(&n.to_str_radix(*radix), str);
             }
         }
     }
@@ -1583,10 +1583,10 @@ mod biguint_tests {
     fn test_from_str_radix() {
         let r = to_str_pairs();
         for r.iter().advance |num_pair| {
-            let &(n, rs) = num_pair;
+            let &(ref n, ref rs) = num_pair;
             for rs.iter().advance |str_pair| {
-                let &(radix, str) = str_pair;
-                assert_eq!(&n, &FromStrRadix::from_str_radix(str, radix).get());
+                let &(ref radix, ref str) = str_pair;
+                assert_eq!(n, &FromStrRadix::from_str_radix(*str, *radix).get());
             }
         }
 

src/libextra/rc.rs

@@ -73,7 +73,7 @@ impl<T> Drop for Rc<T> {
             if self.ptr.is_not_null() {
                 (*self.ptr).count -= 1;
                 if (*self.ptr).count == 0 {
-                    ptr::replace_ptr(self.ptr, intrinsics::uninit());
+                    ptr::read_ptr(self.ptr);
                     free(self.ptr as *c_void)
                 }
             }

src/libextra/term.rs

@@ -119,8 +119,8 @@ impl Terminal {
     pub fn reset(&self) {
         let mut vars = Variables::new();
         let s = do self.ti.strings.find_equiv(&("op"))
-                       .map_consume_default(Err(~"can't find terminfo capability `op`")) |&op| {
-                           expand(op, [], &mut vars)
+                       .map_consume_default(Err(~"can't find terminfo capability `op`")) |op| {
+                           expand(copy *op, [], &mut vars)
                        };
         if s.is_ok() {
             self.out.write(s.unwrap());

src/libextra/terminfo/parm.rs

@@ -81,8 +81,8 @@ pub fn expand(cap: &[u8], params: &[Param], vars: &mut Variables)
 
     // Copy parameters into a local vector for mutability
     let mut mparams = [Number(0), ..9];
-    for mparams.mut_iter().zip(params.iter()).advance |(dst, &src)| {
-        *dst = src;
+    for mparams.mut_iter().zip(params.iter()).advance |(dst, src)| {
+        *dst = copy *src;
     }
 
     for cap.iter().transform(|&x| x).advance |c| {

src/libextra/treemap.rs

@@ -773,15 +773,15 @@ mod test_treemap {
                                             map: &TreeMap<K, V>) {
         assert_eq!(ctrl.is_empty(), map.is_empty());
         for ctrl.iter().advance |x| {
-            let &(k, v) = x;
-            assert!(map.find(&k).unwrap() == &v)
+            let &(ref k, ref v) = x;
+            assert!(map.find(k).unwrap() == v)
         }
         for map.iter().advance |(map_k, map_v)| {
             let mut found = false;
             for ctrl.iter().advance |x| {
-                let &(ctrl_k, ctrl_v) = x;
-                if *map_k == ctrl_k {
-                    assert!(*map_v == ctrl_v);
+                let &(ref ctrl_k, ref ctrl_v) = x;
+                if *map_k == *ctrl_k {
+                    assert!(*map_v == *ctrl_v);
                     found = true;
                     break;
                 }

src/libextra/workcache.rs

@@ -157,8 +157,8 @@ impl<D:Decoder> Decodable<D> for WorkMap {
     fn decode(d: &mut D) -> WorkMap {
         let v : ~[(WorkKey,~str)] = Decodable::decode(d);
         let mut w = WorkMap::new();
-        for v.iter().advance |&(k, v)| {
-            w.insert(copy k, copy v);
+        for v.iter().advance |pair| {
+            w.insert(pair.first(), pair.second());
         }
         w
     }

src/librustc/back/passes.rs

@@ -165,10 +165,10 @@ pub fn create_standard_passes(level: OptLevel) -> ~[~str] {
 }
 
 pub fn populate_pass_manager(sess: Session, pm: &mut PassManager, pass_list:&[~str]) {
-    for pass_list.iter().advance |&nm| {
-        match create_pass(nm) {
+    for pass_list.iter().advance |nm| {
+        match create_pass(*nm) {
             Some(p) => pm.add_pass(p),
-            None    => sess.warn(fmt!("Unknown pass %s", nm))
+            None    => sess.warn(fmt!("Unknown pass %s", *nm))
         }
     }
 }

src/librustc/driver/driver.rs

@@ -314,7 +314,6 @@ pub fn compile_rest(sess: Session,
             method_map: method_map,
             vtable_map: vtable_map,
             write_guard_map: write_guard_map,
-            moves_map: moves_map,
             capture_map: capture_map
         };
 

src/librustc/metadata/loader.rs

@@ -127,7 +127,9 @@ fn find_library_crate_aux(
             cx.diag.span_err(
                     cx.span, fmt!("multiple matching crates for `%s`", crate_name));
                 cx.diag.handler().note("candidates:");
-                for matches.iter().advance |&(ident, data)| {
+                for matches.iter().advance |pair| {
+                    let ident = pair.first();
+                    let data = pair.second();
                     cx.diag.handler().note(fmt!("path: %s", ident));
                     let attrs = decoder::get_crate_attributes(data);
                     note_linkage_attrs(cx.intr, cx.diag, attrs);

src/librustc/middle/astencode.rs

@@ -53,7 +53,6 @@ pub struct Maps {
     method_map: middle::typeck::method_map,
     vtable_map: middle::typeck::vtable_map,
     write_guard_map: middle::borrowck::write_guard_map,
-    moves_map: middle::moves::MovesMap,
     capture_map: middle::moves::CaptureMap,
 }
 
@@ -952,12 +951,6 @@ fn encode_side_tables_for_id(ecx: &e::EncodeContext,
         }
     }
 
-    if maps.moves_map.contains(&id) {
-        do ebml_w.tag(c::tag_table_moves_map) |ebml_w| {
-            ebml_w.id(id);
-        }
-    }
-
     {
         let r = maps.capture_map.find(&id);
         for r.iter().advance |&cap_vars| {
@@ -1121,9 +1114,7 @@ fn decode_side_tables(xcx: @ExtendedDecodeContext,
                 xcx.dcx.tcx.sess.bug(
                     fmt!("unknown tag found in side tables: %x", tag));
             }
-            Some(value) => if value == c::tag_table_moves_map {
-                dcx.maps.moves_map.insert(id);
-            } else {
+            Some(value) => {
                 let val_doc = entry_doc.get(c::tag_table_val as uint);
                 let mut val_dsr = reader::Decoder(val_doc);
                 let val_dsr = &mut val_dsr;

src/librustc/middle/borrowck/check_loans.rs

@@ -65,7 +65,7 @@ pub fn check_loans(bccx: @BorrowckCtxt,
 
 enum MoveError {
     MoveOk,
-    MoveWhileBorrowed(/*move*/@LoanPath, /*loan*/@LoanPath, /*loan*/span)
+    MoveWhileBorrowed(/*loan*/@LoanPath, /*loan*/span)
 }
 
 impl<'self> CheckLoanCtxt<'self> {
@@ -348,7 +348,7 @@ impl<'self> CheckLoanCtxt<'self> {
                         cmt = b;
                     }
 
-                    mc::cat_rvalue |
+                    mc::cat_rvalue(*) |
                     mc::cat_static_item |
                     mc::cat_implicit_self |
                     mc::cat_copied_upvar(*) |
@@ -547,45 +547,50 @@ impl<'self> CheckLoanCtxt<'self> {
                  self.bccx.loan_path_to_str(loan_path)));
     }
 
-    pub fn check_move_out_from_expr(&self, ex: @ast::expr) {
-        match ex.node {
-            ast::expr_paren(*) => {
-                /* In the case of an expr_paren(), the expression inside
-                 * the parens will also be marked as being moved.  Ignore
-                 * the parents then so as not to report duplicate errors. */
+    fn check_move_out_from_expr(&self, expr: @ast::expr) {
+        match expr.node {
+            ast::expr_fn_block(*) => {
+                // moves due to capture clauses are checked
+                // in `check_loans_in_fn`, so that we can
+                // give a better error message
             }
             _ => {
-                let cmt = self.bccx.cat_expr(ex);
-                match self.analyze_move_out_from_cmt(cmt) {
-                    MoveOk => {}
-                    MoveWhileBorrowed(move_path, loan_path, loan_span) => {
-                        self.bccx.span_err(
-                            cmt.span,
-                            fmt!("cannot move out of `%s` \
-                                  because it is borrowed",
-                                 self.bccx.loan_path_to_str(move_path)));
-                        self.bccx.span_note(
-                            loan_span,
-                            fmt!("borrow of `%s` occurs here",
-                                 self.bccx.loan_path_to_str(loan_path)));
-                    }
+                self.check_move_out_from_id(expr.id, expr.span)
+            }
+        }
+    }
+
+    fn check_move_out_from_id(&self, id: ast::node_id, span: span) {
+        for 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,
+                        fmt!("cannot move out of `%s` \
+                              because it is borrowed",
+                             self.bccx.loan_path_to_str(move_path)));
+                    self.bccx.span_note(
+                        loan_span,
+                        fmt!("borrow of `%s` occurs here",
+                             self.bccx.loan_path_to_str(loan_path)));
                 }
             }
         }
     }
 
-    pub fn analyze_move_out_from_cmt(&self, cmt: mc::cmt) -> MoveError {
-        debug!("analyze_move_out_from_cmt(cmt=%s)", cmt.repr(self.tcx()));
+    pub fn analyze_move_out_from(&self,
+                                 expr_id: ast::node_id,
+                                 move_path: @LoanPath) -> MoveError {
+        debug!("analyze_move_out_from(expr_id=%?, move_path=%s)",
+               expr_id, move_path.repr(self.tcx()));
 
         // FIXME(#4384) inadequare if/when we permit `move a.b`
 
         // check for a conflicting loan:
-        let r = opt_loan_path(cmt);
-        for r.iter().advance |&lp| {
-            for self.each_in_scope_restriction(cmt.id, lp) |loan, _| {
-                // Any restriction prevents moves.
-                return MoveWhileBorrowed(lp, loan.loan_path, loan.span);
-            }
+        for self.each_in_scope_restriction(expr_id, move_path) |loan, _| {
+            // Any restriction prevents moves.
+            return MoveWhileBorrowed(loan.loan_path, loan.span);
         }
 
         MoveOk
@@ -652,13 +657,11 @@ fn check_loans_in_fn<'a>(fk: &visit::fn_kind,
                                  closure_id: ast::node_id,
                                  cap_var: &moves::CaptureVar) {
             let var_id = ast_util::def_id_of_def(cap_var.def).node;
-            let ty = ty::node_id_to_type(this.tcx(), var_id);
-            let cmt = this.bccx.cat_def(closure_id, cap_var.span,
-                                        ty, cap_var.def);
-            let move_err = this.analyze_move_out_from_cmt(cmt);
+            let move_path = @LpVar(var_id);
+            let move_err = this.analyze_move_out_from(closure_id, move_path);
             match move_err {
                 MoveOk => {}
-                MoveWhileBorrowed(move_path, loan_path, loan_span) => {
+                MoveWhileBorrowed(loan_path, loan_span) => {
                     this.bccx.span_err(
                         cap_var.span,
                         fmt!("cannot move `%s` into closure \
@@ -689,10 +692,7 @@ fn check_loans_in_expr<'a>(expr: @ast::expr,
            expr.repr(this.tcx()));
 
     this.check_for_conflicting_loans(expr.id);
-
-    if this.bccx.moves_map.contains(&expr.id) {
-        this.check_move_out_from_expr(expr);
-    }
+    this.check_move_out_from_expr(expr);
 
     match expr.node {
       ast::expr_self |
@@ -742,18 +742,7 @@ fn check_loans_in_pat<'a>(pat: @ast::pat,
                                        visit::vt<@mut CheckLoanCtxt<'a>>))
 {
     this.check_for_conflicting_loans(pat.id);
-
-    // Note: moves out of pattern bindings are not checked by
-    // the borrow checker, at least not directly.  What happens
-    // is that if there are any moved bindings, the discriminant
-    // will be considered a move, and this will be checked as
-    // normal.  Then, in `middle::check_match`, we will check
-    // that no move occurs in a binding that is underneath an
-    // `@` or `&`.  Together these give the same guarantees as
-    // `check_move_out_from_expr()` without requiring us to
-    // rewalk the patterns and rebuild the pattern
-    // categorizations.
-
+    this.check_move_out_from_id(pat.id, pat.span);
     visit::visit_pat(pat, (this, vt));
 }