From a9c3841a501691f36f98c47402d679885989637f Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 31 Jan 2015 20:58:26 -0500 Subject: [PATCH] Fix handling of `move` closures -- since they have one fewer deref, we weren't properly adjusting the closure kind in that case. --- src/librustc_typeck/check/upvar.rs | 98 ++++++++++++------- ...d-closures-infer-fnmut-move-missing-mut.rs | 18 ++++ ...d-closures-infer-fnonce-move-call-twice.rs | 21 ++++ .../unboxed-closures-infer-fnmut-move.rs | 25 +++++ .../unboxed-closures-infer-fnonce-move.rs | 37 +++++++ .../run-pass/unboxed-closures-infer-fnonce.rs | 2 +- 6 files changed, 167 insertions(+), 34 deletions(-) create mode 100644 src/test/compile-fail/unboxed-closures-infer-fnmut-move-missing-mut.rs create mode 100644 src/test/compile-fail/unboxed-closures-infer-fnonce-move-call-twice.rs create mode 100644 src/test/run-pass/unboxed-closures-infer-fnmut-move.rs create mode 100644 src/test/run-pass/unboxed-closures-infer-fnonce-move.rs diff --git a/src/librustc_typeck/check/upvar.rs b/src/librustc_typeck/check/upvar.rs index 961a2e39dc0..f7babadd41f 100644 --- a/src/librustc_typeck/check/upvar.rs +++ b/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. diff --git a/src/test/compile-fail/unboxed-closures-infer-fnmut-move-missing-mut.rs b/src/test/compile-fail/unboxed-closures-infer-fnmut-move-missing-mut.rs new file mode 100644 index 00000000000..de17d25b4c3 --- /dev/null +++ b/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 or the MIT license +// , 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 +} diff --git a/src/test/compile-fail/unboxed-closures-infer-fnonce-move-call-twice.rs b/src/test/compile-fail/unboxed-closures-infer-fnonce-move-call-twice.rs new file mode 100644 index 00000000000..f9709b8c596 --- /dev/null +++ b/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 or the MIT license +// , 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 = Vec::new(); + let tick = move || mem::drop(counter); + tick(); + tick(); //~ ERROR use of moved value: `tick` +} diff --git a/src/test/run-pass/unboxed-closures-infer-fnmut-move.rs b/src/test/run-pass/unboxed-closures-infer-fnmut-move.rs new file mode 100644 index 00000000000..794527249bf --- /dev/null +++ b/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 or the MIT license +// , 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); +} diff --git a/src/test/run-pass/unboxed-closures-infer-fnonce-move.rs b/src/test/run-pass/unboxed-closures-infer-fnonce-move.rs new file mode 100644 index 00000000000..9f8fc80819b --- /dev/null +++ b/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 or the MIT license +// , 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); +} diff --git a/src/test/run-pass/unboxed-closures-infer-fnonce.rs b/src/test/run-pass/unboxed-closures-infer-fnonce.rs index 69beae77184..f0f10139c5b 100644 --- a/src/test/run-pass/unboxed-closures-infer-fnonce.rs +++ b/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;