From b3a482ca9b1622a2fd04d8338ac99f5df802b9e2 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 16 Feb 2017 12:46:44 -0500 Subject: [PATCH] move the `FreeRegionMap` into `TypeckTables` --- src/librustc/middle/free_region.rs | 6 ++- src/librustc/ty/context.rs | 24 +++------ src/librustc_borrowck/borrowck/mod.rs | 51 +++++++------------ src/librustc_data_structures/lib.rs | 1 + .../transitive_relation.rs | 33 +++++++++++- src/librustc_typeck/check/regionck.rs | 12 +++-- src/librustc_typeck/check/writeback.rs | 5 ++ 7 files changed, 75 insertions(+), 57 deletions(-) diff --git a/src/librustc/middle/free_region.rs b/src/librustc/middle/free_region.rs index bd35bfc9829..cdb081ab400 100644 --- a/src/librustc/middle/free_region.rs +++ b/src/librustc/middle/free_region.rs @@ -19,7 +19,7 @@ use ty::{self, TyCtxt, FreeRegion, Region}; use ty::wf::ImpliedBound; use rustc_data_structures::transitive_relation::TransitiveRelation; -#[derive(Clone)] +#[derive(Clone, RustcEncodable, RustcDecodable)] pub struct FreeRegionMap { // Stores the relation `a < b`, where `a` and `b` are regions. relation: TransitiveRelation @@ -30,6 +30,10 @@ impl FreeRegionMap { FreeRegionMap { relation: TransitiveRelation::new() } } + pub fn is_empty(&self) -> bool { + self.relation.is_empty() + } + pub fn relate_free_regions_from_implied_bounds<'tcx>(&mut self, implied_bounds: &[ImpliedBound<'tcx>]) { diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 6961e0da362..a0aeb4107c1 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -248,6 +248,11 @@ pub struct TypeckTables<'tcx> { /// If any errors occurred while type-checking this body, /// this field will be set to `true`. pub tainted_by_errors: bool, + + /// Stores the free-region relationships that were deduced from + /// its where clauses and parameter types. These are then + /// read-again by borrowck. + pub free_region_map: FreeRegionMap, } impl<'tcx> TypeckTables<'tcx> { @@ -267,6 +272,7 @@ impl<'tcx> TypeckTables<'tcx> { lints: lint::LintTable::new(), used_trait_imports: DefIdSet(), tainted_by_errors: false, + free_region_map: FreeRegionMap::new(), } } @@ -414,13 +420,6 @@ pub struct GlobalCtxt<'tcx> { pub region_maps: RegionMaps, - // For each fn declared in the local crate, type check stores the - // free-region relationships that were deduced from its where - // clauses and parameter types. These are then read-again by - // borrowck. (They are not used during trans, and hence are not - // serialized or needed for cross-crate fns.) - free_region_maps: RefCell>, - pub hir: hir_map::Map<'tcx>, pub maps: maps::Maps<'tcx>, @@ -645,16 +644,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { interned } - pub fn store_free_region_map(self, id: NodeId, map: FreeRegionMap) { - if self.free_region_maps.borrow_mut().insert(id, map).is_some() { - bug!("Tried to overwrite interned FreeRegionMap for NodeId {:?}", id) - } - } - - pub fn free_region_map(self, id: NodeId) -> FreeRegionMap { - self.free_region_maps.borrow()[&id].clone() - } - pub fn lift>(self, value: &T) -> Option { value.lift_to_tcx(self) } @@ -707,7 +696,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { types: common_types, named_region_map: named_region_map, region_maps: region_maps, - free_region_maps: RefCell::new(FxHashMap()), variance_computed: Cell::new(false), trait_map: resolutions.trait_map, fulfilled_predicates: RefCell::new(fulfilled_predicates), diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index 46179b31d5c..06133de0757 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -32,14 +32,12 @@ use rustc::middle::dataflow::DataFlowOperator; use rustc::middle::dataflow::KillFrom; use rustc::hir::def_id::DefId; use rustc::middle::expr_use_visitor as euv; -use rustc::middle::free_region::FreeRegionMap; use rustc::middle::mem_categorization as mc; use rustc::middle::mem_categorization::Categorization; use rustc::middle::region; use rustc::ty::{self, TyCtxt}; use std::fmt; -use std::mem; use std::rc::Rc; use std::hash::{Hash, Hasher}; use syntax::ast; @@ -72,9 +70,7 @@ impl<'a, 'tcx> Visitor<'tcx> for BorrowckCtxt<'a, 'tcx> { match fk { FnKind::ItemFn(..) | FnKind::Method(..) => { - self.with_temp_region_map(id, |this| { - borrowck_fn(this, fk, fd, b, s, id, fk.attrs()) - }); + borrowck_fn(self, fk, fd, b, s, id, fk.attrs()) } FnKind::Closure(..) => { @@ -105,7 +101,7 @@ impl<'a, 'tcx> Visitor<'tcx> for BorrowckCtxt<'a, 'tcx> { pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { let mut bccx = BorrowckCtxt { tcx: tcx, - free_region_map: FreeRegionMap::new(), + tables: None, stats: BorrowStats { loaned_paths_same: 0, loaned_paths_imm: 0, @@ -167,12 +163,15 @@ fn borrowck_fn<'a, 'tcx>(this: &mut BorrowckCtxt<'a, 'tcx>, attributes: &[ast::Attribute]) { debug!("borrowck_fn(id={})", id); + assert!(this.tables.is_none()); + let owner_def_id = this.tcx.hir.local_def_id(this.tcx.hir.body_owner(body_id)); + let tables = this.tcx.item_tables(owner_def_id); + this.tables = Some(tables); + let body = this.tcx.hir.body(body_id); if attributes.iter().any(|item| item.check_name("rustc_mir_borrowck")) { - this.with_temp_region_map(id, |this| { - mir::borrowck_mir(this, id, attributes) - }); + mir::borrowck_mir(this, id, attributes); } let cfg = cfg::CFG::new(this.tcx, &body.value); @@ -191,6 +190,8 @@ fn borrowck_fn<'a, 'tcx>(this: &mut BorrowckCtxt<'a, 'tcx>, check_loans::check_loans(this, &loan_dfcx, &flowed_moves, &all_loans[..], body); + this.tables = None; + intravisit::walk_fn(this, fk, decl, body_id, sp, id); } @@ -248,7 +249,7 @@ pub fn build_borrowck_dataflow_data_for_fn<'a, 'tcx>( let mut bccx = BorrowckCtxt { tcx: tcx, - free_region_map: FreeRegionMap::new(), + tables: None, stats: BorrowStats { loaned_paths_same: 0, loaned_paths_imm: 0, @@ -267,17 +268,9 @@ pub fn build_borrowck_dataflow_data_for_fn<'a, 'tcx>( pub struct BorrowckCtxt<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, - // Hacky. As we visit various fns, we have to load up the - // free-region map for each one. This map is computed by during - // typeck for each fn item and stored -- closures just use the map - // from the fn item that encloses them. Since we walk the fns in - // order, we basically just overwrite this field as we enter a fn - // item and restore it afterwards in a stack-like fashion. Then - // the borrow checking code can assume that `free_region_map` is - // always the correct map for the current fn. Feels like it'd be - // better to just recompute this, rather than store it, but it's a - // bit of a pain to factor that code out at the moment. - free_region_map: FreeRegionMap, + // tables for the current thing we are checking; set to + // Some in `borrowck_fn` and cleared later + tables: Option<&'a ty::TypeckTables<'tcx>>, // Statistics: stats: BorrowStats @@ -574,19 +567,13 @@ pub enum MovedValueUseKind { // Misc impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { - fn with_temp_region_map(&mut self, id: ast::NodeId, f: F) - where F: for <'b> FnOnce(&'b mut BorrowckCtxt<'a, 'tcx>) - { - let new_free_region_map = self.tcx.free_region_map(id); - let old_free_region_map = mem::replace(&mut self.free_region_map, new_free_region_map); - f(self); - self.free_region_map = old_free_region_map; - } - - pub fn is_subregion_of(&self, r_sub: &'tcx ty::Region, r_sup: &'tcx ty::Region) + pub fn is_subregion_of(&self, + r_sub: &'tcx ty::Region, + r_sup: &'tcx ty::Region) -> bool { - self.free_region_map.is_subregion_of(self.tcx, r_sub, r_sup) + self.tables.unwrap().free_region_map + .is_subregion_of(self.tcx, r_sub, r_sup) } pub fn report(&self, err: BckError<'tcx>) { diff --git a/src/librustc_data_structures/lib.rs b/src/librustc_data_structures/lib.rs index cf6bf1cf1d4..3dce4398f3b 100644 --- a/src/librustc_data_structures/lib.rs +++ b/src/librustc_data_structures/lib.rs @@ -28,6 +28,7 @@ #![feature(shared)] #![feature(collections_range)] #![feature(collections_bound)] +#![cfg_attr(stage0,feature(field_init_shorthand))] #![feature(nonzero)] #![feature(rustc_private)] #![feature(staged_api)] diff --git a/src/librustc_data_structures/transitive_relation.rs b/src/librustc_data_structures/transitive_relation.rs index e09e260afc8..2bce7faf08c 100644 --- a/src/librustc_data_structures/transitive_relation.rs +++ b/src/librustc_data_structures/transitive_relation.rs @@ -9,6 +9,7 @@ // except according to those terms. use bitvec::BitMatrix; +use rustc_serialize::{Encodable, Encoder, Decodable, Decoder}; use std::cell::RefCell; use std::fmt::Debug; use std::mem; @@ -36,10 +37,10 @@ pub struct TransitiveRelation { closure: RefCell>, } -#[derive(Clone, PartialEq, PartialOrd)] +#[derive(Clone, PartialEq, PartialOrd, RustcEncodable, RustcDecodable)] struct Index(usize); -#[derive(Clone, PartialEq)] +#[derive(Clone, PartialEq, RustcEncodable, RustcDecodable)] struct Edge { source: Index, target: Index, @@ -54,6 +55,10 @@ impl TransitiveRelation { } } + pub fn is_empty(&self) -> bool { + self.edges.is_empty() + } + fn index(&self, a: &T) -> Option { self.elements.iter().position(|e| *e == *a).map(Index) } @@ -305,6 +310,30 @@ fn pare_down(candidates: &mut Vec, closure: &BitMatrix) { } } +impl Encodable for TransitiveRelation + where T: Encodable + Debug + PartialEq +{ + fn encode(&self, s: &mut E) -> Result<(), E::Error> { + s.emit_struct("TransitiveRelation", 2, |s| { + s.emit_struct_field("elements", 0, |s| self.elements.encode(s))?; + s.emit_struct_field("edges", 1, |s| self.edges.encode(s))?; + Ok(()) + }) + } +} + +impl Decodable for TransitiveRelation + where T: Decodable + Debug + PartialEq +{ + fn decode(d: &mut D) -> Result { + d.read_struct("TransitiveRelation", 2, |d| { + let elements = d.read_struct_field("elements", 0, |d| Decodable::decode(d))?; + let edges = d.read_struct_field("edges", 1, |d| Decodable::decode(d))?; + Ok(TransitiveRelation { elements, edges, closure: RefCell::new(None) }) + }) + } +} + #[test] fn test_one_step() { let mut relation = TransitiveRelation::new(); diff --git a/src/librustc_typeck/check/regionck.rs b/src/librustc_typeck/check/regionck.rs index e1067d299fa..8bfb390bd2a 100644 --- a/src/librustc_typeck/check/regionck.rs +++ b/src/librustc_typeck/check/regionck.rs @@ -120,6 +120,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { rcx.visit_region_obligations(id); } rcx.resolve_regions_and_report_errors(); + + assert!(self.tables.borrow().free_region_map.is_empty()); + self.tables.borrow_mut().free_region_map = rcx.free_region_map; } /// Region checking during the WF phase for items. `wf_tys` are the @@ -154,10 +157,11 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { rcx.resolve_regions_and_report_errors(); - // For the top-level fn, store the free-region-map. We don't store - // any map for closures; they just share the same map as the - // function that created them. - self.tcx.store_free_region_map(fn_id, rcx.free_region_map); + // In this mode, we also copy the free-region-map into the + // tables of the enclosing fcx. In the other regionck modes + // (e.g., `regionck_item`), we don't have an enclosing tables. + assert!(self.tables.borrow().free_region_map.is_empty()); + self.tables.borrow_mut().free_region_map = rcx.free_region_map; } } diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs index 1382ab34ca5..7fffbd14e21 100644 --- a/src/librustc_typeck/check/writeback.rs +++ b/src/librustc_typeck/check/writeback.rs @@ -50,6 +50,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { wbcx.visit_type_nodes(); wbcx.visit_cast_types(); wbcx.visit_lints(); + wbcx.visit_free_region_map(); let used_trait_imports = mem::replace(&mut self.tables.borrow_mut().used_trait_imports, DefIdSet()); @@ -274,6 +275,10 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> { self.fcx.tables.borrow_mut().lints.transfer(&mut self.tables.lints); } + fn visit_free_region_map(&mut self) { + self.tables.free_region_map = self.fcx.tables.borrow().free_region_map.clone(); + } + fn visit_anon_types(&mut self) { let gcx = self.tcx().global_tcx(); for (&node_id, &concrete_ty) in self.fcx.anon_types.borrow().iter() {