From c00574848b029d3458326a728dae682e60323a0d Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 8 Dec 2015 23:38:36 +0100 Subject: [PATCH 1/2] Ensure borrows of fn/closure params do not outlive invocations. resolve_lifetime.rs: Switch from BlockScope to FnScope in ScopeChain construction. Lifetimes introduced by a fn signature are scoped to the call-site for that fn. (Note `add_scope_and_walk_fn` must only add FnScope for the walk of body, *not* of the fn signature.) region.rs: Introduce new CodeExtentData::CallSiteScope variant. Use CodeExtentData as the cx.parent, rather than just a NodeId. Change DestructionScopeData to CallSiteScopeData. regionck.rs: Thread call_site_scope via Rcx; constrain fn return values. (update; incorporated review feedback from niko.) --- src/librustc/middle/infer/error_reporting.rs | 3 + src/librustc/middle/liveness.rs | 5 +- src/librustc/middle/region.rs | 29 +++++--- src/librustc/middle/resolve_lifetime.rs | 61 ++++++++--------- src/librustc/middle/traits/object_safety.rs | 3 +- src/librustc/middle/ty/mod.rs | 72 ++++++++++---------- src/librustc/middle/ty/structural_impls.rs | 2 +- src/librustc_metadata/tydecode.rs | 11 +++ src/librustc_metadata/tyencode.rs | 2 + src/librustc_typeck/astconv.rs | 2 +- src/librustc_typeck/check/closure.rs | 2 +- src/librustc_typeck/check/mod.rs | 2 +- src/librustc_typeck/check/regionck.rs | 38 ++++++++++- src/librustc_typeck/check/wf.rs | 3 +- src/librustc_typeck/check/wfcheck.rs | 11 +-- src/librustc_typeck/collect.rs | 20 ++++-- 16 files changed, 170 insertions(+), 96 deletions(-) diff --git a/src/librustc/middle/infer/error_reporting.rs b/src/librustc/middle/infer/error_reporting.rs index 3f91c20c32aac..56e1e2480b97c 100644 --- a/src/librustc/middle/infer/error_reporting.rs +++ b/src/librustc/middle/infer/error_reporting.rs @@ -148,6 +148,9 @@ impl<'tcx> ty::ctxt<'tcx> { }; let scope_decorated_tag = match self.region_maps.code_extent_data(scope) { region::CodeExtentData::Misc(_) => tag, + region::CodeExtentData::CallSiteScope { .. } => { + "scope of call-site for function" + } region::CodeExtentData::ParameterScope { .. } => { "scope of parameters for function" } diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 2a8b1b83d224c..2df946513f367 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -1466,10 +1466,11 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { entry_ln: LiveNode, body: &hir::Block) { - // within the fn body, late-bound regions are liberated: + // within the fn body, late-bound regions are liberated + // and must outlive the *call-site* of the function. let fn_ret = self.ir.tcx.liberate_late_bound_regions( - self.ir.tcx.region_maps.item_extent(body.id), + self.ir.tcx.region_maps.call_site_extent(id, body.id), &self.fn_ret(id)); match fn_ret { diff --git a/src/librustc/middle/region.rs b/src/librustc/middle/region.rs index 546b91c9d5d4c..ef12bca45bdbd 100644 --- a/src/librustc/middle/region.rs +++ b/src/librustc/middle/region.rs @@ -125,6 +125,10 @@ pub const DUMMY_CODE_EXTENT : CodeExtent = CodeExtent(1); pub enum CodeExtentData { Misc(ast::NodeId), + // extent of the call-site for a function or closure (outlives + // the parameters as well as the body). + CallSiteScope { fn_id: ast::NodeId, body_id: ast::NodeId }, + // extent of parameters passed to a function or closure (they // outlive its body) ParameterScope { fn_id: ast::NodeId, body_id: ast::NodeId }, @@ -136,20 +140,20 @@ pub enum CodeExtentData { Remainder(BlockRemainder) } -/// extent of destructors for temporaries of node-id +/// extent of call-site for a function/method. #[derive(Clone, PartialEq, PartialOrd, Eq, Ord, Hash, RustcEncodable, RustcDecodable, Debug, Copy)] -pub struct DestructionScopeData { - pub node_id: ast::NodeId +pub struct CallSiteScopeData { + pub fn_id: ast::NodeId, pub body_id: ast::NodeId, } -impl DestructionScopeData { - pub fn new(node_id: ast::NodeId) -> DestructionScopeData { - DestructionScopeData { node_id: node_id } - } +impl CallSiteScopeData { pub fn to_code_extent(&self, region_maps: &RegionMaps) -> CodeExtent { region_maps.lookup_code_extent( - CodeExtentData::DestructionScope(self.node_id)) + match *self { + CallSiteScopeData { fn_id, body_id } => + CodeExtentData::CallSiteScope { fn_id: fn_id, body_id: body_id }, + }) } } @@ -190,6 +194,7 @@ impl CodeExtentData { // precise extent denoted by `self`. CodeExtentData::Remainder(br) => br.block, CodeExtentData::DestructionScope(node_id) => node_id, + CodeExtentData::CallSiteScope { fn_id: _, body_id } | CodeExtentData::ParameterScope { fn_id: _, body_id } => body_id, } } @@ -215,6 +220,7 @@ impl CodeExtent { match ast_map.find(self.node_id(region_maps)) { Some(ast_map::NodeBlock(ref blk)) => { match region_maps.code_extent_data(*self) { + CodeExtentData::CallSiteScope { .. } | CodeExtentData::ParameterScope { .. } | CodeExtentData::Misc(_) | CodeExtentData::DestructionScope(_) => Some(blk.span), @@ -346,6 +352,10 @@ impl RegionMaps { pub fn item_extent(&self, n: ast::NodeId) -> CodeExtent { self.lookup_code_extent(CodeExtentData::DestructionScope(n)) } + pub fn call_site_extent(&self, fn_id: ast::NodeId, body_id: ast::NodeId) -> CodeExtent { + assert!(fn_id != body_id); + self.lookup_code_extent(CodeExtentData::CallSiteScope { fn_id: fn_id, body_id: body_id }) + } pub fn opt_destruction_extent(&self, n: ast::NodeId) -> Option { self.code_extent_interner.borrow().get(&CodeExtentData::DestructionScope(n)).cloned() } @@ -1101,6 +1111,9 @@ fn resolve_fn(visitor: &mut RegionResolutionVisitor, body.id, visitor.cx.parent); + visitor.cx.parent = visitor.new_code_extent( + CodeExtentData::CallSiteScope { fn_id: id, body_id: body.id }); + let fn_decl_scope = visitor.new_code_extent( CodeExtentData::ParameterScope { fn_id: id, body_id: body.id }); diff --git a/src/librustc/middle/resolve_lifetime.rs b/src/librustc/middle/resolve_lifetime.rs index 4425a24590c83..b45d9482809bf 100644 --- a/src/librustc/middle/resolve_lifetime.rs +++ b/src/librustc/middle/resolve_lifetime.rs @@ -42,7 +42,7 @@ pub enum DefRegion { /* lifetime decl */ ast::NodeId), DefLateBoundRegion(ty::DebruijnIndex, /* lifetime decl */ ast::NodeId), - DefFreeRegion(/* block scope */ region::DestructionScopeData, + DefFreeRegion(region::CallSiteScopeData, /* lifetime decl */ ast::NodeId), } @@ -83,9 +83,9 @@ enum ScopeChain<'a> { /// LateScope(['a, 'b, ...], s) extends s with late-bound /// lifetimes introduced by the declaration binder_id. LateScope(&'a Vec, Scope<'a>), - /// lifetimes introduced by items within a code block are scoped - /// to that block. - BlockScope(region::DestructionScopeData, Scope<'a>), + + /// lifetimes introduced by a fn are scoped to the call-site for that fn. + FnScope { fn_id: ast::NodeId, body_id: ast::NodeId, s: Scope<'a> }, RootScope } @@ -172,20 +172,20 @@ impl<'a, 'v> Visitor<'v> for LifetimeContext<'a> { } fn visit_fn(&mut self, fk: FnKind<'v>, fd: &'v hir::FnDecl, - b: &'v hir::Block, s: Span, _: ast::NodeId) { + b: &'v hir::Block, s: Span, fn_id: ast::NodeId) { match fk { FnKind::ItemFn(_, generics, _, _, _, _) => { self.visit_early_late(subst::FnSpace, generics, |this| { - this.walk_fn(fk, fd, b, s) + this.add_scope_and_walk_fn(fk, fd, b, s, fn_id) }) } FnKind::Method(_, sig, _) => { self.visit_early_late(subst::FnSpace, &sig.generics, |this| { - this.walk_fn(fk, fd, b, s) + this.add_scope_and_walk_fn(fk, fd, b, s, fn_id) }) } FnKind::Closure => { - self.walk_fn(fk, fd, b, s) + self.add_scope_and_walk_fn(fk, fd, b, s, fn_id) } } } @@ -236,12 +236,6 @@ impl<'a, 'v> Visitor<'v> for LifetimeContext<'a> { replace(&mut self.labels_in_fn, saved); } - fn visit_block(&mut self, b: &hir::Block) { - self.with(BlockScope(region::DestructionScopeData::new(b.id), - self.scope), - |_, this| intravisit::walk_block(this, b)); - } - fn visit_lifetime(&mut self, lifetime_ref: &hir::Lifetime) { if lifetime_ref.name == special_idents::static_lifetime.name { self.insert_lifetime(lifetime_ref, DefStaticRegion); @@ -437,7 +431,7 @@ fn extract_labels<'v, 'a>(ctxt: &mut LifetimeContext<'a>, b: &'v hir::Block) { label_span: Span) { loop { match *scope { - BlockScope(_, s) => { scope = s; } + FnScope { s, .. } => { scope = s; } RootScope => { return; } EarlyScope(_, lifetimes, s) | @@ -461,14 +455,13 @@ fn extract_labels<'v, 'a>(ctxt: &mut LifetimeContext<'a>, b: &'v hir::Block) { } impl<'a> LifetimeContext<'a> { - // This is just like intravisit::walk_fn, except that it extracts the - // labels of the function body and swaps them in before visiting - // the function body itself. - fn walk_fn<'b>(&mut self, - fk: FnKind, - fd: &hir::FnDecl, - fb: &'b hir::Block, - _span: Span) { + fn add_scope_and_walk_fn<'b>(&mut self, + fk: FnKind, + fd: &hir::FnDecl, + fb: &'b hir::Block, + _span: Span, + fn_id: ast::NodeId) { + match fk { FnKind::ItemFn(_, generics, _, _, _, _) => { intravisit::walk_fn_decl(self, fd); @@ -488,7 +481,8 @@ impl<'a> LifetimeContext<'a> { // `self.labels_in_fn`. extract_labels(self, fb); - self.visit_block(fb); + self.with(FnScope { fn_id: fn_id, body_id: fb.id, s: self.scope }, + |_old_scope, this| this.visit_block(fb)) } fn with(&mut self, wrap_scope: ScopeChain, f: F) where @@ -559,8 +553,11 @@ impl<'a> LifetimeContext<'a> { let mut scope = self.scope; loop { match *scope { - BlockScope(blk_scope, s) => { - return self.resolve_free_lifetime_ref(blk_scope, lifetime_ref, s); + FnScope {fn_id, body_id, s } => { + return self.resolve_free_lifetime_ref( + region::CallSiteScopeData { fn_id: fn_id, body_id: body_id }, + lifetime_ref, + s); } RootScope => { @@ -604,7 +601,7 @@ impl<'a> LifetimeContext<'a> { } fn resolve_free_lifetime_ref(&mut self, - scope_data: region::DestructionScopeData, + scope_data: region::CallSiteScopeData, lifetime_ref: &hir::Lifetime, scope: Scope) { debug!("resolve_free_lifetime_ref \ @@ -622,8 +619,10 @@ impl<'a> LifetimeContext<'a> { scope_data: {:?} scope: {:?} search_result: {:?}", scope_data, scope, search_result); match *scope { - BlockScope(blk_scope_data, s) => { - scope_data = blk_scope_data; + FnScope { fn_id, body_id, s } => { + scope_data = region::CallSiteScopeData { + fn_id: fn_id, body_id: body_id + }; scope = s; } @@ -711,7 +710,7 @@ impl<'a> LifetimeContext<'a> { loop { match *old_scope { - BlockScope(_, s) => { + FnScope { s, .. } => { old_scope = s; } @@ -864,7 +863,7 @@ impl<'a> fmt::Debug for ScopeChain<'a> { match *self { EarlyScope(space, defs, _) => write!(fmt, "EarlyScope({:?}, {:?})", space, defs), LateScope(defs, _) => write!(fmt, "LateScope({:?})", defs), - BlockScope(id, _) => write!(fmt, "BlockScope({:?})", id), + FnScope { fn_id, body_id, s: _ } => write!(fmt, "FnScope({:?}, {:?})", fn_id, body_id), RootScope => write!(fmt, "RootScope"), } } diff --git a/src/librustc/middle/traits/object_safety.rs b/src/librustc/middle/traits/object_safety.rs index 934c35fa20bc8..bd60d0a212252 100644 --- a/src/librustc/middle/traits/object_safety.rs +++ b/src/librustc/middle/traits/object_safety.rs @@ -192,7 +192,8 @@ fn generics_require_sized_self<'tcx>(tcx: &ty::ctxt<'tcx>, }; // Search for a predicate like `Self : Sized` amongst the trait bounds. - let free_substs = tcx.construct_free_substs(generics, ast::DUMMY_NODE_ID); + let free_substs = tcx.construct_free_substs(generics, + tcx.region_maps.node_extent(ast::DUMMY_NODE_ID)); let predicates = predicates.instantiate(tcx, &free_substs).predicates.into_vec(); elaborate_predicates(tcx, predicates) .any(|predicate| { diff --git a/src/librustc/middle/ty/mod.rs b/src/librustc/middle/ty/mod.rs index 71ae8e40b45f0..cf9f59abf1546 100644 --- a/src/librustc/middle/ty/mod.rs +++ b/src/librustc/middle/ty/mod.rs @@ -26,6 +26,7 @@ use middle::cstore::{CrateStore, LOCAL_CRATE}; use middle::def::{self, ExportMap}; use middle::def_id::DefId; use middle::lang_items::{FnTraitLangItem, FnMutTraitLangItem, FnOnceTraitLangItem}; +use middle::region::{CodeExtent}; use middle::subst::{self, ParamSpace, Subst, Substs, VecPerParamSpace}; use middle::traits; use middle::ty; @@ -1098,7 +1099,7 @@ pub struct ParameterEnvironment<'a, 'tcx:'a> { /// FIXME(#3696). It would be nice to refactor so that free /// regions don't have this implicit scope and instead introduce /// relationships in the environment. - pub free_id: ast::NodeId, + pub free_id_outlive: CodeExtent, } impl<'a, 'tcx> ParameterEnvironment<'a, 'tcx> { @@ -1113,7 +1114,7 @@ impl<'a, 'tcx> ParameterEnvironment<'a, 'tcx> { caller_bounds: caller_bounds, selection_cache: traits::SelectionCache::new(), evaluation_cache: traits::EvaluationCache::new(), - free_id: self.free_id, + free_id_outlive: self.free_id_outlive, } } @@ -1131,7 +1132,7 @@ impl<'a, 'tcx> ParameterEnvironment<'a, 'tcx> { cx.construct_parameter_environment(impl_item.span, &scheme.generics, &predicates, - id) + cx.region_maps.item_extent(id)) } hir::ImplItemKind::Const(_, _) => { let def_id = cx.map.local_def_id(id); @@ -1140,7 +1141,7 @@ impl<'a, 'tcx> ParameterEnvironment<'a, 'tcx> { cx.construct_parameter_environment(impl_item.span, &scheme.generics, &predicates, - id) + cx.region_maps.item_extent(id)) } hir::ImplItemKind::Method(_, ref body) => { let method_def_id = cx.map.local_def_id(id); @@ -1152,7 +1153,7 @@ impl<'a, 'tcx> ParameterEnvironment<'a, 'tcx> { impl_item.span, method_generics, method_bounds, - body.id) + cx.region_maps.call_site_extent(id, body.id)) } _ => { cx.sess @@ -1175,7 +1176,7 @@ impl<'a, 'tcx> ParameterEnvironment<'a, 'tcx> { cx.construct_parameter_environment(trait_item.span, &trait_def.generics, &predicates, - id) + cx.region_maps.item_extent(id)) } hir::ConstTraitItem(..) => { let def_id = cx.map.local_def_id(id); @@ -1184,23 +1185,29 @@ impl<'a, 'tcx> ParameterEnvironment<'a, 'tcx> { cx.construct_parameter_environment(trait_item.span, &scheme.generics, &predicates, - id) + cx.region_maps.item_extent(id)) } hir::MethodTraitItem(_, ref body) => { - // for the body-id, use the id of the body - // block, unless this is a trait method with - // no default, then fallback to the method id. - let body_id = body.as_ref().map(|b| b.id).unwrap_or(id); + // Use call-site for extent (unless this is a + // trait method with no default; then fallback + // to the method id). let method_def_id = cx.map.local_def_id(id); match cx.impl_or_trait_item(method_def_id) { MethodTraitItem(ref method_ty) => { let method_generics = &method_ty.generics; let method_bounds = &method_ty.predicates; + let extent = if let Some(ref body) = *body { + // default impl: use call_site extent as free_id_outlive bound. + cx.region_maps.call_site_extent(id, body.id) + } else { + // no default impl: use item extent as free_id_outlive bound. + cx.region_maps.item_extent(id) + }; cx.construct_parameter_environment( trait_item.span, method_generics, method_bounds, - body_id) + extent) } _ => { cx.sess @@ -1223,7 +1230,8 @@ impl<'a, 'tcx> ParameterEnvironment<'a, 'tcx> { cx.construct_parameter_environment(item.span, &fn_scheme.generics, &fn_predicates, - body.id) + cx.region_maps.call_site_extent(id, + body.id)) } hir::ItemEnum(..) | hir::ItemStruct(..) | @@ -1236,7 +1244,7 @@ impl<'a, 'tcx> ParameterEnvironment<'a, 'tcx> { cx.construct_parameter_environment(item.span, &scheme.generics, &predicates, - id) + cx.region_maps.item_extent(id)) } hir::ItemTrait(..) => { let def_id = cx.map.local_def_id(id); @@ -1245,7 +1253,7 @@ impl<'a, 'tcx> ParameterEnvironment<'a, 'tcx> { cx.construct_parameter_environment(item.span, &trait_def.generics, &predicates, - id) + cx.region_maps.item_extent(id)) } _ => { cx.sess.span_bug(item.span, @@ -2576,18 +2584,17 @@ impl<'tcx> ctxt<'tcx> { /// are no free type/lifetime parameters in scope. pub fn empty_parameter_environment<'a>(&'a self) -> ParameterEnvironment<'a,'tcx> { + + // for an empty parameter environment, there ARE no free + // regions, so it shouldn't matter what we use for the free id + let free_id_outlive = self.region_maps.node_extent(ast::DUMMY_NODE_ID); ty::ParameterEnvironment { tcx: self, free_substs: Substs::empty(), caller_bounds: Vec::new(), implicit_region_bound: ty::ReEmpty, selection_cache: traits::SelectionCache::new(), evaluation_cache: traits::EvaluationCache::new(), - - // for an empty parameter - // environment, there ARE no free - // regions, so it shouldn't matter - // what we use for the free id - free_id: ast::DUMMY_NODE_ID } + free_id_outlive: free_id_outlive } } /// Constructs and returns a substitution that can be applied to move from @@ -2596,7 +2603,7 @@ impl<'tcx> ctxt<'tcx> { /// free parameters. Since we currently represent bound/free type /// parameters in the same way, this only has an effect on regions. pub fn construct_free_substs(&self, generics: &Generics<'tcx>, - free_id: NodeId) -> Substs<'tcx> { + free_id_outlive: CodeExtent) -> Substs<'tcx> { // map T => T let mut types = VecPerParamSpace::empty(); for def in generics.types.as_slice() { @@ -2605,8 +2612,6 @@ impl<'tcx> ctxt<'tcx> { types.push(def.space, self.mk_param_from_def(def)); } - let free_id_outlive = self.region_maps.item_extent(free_id); - // map bound 'a => free 'a let mut regions = VecPerParamSpace::empty(); for def in generics.regions.as_slice() { @@ -2623,20 +2628,21 @@ impl<'tcx> ctxt<'tcx> { } } - /// See `ParameterEnvironment` struct def'n for details + /// See `ParameterEnvironment` struct def'n for details. + /// If you were using `free_id: NodeId`, you might try `self.region_maps.item_extent(free_id)` + /// for the `free_id_outlive` parameter. (But note that that is not always quite right.) pub fn construct_parameter_environment<'a>(&'a self, span: Span, generics: &ty::Generics<'tcx>, generic_predicates: &ty::GenericPredicates<'tcx>, - free_id: NodeId) + free_id_outlive: CodeExtent) -> ParameterEnvironment<'a, 'tcx> { // // Construct the free substs. // - let free_substs = self.construct_free_substs(generics, free_id); - let free_id_outlive = self.region_maps.item_extent(free_id); + let free_substs = self.construct_free_substs(generics, free_id_outlive); // // Compute the bounds on Self and the type parameters. @@ -2646,12 +2652,6 @@ impl<'tcx> ctxt<'tcx> { let bounds = self.liberate_late_bound_regions(free_id_outlive, &ty::Binder(bounds)); let predicates = bounds.predicates.into_vec(); - debug!("construct_parameter_environment: free_id={:?} free_subst={:?} predicates={:?}", - free_id, - free_substs, - predicates); - - // // Finally, we have to normalize the bounds in the environment, in // case they contain any associated type projections. This process // can yield errors if the put in illegal associated types, like @@ -2672,10 +2672,10 @@ impl<'tcx> ctxt<'tcx> { caller_bounds: predicates, selection_cache: traits::SelectionCache::new(), evaluation_cache: traits::EvaluationCache::new(), - free_id: free_id, + free_id_outlive: free_id_outlive, }; - let cause = traits::ObligationCause::misc(span, free_id); + let cause = traits::ObligationCause::misc(span, free_id_outlive.node_id(&self.region_maps)); traits::normalize_param_env_or_error(unnormalized_env, cause) } diff --git a/src/librustc/middle/ty/structural_impls.rs b/src/librustc/middle/ty/structural_impls.rs index 41303b46dfd09..e6007809af5e9 100644 --- a/src/librustc/middle/ty/structural_impls.rs +++ b/src/librustc/middle/ty/structural_impls.rs @@ -823,7 +823,7 @@ impl<'a, 'tcx> TypeFoldable<'tcx> for ty::ParameterEnvironment<'a, 'tcx> where ' caller_bounds: self.caller_bounds.fold_with(folder), selection_cache: traits::SelectionCache::new(), evaluation_cache: traits::EvaluationCache::new(), - free_id: self.free_id, + free_id_outlive: self.free_id_outlive, } } } diff --git a/src/librustc_metadata/tydecode.rs b/src/librustc_metadata/tydecode.rs index 34f289456bbde..768203c3e9e59 100644 --- a/src/librustc_metadata/tydecode.rs +++ b/src/librustc_metadata/tydecode.rs @@ -233,6 +233,17 @@ impl<'a,'tcx> TyDecoder<'a,'tcx> { // doesn't care about regions. // // May still be worth fixing though. + 'C' => { + assert_eq!(self.next(), '['); + let fn_id = self.parse_uint() as ast::NodeId; + assert_eq!(self.next(), '|'); + let body_id = self.parse_uint() as ast::NodeId; + assert_eq!(self.next(), ']'); + region::CodeExtentData::CallSiteScope { + fn_id: fn_id, body_id: body_id + } + } + // This creates scopes with the wrong NodeId. (See note above.) 'P' => { assert_eq!(self.next(), '['); let fn_id = self.parse_uint() as ast::NodeId; diff --git a/src/librustc_metadata/tyencode.rs b/src/librustc_metadata/tyencode.rs index bc1edd5c76718..c1910e3f249db 100644 --- a/src/librustc_metadata/tyencode.rs +++ b/src/librustc_metadata/tyencode.rs @@ -286,6 +286,8 @@ pub fn enc_region(w: &mut Encoder, cx: &ctxt, r: ty::Region) { fn enc_scope(w: &mut Encoder, cx: &ctxt, scope: region::CodeExtent) { match cx.tcx.region_maps.code_extent_data(scope) { + region::CodeExtentData::CallSiteScope { + fn_id, body_id } => mywrite!(w, "C[{}|{}]", fn_id, body_id), region::CodeExtentData::ParameterScope { fn_id, body_id } => mywrite!(w, "P[{}|{}]", fn_id, body_id), region::CodeExtentData::Misc(node_id) => mywrite!(w, "M{}", node_id), diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index b83f0a5be6f04..79c93980a2f3d 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -181,7 +181,7 @@ pub fn ast_region_to_region(tcx: &ty::ctxt, lifetime: &hir::Lifetime) Some(&rl::DefFreeRegion(scope, id)) => { ty::ReFree(ty::FreeRegion { - scope: tcx.region_maps.item_extent(scope.node_id), + scope: scope.to_code_extent(&tcx.region_maps), bound_region: ty::BrNamed(tcx.map.local_def_id(id), lifetime.name) }) diff --git a/src/librustc_typeck/check/closure.rs b/src/librustc_typeck/check/closure.rs index 4f7a639539546..7792169d3eb46 100644 --- a/src/librustc_typeck/check/closure.rs +++ b/src/librustc_typeck/check/closure.rs @@ -75,7 +75,7 @@ fn check_closure<'a,'tcx>(fcx: &FnCtxt<'a,'tcx>, fcx.write_ty(expr.id, closure_type); let fn_sig = fcx.tcx().liberate_late_bound_regions( - fcx.tcx().region_maps.item_extent(body.id), &fn_ty.sig); + fcx.tcx().region_maps.call_site_extent(expr.id, body.id), &fn_ty.sig); check_fn(fcx.ccx, hir::Unsafety::Normal, diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 622b2e4238fbb..9ff7cc4607bfa 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -456,7 +456,7 @@ fn check_bare_fn<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>, let inh = Inherited::new(ccx.tcx, &tables, param_env); // Compute the fty from point of view of inside fn. - let fn_scope = ccx.tcx.region_maps.item_extent(body.id); + let fn_scope = ccx.tcx.region_maps.call_site_extent(fn_id, body.id); let fn_sig = fn_ty.sig.subst(ccx.tcx, &inh.infcx.parameter_environment.free_substs); let fn_sig = diff --git a/src/librustc_typeck/check/regionck.rs b/src/librustc_typeck/check/regionck.rs index f980945dbf220..3ecb9468ef47a 100644 --- a/src/librustc_typeck/check/regionck.rs +++ b/src/librustc_typeck/check/regionck.rs @@ -89,7 +89,7 @@ use middle::free_region::FreeRegionMap; use middle::implicator::{self, Implication}; use middle::mem_categorization as mc; use middle::mem_categorization::Categorization; -use middle::region::CodeExtent; +use middle::region::{self, CodeExtent}; use middle::subst::Substs; use middle::traits; use middle::ty::{self, RegionEscape, ReScope, Ty, MethodCall, HasTypeFlags}; @@ -180,6 +180,9 @@ pub struct Rcx<'a, 'tcx: 'a> { // id of innermost fn body id body_id: ast::NodeId, + // call_site scope of innermost fn + call_site_scope: Option, + // id of innermost fn or loop repeating_scope: ast::NodeId, @@ -200,6 +203,7 @@ impl<'a, 'tcx> Rcx<'a, 'tcx> { Rcx { fcx: fcx, repeating_scope: initial_repeating_scope, body_id: initial_body_id, + call_site_scope: None, subject: subject, region_bound_pairs: Vec::new(), free_region_map: FreeRegionMap::new(), @@ -214,6 +218,10 @@ impl<'a, 'tcx> Rcx<'a, 'tcx> { self.fcx.infcx() } + fn set_call_site_scope(&mut self, call_site_scope: Option) -> Option { + mem::replace(&mut self.call_site_scope, call_site_scope) + } + fn set_body_id(&mut self, body_id: ast::NodeId) -> ast::NodeId { mem::replace(&mut self.body_id, body_id) } @@ -275,7 +283,7 @@ impl<'a, 'tcx> Rcx<'a, 'tcx> { } fn visit_fn_body(&mut self, - id: ast::NodeId, + id: ast::NodeId, // the id of the fn itself fn_decl: &hir::FnDecl, body: &hir::Block, span: Span) @@ -283,6 +291,10 @@ impl<'a, 'tcx> Rcx<'a, 'tcx> { // When we enter a function, we can derive debug!("visit_fn_body(id={})", id); + let call_site = self.fcx.tcx().region_maps.lookup_code_extent( + region::CodeExtentData::CallSiteScope { fn_id: id, body_id: body.id }); + let old_call_site_scope = self.set_call_site_scope(Some(call_site)); + let fn_sig = { let fn_sig_map = &self.infcx().tables.borrow().liberated_fn_sigs; match fn_sig_map.get(&id) { @@ -300,7 +312,7 @@ impl<'a, 'tcx> Rcx<'a, 'tcx> { // For the return type, if diverging, substitute `bool` just // because it will have no effect. // - // FIXME(#25759) return types should not be implied bounds + // FIXME(#27579) return types should not be implied bounds let fn_sig_tys: Vec<_> = fn_sig.inputs.iter() .cloned() @@ -315,9 +327,18 @@ impl<'a, 'tcx> Rcx<'a, 'tcx> { self.visit_block(body); self.visit_region_obligations(body.id); + let call_site_scope = self.call_site_scope.unwrap(); + debug!("visit_fn_body body.id {} call_site_scope: {:?}", + body.id, call_site_scope); + type_of_node_must_outlive(self, + infer::CallReturn(span), + body.id, + ty::ReScope(call_site_scope)); + self.region_bound_pairs.truncate(old_region_bounds_pairs_len); self.set_body_id(old_body_id); + self.set_call_site_scope(old_call_site_scope); } fn visit_region_obligations(&mut self, node_id: ast::NodeId) @@ -834,6 +855,17 @@ fn visit_expr(rcx: &mut Rcx, expr: &hir::Expr) { rcx.set_repeating_scope(repeating_scope); } + hir::ExprRet(Some(ref ret_expr)) => { + let call_site_scope = rcx.call_site_scope; + debug!("visit_expr ExprRet ret_expr.id {} call_site_scope: {:?}", + ret_expr.id, call_site_scope); + type_of_node_must_outlive(rcx, + infer::CallReturn(ret_expr.span), + ret_expr.id, + ty::ReScope(call_site_scope.unwrap())); + intravisit::walk_expr(rcx, expr); + } + _ => { intravisit::walk_expr(rcx, expr); } diff --git a/src/librustc_typeck/check/wf.rs b/src/librustc_typeck/check/wf.rs index 3daf5003b97e0..ee2845c824e3b 100644 --- a/src/librustc_typeck/check/wf.rs +++ b/src/librustc_typeck/check/wf.rs @@ -137,10 +137,11 @@ impl<'ccx, 'tcx> CheckTypeWellFormedVisitor<'ccx, 'tcx> { let type_scheme = ccx.tcx.lookup_item_type(item_def_id); let type_predicates = ccx.tcx.lookup_predicates(item_def_id); reject_non_type_param_bounds(ccx.tcx, item.span, &type_predicates); + let free_id_outlive = ccx.tcx.region_maps.item_extent(item.id); let param_env = ccx.tcx.construct_parameter_environment(item.span, &type_scheme.generics, &type_predicates, - item.id); + free_id_outlive); let tables = RefCell::new(ty::Tables::empty()); let inh = Inherited::new(ccx.tcx, &tables, param_env); let fcx = blank_fn_ctxt(ccx, &inh, ty::FnConverging(type_scheme.ty), item.id); diff --git a/src/librustc_typeck/check/wfcheck.rs b/src/librustc_typeck/check/wfcheck.rs index bfbf8fff4f537..809cbb88bd6b3 100644 --- a/src/librustc_typeck/check/wfcheck.rs +++ b/src/librustc_typeck/check/wfcheck.rs @@ -13,6 +13,7 @@ use check::{FnCtxt, Inherited, blank_fn_ctxt, regionck}; use constrained_type_params::{identify_constrained_type_params, Parameter}; use CrateCtxt; use middle::def_id::DefId; +use middle::region::{CodeExtent}; use middle::subst::{self, TypeSpace, FnSpace, ParamSpace, SelfSpace}; use middle::traits; use middle::ty::{self, Ty}; @@ -134,7 +135,7 @@ impl<'ccx, 'tcx> CheckTypeWellFormedVisitor<'ccx, 'tcx> { let code = self.code.clone(); self.with_fcx(item_id, span, |fcx, this| { let free_substs = &fcx.inh.infcx.parameter_environment.free_substs; - let free_id = fcx.inh.infcx.parameter_environment.free_id; + let free_id_outlive = fcx.inh.infcx.parameter_environment.free_id_outlive; let item = fcx.tcx().impl_or_trait_item(fcx.tcx().map.local_def_id(item_id)); @@ -153,7 +154,7 @@ impl<'ccx, 'tcx> CheckTypeWellFormedVisitor<'ccx, 'tcx> { let method_ty = fcx.instantiate_type_scheme(span, free_substs, &method.fty); let predicates = fcx.instantiate_bounds(span, free_substs, &method.predicates); this.check_fn_or_method(fcx, span, &method_ty, &predicates, - free_id, &mut implied_bounds); + free_id_outlive, &mut implied_bounds); } ty::TypeTraitItem(assoc_type) => { if let Some(ref ty) = assoc_type.ty { @@ -263,8 +264,9 @@ impl<'ccx, 'tcx> CheckTypeWellFormedVisitor<'ccx, 'tcx> { let predicates = fcx.instantiate_bounds(item.span, free_substs, &predicates); let mut implied_bounds = vec![]; + let free_id_outlive = fcx.tcx().region_maps.call_site_extent(item.id, body.id); this.check_fn_or_method(fcx, item.span, bare_fn_ty, &predicates, - body.id, &mut implied_bounds); + free_id_outlive, &mut implied_bounds); implied_bounds }) } @@ -355,12 +357,11 @@ impl<'ccx, 'tcx> CheckTypeWellFormedVisitor<'ccx, 'tcx> { span: Span, fty: &ty::BareFnTy<'tcx>, predicates: &ty::InstantiatedPredicates<'tcx>, - free_id: ast::NodeId, + free_id_outlive: CodeExtent, implied_bounds: &mut Vec>) { let free_substs = &fcx.inh.infcx.parameter_environment.free_substs; let fty = fcx.instantiate_type_scheme(span, free_substs, fty); - let free_id_outlive = fcx.tcx().region_maps.item_extent(free_id); let sig = fcx.tcx().liberate_late_bound_regions(free_id_outlive, &fty.sig); for &input_ty in &sig.inputs { diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 36be59da6efb6..33a99bb0769c6 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -889,11 +889,13 @@ fn convert_item(ccx: &CrateCtxt, it: &hir::Item) { for impl_item in impl_items { if let hir::ImplItemKind::Method(ref sig, ref body) = impl_item.node { let body_id = body.id; + let body_scope = ccx.tcx.region_maps.call_site_extent(impl_item.id, body_id); check_method_self_type(ccx, &BindingRscope::new(), ccx.method_ty(impl_item.id), selfty, &sig.explicit_self, + body_scope, body_id); } } @@ -988,8 +990,16 @@ fn convert_item(ccx: &CrateCtxt, it: &hir::Item) { // This must be done after `collect_trait_methods` so that // we have a method type stored for every method. for trait_item in trait_items { - let sig = match trait_item.node { - hir::MethodTraitItem(ref sig, _) => sig, + let (sig, the_scope, the_id) = match trait_item.node { + hir::MethodTraitItem(ref sig, Some(ref body)) => { + let body_scope = + ccx.tcx.region_maps.call_site_extent(trait_item.id, body.id); + (sig, body_scope, body.id) + } + hir::MethodTraitItem(ref sig, None) => { + let item_scope = ccx.tcx.region_maps.item_extent(trait_item.id); + (sig, item_scope, it.id) + } _ => continue }; check_method_self_type(ccx, @@ -997,7 +1007,8 @@ fn convert_item(ccx: &CrateCtxt, it: &hir::Item) { ccx.method_ty(trait_item.id), tcx.mk_self_type(), &sig.explicit_self, - it.id) + the_scope, + the_id) } }, hir::ItemStruct(ref struct_def, _) => { @@ -2282,6 +2293,7 @@ fn check_method_self_type<'a, 'tcx, RS:RegionScope>( method_type: Rc>, required_type: Ty<'tcx>, explicit_self: &hir::ExplicitSelf, + body_scope: region::CodeExtent, body_id: ast::NodeId) { let tcx = ccx.tcx; @@ -2293,8 +2305,6 @@ fn check_method_self_type<'a, 'tcx, RS:RegionScope>( _ => typ, }; - let body_scope = tcx.region_maps.item_extent(body_id); - // "Required type" comes from the trait definition. It may // contain late-bound regions from the method, but not the // trait (since traits only have early-bound region From 5299441954b4678f23f3c0c0b2080b3430a2d028 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 10 Dec 2015 19:30:37 +0100 Subject: [PATCH 2/2] Regression tests for Issue 29793. --- .../region-borrow-params-issue-29793-big.rs | 84 +++++++ .../region-borrow-params-issue-29793-small.rs | 223 ++++++++++++++++++ 2 files changed, 307 insertions(+) create mode 100644 src/test/compile-fail/region-borrow-params-issue-29793-big.rs create mode 100644 src/test/compile-fail/region-borrow-params-issue-29793-small.rs diff --git a/src/test/compile-fail/region-borrow-params-issue-29793-big.rs b/src/test/compile-fail/region-borrow-params-issue-29793-big.rs new file mode 100644 index 0000000000000..887f7836ee143 --- /dev/null +++ b/src/test/compile-fail/region-borrow-params-issue-29793-big.rs @@ -0,0 +1,84 @@ +// 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. + +// Issue #29793, big regression test: do not let borrows of +// parameters to ever be returned (expanded with exploration of +// variations). +// +// This is the version of the test that actually exposed unsound +// behavior (because the improperly accepted closure was actually +// able to be invoked). + +struct WrapA(Option); + +impl WrapA { + fn new() -> WrapA { + WrapA(None) + } + fn set(mut self, f: F) -> Self { + self.0 = Some(f); + self + } +} + +struct WrapB(Option); + +impl WrapB { + fn new() -> WrapB { + WrapB(None) + } + fn set(mut self, f: F) -> Self { + self.0 = Some(f); + self + } +} + +trait DoStuff : Sized { + fn handle(self); +} + +impl DoStuff for WrapA + where F: FnMut(usize, usize) -> T, T: DoStuff { + fn handle(mut self) { + if let Some(ref mut f) = self.0 { + let x = f(1, 2); + let _foo = [0usize; 16]; + x.handle(); + } + } + } + +impl DoStuff for WrapB where F: FnMut(bool) -> usize { + fn handle(mut self) { + if let Some(ref mut f) = self.0 { + println!("{}", f(true)); + } + } +} + +impl WrapA + where F: FnMut(usize, usize) -> T, T: DoStuff { + fn handle_ref(&mut self) { + if let Some(ref mut f) = self.0 { + let x = f(1, 2); + } + } + } + +fn main() { + let mut w = WrapA::new().set(|x: usize, y: usize| { + WrapB::new().set(|t: bool| if t { x } else { y }) // (separate errors for `x` vs `y`) + //~^ ERROR `x` does not live long enough + //~| ERROR `y` does not live long enough + }); + + w.handle(); // This works + // w.handle_ref(); // This doesn't +} diff --git a/src/test/compile-fail/region-borrow-params-issue-29793-small.rs b/src/test/compile-fail/region-borrow-params-issue-29793-small.rs new file mode 100644 index 0000000000000..4fda8ec3f384e --- /dev/null +++ b/src/test/compile-fail/region-borrow-params-issue-29793-small.rs @@ -0,0 +1,223 @@ +// 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. + +// Issue #29793, small regression tests: do not let borrows of +// parameters to ever be returned (expanded with exploration of +// variations). + +// CLOSURES + +fn escaping_borrow_of_closure_params_1() { + let g = |x: usize, y:usize| { + let f = |t: bool| if t { x } else { y }; // (separate errors for `x` vs `y`) + //~^ ERROR `x` does not live long enough + //~| ERROR `y` does not live long enough + return f; + }; + + // We delberately do not call `g`; this small version of the test, + // after adding such a call, was (properly) rejected even when the + // system still suffered from issue #29793. + + // g(10, 20)(true); +} + +fn escaping_borrow_of_closure_params_2() { + let g = |x: usize, y:usize| { + let f = |t: bool| if t { x } else { y }; // (separate errors for `x` vs `y`) + //~^ ERROR `x` does not live long enough + //~| ERROR `y` does not live long enough + f + }; + + // (we don't call `g`; see above) +} + +fn move_of_closure_params() { + let g = |x: usize, y:usize| { + let f = move |t: bool| if t { x } else { y }; + f; + }; + // (this code is fine, so lets go ahead and ensure rustc accepts call of `g`) + (g(1,2)); +} + +fn ok_borrow_of_fn_params(a: usize, b:usize) { + let g = |x: usize, y:usize| { + let f = |t: bool| if t { a } else { b }; + return f; + }; + // (this code is fine, so lets go ahead and ensure rustc accepts call of `g`) + (g(1,2))(true); +} + +// TOP-LEVEL FN'S + +fn escaping_borrow_of_fn_params_1() { + fn g<'a>(x: usize, y:usize) -> Box usize + 'a> { + let f = |t: bool| if t { x } else { y }; // (separate errors for `x` vs `y`) + //~^ ERROR E0373 + //~| ERROR E0373 + return Box::new(f); + }; + + // (we don't call `g`; see above) +} + +fn escaping_borrow_of_fn_params_2() { + fn g<'a>(x: usize, y:usize) -> Box usize + 'a> { + let f = |t: bool| if t { x } else { y }; // (separate errors for `x` vs `y`) + //~^ ERROR E0373 + //~| ERROR E0373 + Box::new(f) + }; + + // (we don't call `g`; see above) +} + +fn move_of_fn_params() { + fn g<'a>(x: usize, y:usize) -> Box usize + 'a> { + let f = move |t: bool| if t { x } else { y }; + return Box::new(f); + }; + // (this code is fine, so lets go ahead and ensure rustc accepts call of `g`) + (g(1,2))(true); +} + +// INHERENT METHODS + +fn escaping_borrow_of_method_params_1() { + struct S; + impl S { + fn g<'a>(&self, x: usize, y:usize) -> Box usize + 'a> { + let f = |t: bool| if t { x } else { y }; // (separate errors for `x` vs `y`) + //~^ ERROR E0373 + //~| ERROR E0373 + return Box::new(f); + } + } + + // (we don't call `g`; see above) +} + +fn escaping_borrow_of_method_params_2() { + struct S; + impl S { + fn g<'a>(&self, x: usize, y:usize) -> Box usize + 'a> { + let f = |t: bool| if t { x } else { y }; // (separate errors for `x` vs `y`) + //~^ ERROR E0373 + //~| ERROR E0373 + Box::new(f) + } + } + // (we don't call `g`; see above) +} + +fn move_of_method_params() { + struct S; + impl S { + fn g<'a>(&self, x: usize, y:usize) -> Box usize + 'a> { + let f = move |t: bool| if t { x } else { y }; + return Box::new(f); + } + } + // (this code is fine, so lets go ahead and ensure rustc accepts call of `g`) + (S.g(1,2))(true); +} + +// TRAIT IMPL METHODS + +fn escaping_borrow_of_trait_impl_params_1() { + trait T { fn g<'a>(&self, x: usize, y:usize) -> Box usize + 'a>; } + struct S; + impl T for S { + fn g<'a>(&self, x: usize, y:usize) -> Box usize + 'a> { + let f = |t: bool| if t { x } else { y }; // (separate errors for `x` vs `y`) + //~^ ERROR E0373 + //~| ERROR E0373 + return Box::new(f); + } + } + + // (we don't call `g`; see above) +} + +fn escaping_borrow_of_trait_impl_params_2() { + trait T { fn g<'a>(&self, x: usize, y:usize) -> Box usize + 'a>; } + struct S; + impl T for S { + fn g<'a>(&self, x: usize, y:usize) -> Box usize + 'a> { + let f = |t: bool| if t { x } else { y }; // (separate errors for `x` vs `y`) + //~^ ERROR E0373 + //~| ERROR E0373 + Box::new(f) + } + } + // (we don't call `g`; see above) +} + +fn move_of_trait_impl_params() { + trait T { fn g<'a>(&self, x: usize, y:usize) -> Box usize + 'a>; } + struct S; + impl T for S { + fn g<'a>(&self, x: usize, y:usize) -> Box usize + 'a> { + let f = move |t: bool| if t { x } else { y }; + return Box::new(f); + } + } + // (this code is fine, so lets go ahead and ensure rustc accepts call of `g`) + (S.g(1,2))(true); +} + +// TRAIT DEFAULT METHODS + +fn escaping_borrow_of_trait_default_params_1() { + struct S; + trait T { + fn g<'a>(&self, x: usize, y:usize) -> Box usize + 'a> { + let f = |t: bool| if t { x } else { y }; // (separate errors for `x` vs `y`) + //~^ ERROR E0373 + //~| ERROR E0373 + return Box::new(f); + } + } + impl T for S {} + // (we don't call `g`; see above) +} + +fn escaping_borrow_of_trait_default_params_2() { + struct S; + trait T { + fn g<'a>(&self, x: usize, y:usize) -> Box usize + 'a> { + let f = |t: bool| if t { x } else { y }; // (separate errors for `x` vs `y`) + //~^ ERROR E0373 + //~| ERROR E0373 + Box::new(f) + } + } + impl T for S {} + // (we don't call `g`; see above) +} + +fn move_of_trait_default_params() { + struct S; + trait T { + fn g<'a>(&self, x: usize, y:usize) -> Box usize + 'a> { + let f = move |t: bool| if t { x } else { y }; + return Box::new(f); + } + } + impl T for S {} + // (this code is fine, so lets go ahead and ensure rustc accepts call of `g`) + (S.g(1,2))(true); +} + +fn main() { } +