Skip to content

Commit

Permalink
Fix handling of move closures -- since they have one fewer deref, w…
Browse files Browse the repository at this point in the history
…e weren't properly adjusting the closure kind in that case.
  • Loading branch information
nikomatsakis committed Feb 1, 2015
1 parent e778ea4 commit a9c3841
Show file tree
Hide file tree
Showing 6 changed files with 167 additions and 34 deletions.
98 changes: 65 additions & 33 deletions src/librustc_typeck/check/upvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
}
}
}
_ => { }
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
@@ -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`
}
25 changes: 25 additions & 0 deletions src/test/run-pass/unboxed-closures-infer-fnmut-move.rs
Original file line number Diff line number Diff line change
@@ -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);
}
37 changes: 37 additions & 0 deletions src/test/run-pass/unboxed-closures-infer-fnonce-move.rs
Original file line number Diff line number Diff line change
@@ -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);
}
2 changes: 1 addition & 1 deletion src/test/run-pass/unboxed-closures-infer-fnonce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit a9c3841

Please sign in to comment.