Fix handling of move closures -- since they have one fewer deref, we weren't properly adjusting the closure kind in that case.

Author: nikomatsakis

src/librustc_typeck/check/upvar.rs

@@ -263,16 +263,29 @@ impl<'a,'tcx> AdjustBorrowKind<'a,'tcx> {
         match guarantor.cat {
             mc::cat_deref(_, _, mc::BorrowedPtr(..)) |
             mc::cat_deref(_, _, mc::Implicit(..)) => {
-                if let mc::NoteUpvarRef(upvar_id) = cmt.note {
-                    debug!("adjust_upvar_borrow_kind_for_consume: \
-                            setting upvar_id={:?} to by value",
-                           upvar_id);
+                match cmt.note {
+                    mc::NoteUpvarRef(upvar_id) => {
+                        debug!("adjust_upvar_borrow_kind_for_consume: \
+                                setting upvar_id={:?} to by value",
+                               upvar_id);
 
-                    // to move out of an upvar, this must be a FnOnce closure
-                    self.adjust_closure_kind(upvar_id.closure_expr_id, ty::FnOnceClosureKind);
+                        // to move out of an upvar, this must be a FnOnce closure
+                        self.adjust_closure_kind(upvar_id.closure_expr_id, ty::FnOnceClosureKind);
 
-                    let mut upvar_capture_map = self.fcx.inh.upvar_capture_map.borrow_mut();
-                    upvar_capture_map.insert(upvar_id, ty::UpvarCapture::ByValue);
+                        let mut upvar_capture_map = self.fcx.inh.upvar_capture_map.borrow_mut();
+                        upvar_capture_map.insert(upvar_id, ty::UpvarCapture::ByValue);
+                    }
+                    mc::NoteClosureEnv(upvar_id) => {
+                        // we get just a closureenv ref if this is a
+                        // `move` closure, or if the upvar has already
+                        // been inferred to by-value. In any case, we
+                        // must still adjust the kind of the closure
+                        // to be a FnOnce closure to permit moves out
+                        // of the environment.
+                        self.adjust_closure_kind(upvar_id.closure_expr_id, ty::FnOnceClosureKind);
+                    }
+                    mc::NoteNone => {
+                    }
                 }
             }
             _ => { }
@@ -297,15 +310,7 @@ impl<'a,'tcx> AdjustBorrowKind<'a,'tcx> {
 
             mc::cat_deref(base, _, mc::BorrowedPtr(..)) |
             mc::cat_deref(base, _, mc::Implicit(..)) => {
-                if let mc::NoteUpvarRef(upvar_id) = cmt.note {
-                    // if this is an implicit deref of an
-                    // upvar, then we need to modify the
-                    // borrow_kind of the upvar to make sure it
-                    // is inferred to mutable if necessary
-                    let mut upvar_capture_map = self.fcx.inh.upvar_capture_map.borrow_mut();
-                    let ub = &mut upvar_capture_map[upvar_id];
-                    self.adjust_upvar_borrow_kind(upvar_id, ub, ty::MutBorrow);
-                } else {
+                if !self.try_adjust_upvar_deref(&cmt.note, ty::MutBorrow) {
                     // assignment to deref of an `&mut`
                     // borrowed pointer implies that the
                     // pointer itself must be unique, but not
@@ -339,15 +344,7 @@ impl<'a,'tcx> AdjustBorrowKind<'a,'tcx> {
 
             mc::cat_deref(base, _, mc::BorrowedPtr(..)) |
             mc::cat_deref(base, _, mc::Implicit(..)) => {
-                if let mc::NoteUpvarRef(upvar_id) = cmt.note {
-                    // if this is an implicit deref of an
-                    // upvar, then we need to modify the
-                    // borrow_kind of the upvar to make sure it
-                    // is inferred to unique if necessary
-                    let mut ub = self.fcx.inh.upvar_capture_map.borrow_mut();
-                    let ub = &mut ub[upvar_id];
-                    self.adjust_upvar_borrow_kind(upvar_id, ub, ty::UniqueImmBorrow);
-                } else {
+                if !self.try_adjust_upvar_deref(&cmt.note, ty::UniqueImmBorrow) {
                     // for a borrowed pointer to be unique, its
                     // base must be unique
                     self.adjust_upvar_borrow_kind_for_unique(base);
@@ -363,6 +360,48 @@ impl<'a,'tcx> AdjustBorrowKind<'a,'tcx> {
         }
     }
 
+    fn try_adjust_upvar_deref(&self,
+                              note: &mc::Note,
+                              borrow_kind: ty::BorrowKind)
+                              -> bool
+    {
+        assert!(match borrow_kind {
+            ty::MutBorrow => true,
+            ty::UniqueImmBorrow => true,
+
+            // imm borrows never require adjusting any kinds, so we don't wind up here
+            ty::ImmBorrow => false,
+        });
+
+        match *note {
+            mc::NoteUpvarRef(upvar_id) => {
+                // if this is an implicit deref of an
+                // upvar, then we need to modify the
+                // borrow_kind of the upvar to make sure it
+                // is inferred to mutable if necessary
+                let mut upvar_capture_map = self.fcx.inh.upvar_capture_map.borrow_mut();
+                let ub = &mut upvar_capture_map[upvar_id];
+                self.adjust_upvar_borrow_kind(upvar_id, ub, borrow_kind);
+
+                // also need to be in an FnMut closure since this is not an ImmBorrow
+                self.adjust_closure_kind(upvar_id.closure_expr_id, ty::FnMutClosureKind);
+
+                true
+            }
+            mc::NoteClosureEnv(upvar_id) => {
+                // this kind of deref occurs in a `move` closure, or
+                // for a by-value upvar; in either case, to mutate an
+                // upvar, we need to be an FnMut closure
+                self.adjust_closure_kind(upvar_id.closure_expr_id, ty::FnMutClosureKind);
+
+                true
+            }
+            mc::NoteNone => {
+                false
+            }
+        }
+    }
+
     /// We infer the borrow_kind with which to borrow upvars in a stack closure. The borrow_kind
     /// basically follows a lattice of `imm < unique-imm < mut`, moving from left to right as needed
     /// (but never right to left). Here the argument `mutbl` is the borrow_kind that is required by
@@ -374,13 +413,6 @@ impl<'a,'tcx> AdjustBorrowKind<'a,'tcx> {
         debug!("adjust_upvar_borrow_kind(upvar_id={:?}, upvar_capture={:?}, kind={:?})",
                upvar_id, upvar_capture, kind);
 
-        match kind {
-            ty::ImmBorrow => { }
-            ty::UniqueImmBorrow | ty::MutBorrow => {
-                self.adjust_closure_kind(upvar_id.closure_expr_id, ty::FnMutClosureKind);
-            }
-        }
-
         match *upvar_capture {
             ty::UpvarCapture::ByValue => {
                 // Upvar is already by-value, the strongest criteria.

src/test/compile-fail/unboxed-closures-infer-fnmut-move-missing-mut.rs

@@ -0,0 +1,18 @@
+// Copyright 2015 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.
+
+// Test that we are able to infer a suitable kind for this closure
+// that is just called (`FnMut`).
+
+fn main() {
+    let mut counter = 0;
+    let tick = move || counter += 1;
+    tick(); //~ ERROR cannot borrow immutable local variable `tick` as mutable
+}

src/test/compile-fail/unboxed-closures-infer-fnonce-move-call-twice.rs

@@ -0,0 +1,21 @@
+// Copyright 2015 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.
+
+// Test that we are able to infer a suitable kind for this closure
+// that is just called (`FnMut`).
+
+use std::mem;
+
+fn main() {
+    let mut counter: Vec<i32> = Vec::new();
+    let tick = move || mem::drop(counter);
+    tick();
+    tick(); //~ ERROR use of moved value: `tick`
+}

src/test/run-pass/unboxed-closures-infer-fnmut-move.rs

@@ -0,0 +1,25 @@
+// Copyright 2015 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.
+
+// Test that we are able to infer a suitable kind for this `move`
+// closure that is just called (`FnMut`).
+
+fn main() {
+    let mut counter = 0;
+
+    let v = {
+        let mut tick = move || { counter += 1; counter };
+        tick();
+        tick()
+    };
+
+    assert_eq!(counter, 0);
+    assert_eq!(v, 2);
+}

src/test/run-pass/unboxed-closures-infer-fnonce-move.rs

@@ -0,0 +1,37 @@
+// Copyright 2015 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.
+
+#![feature(unsafe_destructor)]
+
+// Test that we are able to infer a suitable kind for this `move`
+// closure that is just called (`FnOnce`).
+
+use std::mem;
+
+struct DropMe<'a>(&'a mut i32);
+
+#[unsafe_destructor]
+impl<'a> Drop for DropMe<'a> {
+    fn drop(&mut self) {
+        *self.0 += 1;
+    }
+}
+
+fn main() {
+    let mut counter = 0;
+
+    {
+        let drop_me = DropMe(&mut counter);
+        let tick = move || mem::drop(drop_me);
+        tick();
+    }
+
+    assert_eq!(counter, 1);
+}

src/test/run-pass/unboxed-closures-infer-fnonce.rs

@@ -11,7 +11,7 @@
 #![feature(unsafe_destructor)]
 
 // Test that we are able to infer a suitable kind for this closure
-// that is just called (`FnMut`).
+// that is just called (`FnOnce`).
 
 use std::mem;