From 2ec305d1bcd29a989405ccee32bd7a113058584e Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 22 Sep 2014 14:47:06 -0400 Subject: [PATCH] Move checks for closure bounds out of kind.rs --- src/librustc/diagnostics.rs | 3 - src/librustc/middle/kind.rs | 135 ------------------ src/librustc/middle/traits/coherence.rs | 4 +- src/librustc/middle/traits/mod.rs | 12 +- src/librustc/middle/typeck/check/regionck.rs | 68 ++++++--- src/librustc/middle/typeck/check/vtable2.rs | 8 ++ .../middle/typeck/infer/error_reporting.rs | 2 +- ...ure-bounds-static-cant-capture-borrowed.rs | 5 +- src/test/compile-fail/kindck-nonsendable-1.rs | 6 +- src/test/compile-fail/no-send-res-ports.rs | 2 +- 10 files changed, 74 insertions(+), 171 deletions(-) diff --git a/src/librustc/diagnostics.rs b/src/librustc/diagnostics.rs index 68027812070..23591de1b54 100644 --- a/src/librustc/diagnostics.rs +++ b/src/librustc/diagnostics.rs @@ -146,10 +146,7 @@ register_diagnostics!( E0139, E0140, E0141, - E0143, E0144, - E0145, - E0146, E0152, E0153, E0154, diff --git a/src/librustc/middle/kind.rs b/src/librustc/middle/kind.rs index 08e640f597a..0715baf2c5a 100644 --- a/src/librustc/middle/kind.rs +++ b/src/librustc/middle/kind.rs @@ -30,11 +30,6 @@ pub struct Context<'a,'tcx:'a> { } impl<'a, 'tcx, 'v> Visitor<'v> for Context<'a, 'tcx> { - fn visit_fn(&mut self, fk: visit::FnKind, fd: &'v FnDecl, - b: &'v Block, s: Span, n: NodeId) { - check_fn(self, fk, fd, b, s, n); - } - fn visit_ty(&mut self, t: &Ty) { check_ty(self, t); } @@ -48,110 +43,6 @@ pub fn check_crate(tcx: &ty::ctxt) { tcx.sess.abort_if_errors(); } -// Yields the appropriate function to check the kind of closed over -// variables. `id` is the NodeId for some expression that creates the -// closure. -fn with_appropriate_checker(cx: &Context, - id: NodeId, - fn_span: Span, - b: |checker: |&Context, &ty::Freevar||) { - fn check_for_uniq(cx: &Context, - fn_span: Span, - fv: &ty::Freevar, - bounds: ty::BuiltinBounds) { - // all captured data must be owned, regardless of whether it is - // moved in or copied in. - let id = fv.def.def_id().node; - let var_t = ty::node_id_to_type(cx.tcx, id); - - check_freevar_bounds(cx, fn_span, fv.span, var_t, bounds, None); - } - - fn check_for_block(cx: &Context, - fn_span: Span, - fn_id: NodeId, - fv: &ty::Freevar, - bounds: ty::BuiltinBounds) { - let id = fv.def.def_id().node; - let var_t = ty::node_id_to_type(cx.tcx, id); - let upvar_id = ty::UpvarId { var_id: id, closure_expr_id: fn_id }; - let upvar_borrow = cx.tcx.upvar_borrow(upvar_id); - let implicit_borrowed_type = - ty::mk_rptr(cx.tcx, - upvar_borrow.region, - ty::mt { mutbl: upvar_borrow.kind.to_mutbl_lossy(), - ty: var_t }); - check_freevar_bounds(cx, fn_span, fv.span, implicit_borrowed_type, - bounds, Some(var_t)); - } - - fn check_for_bare(cx: &Context, fv: &ty::Freevar) { - span_err!(cx.tcx.sess, fv.span, E0143, - "can't capture dynamic environment in a fn item; \ - use the || {} closure form instead", "{ ... }"); - } // same check is done in resolve.rs, but shouldn't be done - - let fty = ty::node_id_to_type(cx.tcx, id); - match ty::get(fty).sty { - ty::ty_closure(box ty::ClosureTy { - store: ty::UniqTraitStore, - bounds: bounds, - .. - }) => { - b(|cx, fv| check_for_uniq(cx, fn_span, fv, - bounds.builtin_bounds)) - } - - ty::ty_closure(box ty::ClosureTy { - store: ty::RegionTraitStore(..), bounds, .. - }) => { - b(|cx, fv| check_for_block(cx, fn_span, id, fv, - bounds.builtin_bounds)) - } - - ty::ty_bare_fn(_) => { - b(check_for_bare) - } - - ty::ty_unboxed_closure(..) => {} - - ref s => { - cx.tcx.sess.bug(format!("expect fn type in kind checker, not \ - {:?}", - s).as_slice()); - } - } -} - -// Check that the free variables used in a shared/sendable closure conform -// to the copy/move kind bounds. Then recursively check the function body. -fn check_fn( - cx: &mut Context, - fk: visit::FnKind, - decl: &FnDecl, - body: &Block, - sp: Span, - fn_id: NodeId) { - - // { - visit::walk_fn(cx, fk, decl, body, sp) - } - visit::FkItemFn(..) | visit::FkMethod(..) => { - visit::walk_fn(cx, fk, decl, body, sp); - } - } -} - fn check_ty(cx: &mut Context, aty: &Ty) { match aty.node { TyPath(_, _, id) => { @@ -208,29 +99,3 @@ pub fn check_typaram_bounds(cx: &Context, }); } -pub fn check_freevar_bounds(cx: &Context, fn_span: Span, sp: Span, ty: ty::t, - bounds: ty::BuiltinBounds, referenced_ty: Option) -{ - check_builtin_bounds(cx, ty, bounds, |missing| { - // Will be Some if the freevar is implicitly borrowed (stack closure). - // Emit a less mysterious error message in this case. - match referenced_ty { - Some(rty) => { - span_err!(cx.tcx.sess, sp, E0145, - "cannot implicitly borrow variable of type `{}` in a \ - bounded stack closure (implicit reference does not fulfill `{}`)", - ty_to_string(cx.tcx, rty), missing.user_string(cx.tcx)); - } - None => { - span_err!(cx.tcx.sess, sp, E0146, - "cannot capture variable of type `{}`, which does \ - not fulfill `{}`, in a bounded closure", - ty_to_string(cx.tcx, ty), missing.user_string(cx.tcx)); - } - } - span_note!(cx.tcx.sess, fn_span, - "this closure's environment must satisfy `{}`", - bounds.user_string(cx.tcx)); - }); -} - diff --git a/src/librustc/middle/traits/coherence.rs b/src/librustc/middle/traits/coherence.rs index d84729d1935..9b179d3e895 100644 --- a/src/librustc/middle/traits/coherence.rs +++ b/src/librustc/middle/traits/coherence.rs @@ -10,9 +10,9 @@ /*! See `doc.rs` for high-level documentation */ -use super::DUMMY_CAUSE; use super::{EvaluatedToMatch, EvaluatedToAmbiguity, EvaluatedToUnmatch}; use super::{evaluate_impl}; +use super::ObligationCause; use super::util; use middle::subst; @@ -39,7 +39,7 @@ pub fn impl_can_satisfy(infcx: &InferCtxt, // Determine whether `impl2` can provide an implementation for those // same types. let param_env = ty::empty_parameter_environment(); - match evaluate_impl(infcx, ¶m_env, infcx.tcx, DUMMY_CAUSE, + match evaluate_impl(infcx, ¶m_env, infcx.tcx, ObligationCause::dummy(), impl2_def_id, impl1_self_ty) { EvaluatedToMatch | EvaluatedToAmbiguity => true, EvaluatedToUnmatch => false, diff --git a/src/librustc/middle/traits/mod.rs b/src/librustc/middle/traits/mod.rs index 4ce32383525..db19ffd4fe2 100644 --- a/src/librustc/middle/traits/mod.rs +++ b/src/librustc/middle/traits/mod.rs @@ -77,11 +77,11 @@ pub enum ObligationCauseCode { StructInitializerSized, // S { ... } must be Sized VariableType(ast::NodeId), // Type of each variable must be Sized RepeatVec, // [T,..n] --> T must be Copy -} -pub static DUMMY_CAUSE: ObligationCause = - ObligationCause { span: DUMMY_SP, - code: MiscObligation }; + // Captures of variable the given id by a closure (span is the + // span of the closure) + ClosureCapture(ast::NodeId, Span) +} pub type Obligations = subst::VecPerParamSpace; @@ -358,6 +358,10 @@ impl ObligationCause { pub fn misc(span: Span) -> ObligationCause { ObligationCause { span: span, code: MiscObligation } } + + pub fn dummy() -> ObligationCause { + ObligationCause { span: DUMMY_SP, code: MiscObligation } + } } impl Vtable { diff --git a/src/librustc/middle/typeck/check/regionck.rs b/src/librustc/middle/typeck/check/regionck.rs index 85fe0a42c49..d45155c2ccd 100644 --- a/src/librustc/middle/typeck/check/regionck.rs +++ b/src/librustc/middle/typeck/check/regionck.rs @@ -120,11 +120,13 @@ and report an error, and it just seems like more mess in the end.) use middle::def; use middle::mem_categorization as mc; +use middle::traits; use middle::ty::{ReScope}; use middle::ty; use middle::typeck::astconv::AstConv; use middle::typeck::check::FnCtxt; use middle::typeck::check::regionmanip; +use middle::typeck::check::vtable2; use middle::typeck::infer::resolve_and_force_all_but_regions; use middle::typeck::infer::resolve_type; use middle::typeck::infer; @@ -165,6 +167,11 @@ pub fn regionck_fn(fcx: &FnCtxt, id: ast::NodeId, blk: &ast::Block) { // regionck assumes typeck succeeded rcx.visit_fn_body(id, blk); } + + // Region checking a fn can introduce new trait obligations, + // particularly around closure bounds. + vtable2::select_all_fcx_obligations_or_error(fcx); + fcx.infcx().resolve_regions_and_report_errors(); } @@ -848,16 +855,6 @@ fn check_expr_fn_block(rcx: &mut Rcx, } }); } - ty::ty_closure(box ty::ClosureTy{store: ty::UniqTraitStore, - bounds: ref bounds, - ..}) => { - // For proc, ensure that the *types* of the variables - // outlive region bound, since they are captured by value. - ty::with_freevars(tcx, expr.id, |freevars| { - ensure_free_variable_types_outlive_closure_bound( - rcx, bounds.region_bound, expr, freevars); - }); - } ty::ty_unboxed_closure(_, region) => { ty::with_freevars(tcx, expr.id, |freevars| { // No free variables means that there is no environment and @@ -868,8 +865,9 @@ fn check_expr_fn_block(rcx: &mut Rcx, // NDM -- this seems wrong, discuss with pcwalton, should // be straightforward enough. if !freevars.is_empty() { + let bounds = ty::region_existential_bound(region); ensure_free_variable_types_outlive_closure_bound( - rcx, region, expr, freevars); + rcx, bounds, expr, freevars); } }) } @@ -881,20 +879,26 @@ fn check_expr_fn_block(rcx: &mut Rcx, rcx.set_repeating_scope(repeating_scope); match ty::get(function_type).sty { - ty::ty_closure(box ty::ClosureTy { - store: ty::RegionTraitStore(..), - .. - }) => { + ty::ty_closure(box ty::ClosureTy { store: ty::RegionTraitStore(..), .. }) => { ty::with_freevars(tcx, expr.id, |freevars| { propagate_upupvar_borrow_kind(rcx, expr, freevars); }) } - _ => () + _ => {} + } + + match ty::get(function_type).sty { + ty::ty_closure(box ty::ClosureTy {bounds, ..}) => { + ty::with_freevars(tcx, expr.id, |freevars| { + ensure_free_variable_types_outlive_closure_bound(rcx, bounds, expr, freevars); + }) + } + _ => {} } fn ensure_free_variable_types_outlive_closure_bound( rcx: &mut Rcx, - region_bound: ty::Region, + bounds: ty::ExistentialBounds, expr: &ast::Expr, freevars: &[ty::Freevar]) { @@ -908,7 +912,7 @@ fn check_expr_fn_block(rcx: &mut Rcx, let tcx = rcx.fcx.ccx.tcx; debug!("ensure_free_variable_types_outlive_closure_bound({}, {})", - region_bound.repr(tcx), expr.repr(tcx)); + bounds.region_bound.repr(tcx), expr.repr(tcx)); for freevar in freevars.iter() { let var_node_id = { @@ -917,11 +921,35 @@ fn check_expr_fn_block(rcx: &mut Rcx, def_id.node }; - let var_ty = rcx.resolve_node_type(var_node_id); + // Compute the type of the field in the environment that + // represents `var_node_id`. For a by-value closure, this + // will be the same as the type of the variable. For a + // by-reference closure, this will be `&T` where `T` is + // the type of the variable. + let raw_var_ty = rcx.resolve_node_type(var_node_id); + let upvar_id = ty::UpvarId { var_id: var_node_id, + closure_expr_id: expr.id }; + let var_ty = match rcx.fcx.inh.upvar_borrow_map.borrow().find(&upvar_id) { + Some(upvar_borrow) => { + ty::mk_rptr(rcx.tcx(), + upvar_borrow.region, + ty::mt { mutbl: upvar_borrow.kind.to_mutbl_lossy(), + ty: raw_var_ty }) + } + None => raw_var_ty + }; + // Check that the type meets the criteria of the existential bounds: + for builtin_bound in bounds.builtin_bounds.iter() { + let code = traits::ClosureCapture(var_node_id, expr.span); + let cause = traits::ObligationCause::new(freevar.span, code); + let obligation = traits::obligation_for_builtin_bound(rcx.tcx(), cause, + var_ty, builtin_bound); + rcx.fcx.inh.fulfillment_cx.borrow_mut().register_obligation(rcx.tcx(), obligation); + } type_must_outlive( rcx, infer::RelateProcBound(expr.span, var_node_id, var_ty), - var_ty, region_bound); + var_ty, bounds.region_bound); } } diff --git a/src/librustc/middle/typeck/check/vtable2.rs b/src/librustc/middle/typeck/check/vtable2.rs index e0bf04a1cc3..345e74fbc90 100644 --- a/src/librustc/middle/typeck/check/vtable2.rs +++ b/src/librustc/middle/typeck/check/vtable2.rs @@ -398,5 +398,13 @@ fn note_obligation_cause(fcx: &FnCtxt, "use \"#[unsafe_destructor]\" on the implementation \ to force the compiler to allow this"); } + traits::ClosureCapture(var_id, closure_span) => { + let name = ty::local_var_name_str(tcx, var_id); + span_note!(tcx.sess, closure_span, + "the closure that captures `{}` requires that all captured variables \" + implement the trait `{}`", + name, + trait_name); + } } } diff --git a/src/librustc/middle/typeck/infer/error_reporting.rs b/src/librustc/middle/typeck/infer/error_reporting.rs index 742c2b8aabd..2ad6a1f72e2 100644 --- a/src/librustc/middle/typeck/infer/error_reporting.rs +++ b/src/librustc/middle/typeck/infer/error_reporting.rs @@ -1560,7 +1560,7 @@ impl<'a, 'tcx> ErrorReportingHelpers for InferCtxt<'a, 'tcx> { "...so that it can be closed over into an object"); } infer::RelateProcBound(span, var_node_id, _ty) => { - self.tcx.sess.span_err( + self.tcx.sess.span_note( span, format!( "...so that the variable `{}` can be captured \ diff --git a/src/test/compile-fail/closure-bounds-static-cant-capture-borrowed.rs b/src/test/compile-fail/closure-bounds-static-cant-capture-borrowed.rs index c0b463535d4..6769740294b 100644 --- a/src/test/compile-fail/closure-bounds-static-cant-capture-borrowed.rs +++ b/src/test/compile-fail/closure-bounds-static-cant-capture-borrowed.rs @@ -12,8 +12,9 @@ fn bar(blk: ||:'static) { } fn foo(x: &()) { - bar(|| { - let _ = x; //~ ERROR captured variable `x` does not outlive + bar(|| { //~ ERROR cannot infer an appropriate lifetime + let _ = x; + //~^ ERROR captured variable `x` does not outlive }) } diff --git a/src/test/compile-fail/kindck-nonsendable-1.rs b/src/test/compile-fail/kindck-nonsendable-1.rs index de4b6ed3545..f292d159982 100644 --- a/src/test/compile-fail/kindck-nonsendable-1.rs +++ b/src/test/compile-fail/kindck-nonsendable-1.rs @@ -15,8 +15,8 @@ fn foo(_x: Gc) {} fn main() { let x = box(GC) 3u; - let _: proc():Send = proc() foo(x); //~ ERROR does not fulfill `Send` - let _: proc():Send = proc() foo(x); //~ ERROR does not fulfill `Send` - let _: proc():Send = proc() foo(x); //~ ERROR does not fulfill `Send` + let _: proc():Send = proc() foo(x); //~ ERROR `core::kinds::Send` is not implemented + let _: proc():Send = proc() foo(x); //~ ERROR `core::kinds::Send` is not implemented + let _: proc():Send = proc() foo(x); //~ ERROR `core::kinds::Send` is not implemented let _: proc() = proc() foo(x); } diff --git a/src/test/compile-fail/no-send-res-ports.rs b/src/test/compile-fail/no-send-res-ports.rs index f58350cf093..ddbfbc41eca 100644 --- a/src/test/compile-fail/no-send-res-ports.rs +++ b/src/test/compile-fail/no-send-res-ports.rs @@ -37,7 +37,7 @@ fn main() { task::spawn(proc() { let y = x; - //~^ ERROR does not fulfill `Send` + //~^ ERROR `core::kinds::Send` is not implemented println!("{:?}", y); }); }