diff --git a/src/librustc_typeck/check/callee.rs b/src/librustc_typeck/check/callee.rs index d7a28f1843c..93c6445606e 100644 --- a/src/librustc_typeck/check/callee.rs +++ b/src/librustc_typeck/check/callee.rs @@ -14,7 +14,7 @@ use super::check_argument_types; use super::check_expr; use super::check_method_argument_types; use super::demand; -use super::DeferredResolution; +use super::DeferredCallResolution; use super::err_args; use super::Expectation; use super::expected_types_for_fn_args; @@ -99,8 +99,8 @@ pub fn check_call<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, confirm_builtin_call(fcx, call_expr, callee_ty, arg_exprs, expected); } - Some(CallStep::Closure(fn_sig)) => { - confirm_closure_call(fcx, call_expr, arg_exprs, expected, fn_sig); + Some(CallStep::DeferredClosure(fn_sig)) => { + confirm_deferred_closure_call(fcx, call_expr, arg_exprs, expected, fn_sig); } Some(CallStep::Overloaded(method_callee)) => { @@ -112,7 +112,7 @@ pub fn check_call<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, enum CallStep<'tcx> { Builtin, - Closure(ty::FnSig<'tcx>), + DeferredClosure(ty::FnSig<'tcx>), Overloaded(ty::MethodCallee<'tcx>) } @@ -138,21 +138,28 @@ fn try_overloaded_call_step<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, } ty::ty_closure(def_id, _, substs) => { - let closure_ty = - fcx.closure_type(def_id, substs); - let fn_sig = - fcx.infcx().replace_late_bound_regions_with_fresh_var(call_expr.span, - infer::FnCall, - &closure_ty.sig).0; - fcx.record_deferred_resolution(box CallResolution { - call_expr: call_expr, - callee_expr: callee_expr, - adjusted_ty: adjusted_ty, - autoderefref: autoderefref, - fn_sig: fn_sig.clone(), - closure_def_id: def_id, - }); - return Some(CallStep::Closure(fn_sig)); + assert_eq!(def_id.krate, ast::LOCAL_CRATE); + + // Check whether this is a call to a closure where we + // haven't yet decided on whether the closure is fn vs + // fnmut vs fnonce. If so, we have to defer further processing. + if fcx.closure_kind(def_id).is_none() { + let closure_ty = + fcx.closure_type(def_id, substs); + let fn_sig = + fcx.infcx().replace_late_bound_regions_with_fresh_var(call_expr.span, + infer::FnCall, + &closure_ty.sig).0; + fcx.record_deferred_call_resolution( + def_id, + box CallResolution {call_expr: call_expr, + callee_expr: callee_expr, + adjusted_ty: adjusted_ty, + autoderefref: autoderefref, + fn_sig: fn_sig.clone(), + closure_def_id: def_id}); + return Some(CallStep::DeferredClosure(fn_sig)); + } } _ => {} @@ -258,11 +265,11 @@ fn confirm_builtin_call<'a,'tcx>(fcx: &FnCtxt<'a,'tcx>, write_call(fcx, call_expr, fn_sig.output); } -fn confirm_closure_call<'a,'tcx>(fcx: &FnCtxt<'a,'tcx>, - call_expr: &ast::Expr, - arg_exprs: &'tcx [P], - expected: Expectation<'tcx>, - fn_sig: ty::FnSig<'tcx>) +fn confirm_deferred_closure_call<'a,'tcx>(fcx: &FnCtxt<'a,'tcx>, + call_expr: &ast::Expr, + arg_exprs: &'tcx [P], + expected: Expectation<'tcx>, + fn_sig: ty::FnSig<'tcx>) { // `fn_sig` is the *signature* of the cosure being called. We // don't know the full details yet (`Fn` vs `FnMut` etc), but we @@ -338,22 +345,18 @@ impl<'tcx> Repr<'tcx> for CallResolution<'tcx> { } } -impl<'tcx> DeferredResolution<'tcx> for CallResolution<'tcx> { - fn attempt_resolution<'a>(&self, fcx: &FnCtxt<'a,'tcx>) -> bool { - debug!("attempt_resolution() {}", +impl<'tcx> DeferredCallResolution<'tcx> for CallResolution<'tcx> { + fn resolve<'a>(&mut self, fcx: &FnCtxt<'a,'tcx>) { + debug!("DeferredCallResolution::resolve() {}", self.repr(fcx.tcx())); - match fcx.closure_kind(self.closure_def_id) { - Some(_) => { } - None => { - return false; - } - } + // we should not be invoked until the closure kind has been + // determined by upvar inference + assert!(fcx.closure_kind(self.closure_def_id).is_some()); // We may now know enough to figure out fn vs fnmut etc. match try_overloaded_call_traits(fcx, self.call_expr, self.callee_expr, self.adjusted_ty, self.autoderefref.clone()) { - None => false, Some(method_callee) => { // One problem is that when we get here, we are going // to have a newly instantiated function signature @@ -382,8 +385,11 @@ impl<'tcx> DeferredResolution<'tcx> for CallResolution<'tcx> { self.fn_sig.output.unwrap()); write_overloaded_call_method_map(fcx, self.call_expr, method_callee); - - true + } + None => { + fcx.tcx().sess.span_bug( + self.call_expr.span, + "failed to find an overloaded call trait for closure call"); } } } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index ed195035a41..0aad1f99ce8 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -110,6 +110,7 @@ use util::nodemap::{DefIdMap, FnvHashMap, NodeMap}; use util::lev_distance::lev_distance; use std::cell::{Cell, Ref, RefCell}; +use std::collections::hash_map::Entry::{Occupied, Vacant}; use std::mem::replace; use std::rc::Rc; use std::iter::repeat; @@ -172,15 +173,21 @@ pub struct Inherited<'a, 'tcx: 'a> { // Tracks trait obligations incurred during this function body. fulfillment_cx: RefCell>, - // - deferred_resolutions: RefCell>>, + // When we process a call like `c()` where `c` is a closure type, + // we may not have decided yet whether `c` is a `Fn`, `FnMut`, or + // `FnOnce` closure. In that case, we defer full resolution of the + // call until upvar inference can kick in and make the + // decision. We keep these deferred resolutions sorted by the + // def-id of the closure, so that once we decide, we can easily go + // back and process them. + deferred_call_resolutions: RefCell>>>, } -trait DeferredResolution<'tcx> { - fn attempt_resolution<'a>(&self, fcx: &FnCtxt<'a,'tcx>) -> bool; +trait DeferredCallResolution<'tcx> { + fn resolve<'a>(&mut self, fcx: &FnCtxt<'a,'tcx>); } -type DeferredResolutionHandler<'tcx> = Box+'tcx>; +type DeferredCallResolutionHandler<'tcx> = Box+'tcx>; /// When type-checking an expression, we propagate downward /// whatever type hint we are able in the form of an `Expectation`. @@ -391,7 +398,7 @@ impl<'a, 'tcx> Inherited<'a, 'tcx> { closure_kinds: RefCell::new(DefIdMap()), fn_sig_map: RefCell::new(NodeMap()), fulfillment_cx: RefCell::new(traits::FulfillmentContext::new()), - deferred_resolutions: RefCell::new(Vec::new()), + deferred_call_resolutions: RefCell::new(DefIdMap()), } } @@ -1295,8 +1302,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if ty::type_has_ty_infer(t) || ty::type_is_error(t) { Err(()) } else { Ok(t) } } - fn record_deferred_resolution(&self, r: DeferredResolutionHandler<'tcx>) { - self.inh.deferred_resolutions.borrow_mut().push(r); + fn record_deferred_call_resolution(&self, + closure_def_id: ast::DefId, + r: DeferredCallResolutionHandler<'tcx>) { + let mut deferred_call_resolutions = self.inh.deferred_call_resolutions.borrow_mut(); + let mut vec = match deferred_call_resolutions.entry(closure_def_id) { + Occupied(entry) => entry.into_mut(), + Vacant(entry) => entry.insert(Vec::new()), + }; + vec.push(r); + } + + fn remove_deferred_call_resolutions(&self, + closure_def_id: ast::DefId) + -> Vec> + { + let mut deferred_call_resolutions = self.inh.deferred_call_resolutions.borrow_mut(); + deferred_call_resolutions.remove(&closure_def_id).unwrap_or(Vec::new()) } pub fn tag(&self) -> String { diff --git a/src/librustc_typeck/check/upvar.rs b/src/librustc_typeck/check/upvar.rs index 2b043076e46..961a2e39dc0 100644 --- a/src/librustc_typeck/check/upvar.rs +++ b/src/librustc_typeck/check/upvar.rs @@ -67,6 +67,9 @@ pub fn closure_analyze_fn(fcx: &FnCtxt, let mut adjust = AdjustBorrowKind::new(fcx, &closures_with_inferred_kinds); adjust.visit_block(body); + + // it's our job to process these. + assert!(fcx.inh.deferred_call_resolutions.borrow().is_empty()); } /////////////////////////////////////////////////////////////////////////// @@ -186,10 +189,56 @@ impl<'a,'tcx> AdjustBorrowKind<'a,'tcx> { self.visit_block(body); - debug!("analyzing fn body with id {}", body.id); + debug!("analyzing closure `{}` with fn body id `{}`", id, body.id); let mut euv = euv::ExprUseVisitor::new(self, self.fcx); euv.walk_fn(decl, body); + + // If we had not yet settled on a closure kind for this closure, + // then we should have by now. Process and remove any deferred resolutions. + // + // Interesting fact: all calls to this closure must come + // *after* its definition. Initially, I thought that some + // kind of fixed-point iteration would be required, due to the + // possibility of twisted examples like this one: + // + // ```rust + // let mut closure0 = None; + // let vec = vec!(1, 2, 3); + // + // loop { + // { + // let closure1 = || { + // match closure0.take() { + // Some(c) => { + // return c(); // (*) call to `closure0` before it is defined + // } + // None => { } + // } + // }; + // closure1(); + // } + // + // closure0 = || vec; + // } + // ``` + // + // However, this turns out to be wrong. Examples like this + // fail to compile because the type of the variable `c` above + // is an inference variable. And in fact since closure types + // cannot be written, there is no way to make this example + // work without a boxed closure. This implies that we can't + // have two closures that recursively call one another without + // some form of boxing (and hence explicit writing of a + // closure kind) involved. Huzzah. -nmatsakis + let closure_def_id = ast_util::local_def(id); + if self.closures_with_inferred_kinds.contains(&id) { + let mut deferred_call_resolutions = + self.fcx.remove_deferred_call_resolutions(closure_def_id); + for deferred_call_resolution in deferred_call_resolutions.iter_mut() { + deferred_call_resolution.resolve(self.fcx); + } + } } fn adjust_upvar_borrow_kind_for_consume(&self, diff --git a/src/librustc_typeck/check/vtable.rs b/src/librustc_typeck/check/vtable.rs index 60e673bcc7b..5cf71a9be6a 100644 --- a/src/librustc_typeck/check/vtable.rs +++ b/src/librustc_typeck/check/vtable.rs @@ -280,8 +280,6 @@ fn check_object_type_binds_all_associated_types<'tcx>(tcx: &ty::ctxt<'tcx>, pub fn select_all_fcx_obligations_and_apply_defaults(fcx: &FnCtxt) { debug!("select_all_fcx_obligations_and_apply_defaults"); - fcx.inh.deferred_resolutions.borrow_mut() - .retain(|r| !r.attempt_resolution(fcx)); select_fcx_obligations_where_possible(fcx); fcx.default_type_parameters(); select_fcx_obligations_where_possible(fcx); @@ -290,6 +288,10 @@ pub fn select_all_fcx_obligations_and_apply_defaults(fcx: &FnCtxt) { pub fn select_all_fcx_obligations_or_error(fcx: &FnCtxt) { debug!("select_all_fcx_obligations_or_error"); + // upvar inference should have ensured that all deferrred call + // resolutions are handled by now. + assert!(fcx.inh.deferred_call_resolutions.borrow().is_empty()); + select_all_fcx_obligations_and_apply_defaults(fcx); let mut fulfillment_cx = fcx.inh.fulfillment_cx.borrow_mut(); let r = fulfillment_cx.select_all_or_error(fcx.infcx(), fcx); diff --git a/src/test/compile-fail/unboxed-closures-failed-recursive-fn-1.rs b/src/test/compile-fail/unboxed-closures-failed-recursive-fn-1.rs new file mode 100644 index 00000000000..7398e6f1089 --- /dev/null +++ b/src/test/compile-fail/unboxed-closures-failed-recursive-fn-1.rs @@ -0,0 +1,33 @@ +// 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. + +// Various unsuccessful attempts to put the unboxed closure kind +// inference into an awkward position that might require fixed point +// iteration (basically where inferring the kind of a closure `c` +// would require knowing the kind of `c`). I currently believe this is +// impossible. + +fn a() { + // This case of recursion wouldn't even require fixed-point + // iteration, but it still doesn't work. The weird structure with + // the `Option` is to avoid giving any useful hints about the `Fn` + // kind via the expected type. + let mut factorial: Option u32>> = None; + + let f = |x: u32| -> u32 { + let g = factorial.as_ref().unwrap(); + if x == 0 {1} else {x * g(x-1)} + }; + + factorial = Some(Box::new(f)); + //~^ ERROR cannot assign to `factorial` because it is borrowed +} + +fn main() { } diff --git a/src/test/compile-fail/unboxed-closures-failed-recursive-fn-2.rs b/src/test/compile-fail/unboxed-closures-failed-recursive-fn-2.rs new file mode 100644 index 00000000000..f40c8fc7474 --- /dev/null +++ b/src/test/compile-fail/unboxed-closures-failed-recursive-fn-2.rs @@ -0,0 +1,39 @@ +// 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. + +// Various unsuccessful attempts to put the unboxed closure kind +// inference into an awkward position that might require fixed point +// iteration (basically where inferring the kind of a closure `c` +// would require knowing the kind of `c`). I currently believe this is +// impossible. + +fn a() { + let mut closure0 = None; + let vec = vec!(1, 2, 3); + + loop { + { + let closure1 = || { + match closure0.take() { + Some(c) => { + return c(); + //~^ ERROR the type of this value must be known in this context + } + None => { } + } + }; + closure1(); + } + + closure0 = || vec; + } +} + +fn main() { } diff --git a/src/test/compile-fail/unboxed-closures-infer-fnmut-calling-fnmut-no-mut.rs b/src/test/compile-fail/unboxed-closures-infer-fnmut-calling-fnmut-no-mut.rs new file mode 100644 index 00000000000..afbc141b5d2 --- /dev/null +++ b/src/test/compile-fail/unboxed-closures-infer-fnmut-calling-fnmut-no-mut.rs @@ -0,0 +1,32 @@ +// 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; + + // Here this must be inferred to FnMut so that it can mutate counter, + // but we forgot the mut. + let tick1 = || { + counter += 1; + }; + + // In turn, tick2 must be inferred to FnMut so that it can call + // tick1, but we forgot the mut. The error message we currently + // get seems... suboptimal. + let tick2 = || { //~ ERROR closure cannot assign to immutable local variable `tick1` + tick1(); + }; + + tick2(); //~ ERROR cannot borrow +} + diff --git a/src/test/compile-fail/unboxed-closures-recursive-fn-using-fn-mut.rs b/src/test/compile-fail/unboxed-closures-recursive-fn-using-fn-mut.rs new file mode 100644 index 00000000000..215b2c6798e --- /dev/null +++ b/src/test/compile-fail/unboxed-closures-recursive-fn-using-fn-mut.rs @@ -0,0 +1,48 @@ +// 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(core,unboxed_closures)] + +use std::marker::CovariantType; + +// A erroneous variant of `run-pass/unboxed_closures-infer-recursive-fn.rs` +// where we attempt to perform mutation in the recursive function. This fails to compile +// because it winds up requiring `FnMut` which enforces linearity. + +struct YCombinator { + func: F, + marker: CovariantType<(A,R)>, +} + +impl YCombinator { + fn new(f: F) -> YCombinator { + YCombinator { func: f, marker: CovariantType } + } +} + +impl R, A) -> R> FnMut<(A,)> for YCombinator { + type Output = R; + + extern "rust-call" fn call_mut(&mut self, (arg,): (A,)) -> R { + (self.func)(self, arg) + //~^ ERROR cannot borrow `*self` as mutable more than once at a time + } +} + +fn main() { + let mut counter = 0; + let factorial = |recur: &mut FnMut(u32) -> u32, arg: u32| -> u32 { + counter += 1; + if arg == 0 {1} else {arg * recur(arg-1)} + }; + let mut factorial: YCombinator<_,u32,u32> = YCombinator::new(factorial); + let mut r = factorial(10); + assert_eq!(3628800, r); +} diff --git a/src/test/run-pass/unboxed-closures-infer-fnmut-calling-fnmut.rs b/src/test/run-pass/unboxed-closures-infer-fnmut-calling-fnmut.rs new file mode 100644 index 00000000000..09b8c8f4454 --- /dev/null +++ b/src/test/run-pass/unboxed-closures-infer-fnmut-calling-fnmut.rs @@ -0,0 +1,29 @@ +// 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; + + { + // Here this must be inferred to FnMut so that it can mutate counter: + let mut tick1 = || counter += 1; + + // In turn, tick2 must be inferred to FnMut so that it can call tick1: + let mut tick2 = || { tick1(); tick1(); }; + + tick2(); + } + + assert_eq!(counter, 2); +} + diff --git a/src/test/run-pass/unboxed-closures-infer-recursive-fn.rs b/src/test/run-pass/unboxed-closures-infer-recursive-fn.rs new file mode 100644 index 00000000000..1f9b821178c --- /dev/null +++ b/src/test/run-pass/unboxed-closures-infer-recursive-fn.rs @@ -0,0 +1,47 @@ +// 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(core,unboxed_closures)] + +use std::marker::CovariantType; + +// Test that we are able to infer a suitable kind for a "recursive" +// closure. As far as I can tell, coding up a recursive closure +// requires the good ol' [Y Combinator]. +// +// [Y Combinator]: http://en.wikipedia.org/wiki/Fixed-point_combinator#Y_combinator + +struct YCombinator { + func: F, + marker: CovariantType<(A,R)>, +} + +impl YCombinator { + fn new(f: F) -> YCombinator { + YCombinator { func: f, marker: CovariantType } + } +} + +impl R, A) -> R> Fn<(A,)> for YCombinator { + type Output = R; + + extern "rust-call" fn call(&self, (arg,): (A,)) -> R { + (self.func)(self, arg) + } +} + +fn main() { + let factorial = |recur: &Fn(u32) -> u32, arg: u32| -> u32 { + if arg == 0 {1} else {arg * recur(arg-1)} + }; + let factorial: YCombinator<_,u32,u32> = YCombinator::new(factorial); + let r = factorial(10); + assert_eq!(3628800, r); +}