From ada809917dc4b6f7a48fb60df06c3753bc8a74f3 Mon Sep 17 00:00:00 2001 From: cjkenn Date: Sun, 15 Oct 2017 14:50:40 -0700 Subject: [PATCH 1/4] Initial work to remove typeck_tables call from check_unused --- src/librustc/ty/context.rs | 6 +++--- src/librustc_typeck/check/method/mod.rs | 9 +++++++-- src/librustc_typeck/check/mod.rs | 9 ++++++++- src/librustc_typeck/check/writeback.rs | 4 +++- src/librustc_typeck/check_unused.rs | 7 ++++--- 5 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 24ba38cf14779..77889b7c10e86 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -388,7 +388,7 @@ pub struct TypeckTables<'tcx> { /// Set of trait imports actually used in the method resolution. /// This is used for warning unused imports. - pub used_trait_imports: DefIdSet, + pub used_trait_imports: Rc>, /// If any errors occurred while type-checking this body, /// this field will be set to `true`. @@ -418,7 +418,7 @@ impl<'tcx> TypeckTables<'tcx> { liberated_fn_sigs: ItemLocalMap(), fru_field_types: ItemLocalMap(), cast_kinds: ItemLocalMap(), - used_trait_imports: DefIdSet(), + used_trait_imports: Rc::new(RefCell::new(DefIdSet())), tainted_by_errors: false, free_region_map: FreeRegionMap::new(), } @@ -782,7 +782,7 @@ impl<'gcx> HashStable> for TypeckTables<'gcx> { cast_kinds.hash_stable(hcx, hasher); generator_sigs.hash_stable(hcx, hasher); generator_interiors.hash_stable(hcx, hasher); - used_trait_imports.hash_stable(hcx, hasher); + used_trait_imports.borrow_mut().hash_stable(hcx, hasher); tainted_by_errors.hash_stable(hcx, hasher); free_region_map.hash_stable(hcx, hasher); }) diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index d4eda13c6cd40..a0c93df80689c 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -163,7 +163,10 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if let Some(import_id) = pick.import_id { let import_def_id = self.tcx.hir.local_def_id(import_id); debug!("used_trait_import: {:?}", import_def_id); - self.tables.borrow_mut().used_trait_imports.insert(import_def_id); + + let tables = self.tables.borrow_mut(); + let mut ut = tables.used_trait_imports.borrow_mut(); + ut.insert(import_def_id); } self.tcx.check_stability(pick.item.def_id, call_expr.id, span); @@ -361,7 +364,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if let Some(import_id) = pick.import_id { let import_def_id = self.tcx.hir.local_def_id(import_id); debug!("used_trait_import: {:?}", import_def_id); - self.tables.borrow_mut().used_trait_imports.insert(import_def_id); + let tables = self.tables.borrow_mut(); + let mut ut = tables.used_trait_imports.borrow_mut(); + ut.insert(import_def_id); } let def = pick.item.def(); diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 0ebccbc835fc1..f6b4dd8470c0c 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -106,9 +106,10 @@ use session::{CompileIncomplete, Session}; use TypeAndSubsts; use lint; use util::common::{ErrorReported, indenter}; -use util::nodemap::{DefIdMap, FxHashMap, NodeMap}; +use util::nodemap::{DefIdMap, DefIdSet, FxHashMap, NodeMap}; use std::cell::{Cell, RefCell, Ref, RefMut}; +use std::rc::Rc; use std::collections::hash_map::Entry; use std::cmp; use std::fmt::Display; @@ -921,6 +922,12 @@ fn typeck_tables_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, tables } +pub fn get_used_trait_imports<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + def_id: DefId) + -> Rc> { + Rc::clone(&tcx.typeck_tables_of(def_id).used_trait_imports) +} + fn check_abi<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, span: Span, abi: Abi) { if !tcx.sess.target.target.is_abi_supported(abi) { struct_span_err!(tcx.sess, span, E0570, diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs index b3648d357e515..b6f2551da2077 100644 --- a/src/librustc_typeck/check/writeback.rs +++ b/src/librustc_typeck/check/writeback.rs @@ -23,6 +23,8 @@ use rustc::util::nodemap::DefIdSet; use syntax::ast; use syntax_pos::Span; use std::mem; +use std::rc::Rc; +use std::cell::RefCell; /////////////////////////////////////////////////////////////////////////// // Entry point @@ -49,7 +51,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { wbcx.visit_generator_interiors(); let used_trait_imports = mem::replace(&mut self.tables.borrow_mut().used_trait_imports, - DefIdSet()); + Rc::new(RefCell::new(DefIdSet()))); debug!("used_trait_imports({:?}) = {:?}", item_def_id, used_trait_imports); wbcx.tables.used_trait_imports = used_trait_imports; diff --git a/src/librustc_typeck/check_unused.rs b/src/librustc_typeck/check_unused.rs index 0c35b5e6834de..cd2bd8499cf38 100644 --- a/src/librustc_typeck/check_unused.rs +++ b/src/librustc_typeck/check_unused.rs @@ -19,6 +19,8 @@ use rustc::hir::itemlikevisit::ItemLikeVisitor; use rustc::hir; use rustc::util::nodemap::DefIdSet; +use check::get_used_trait_imports; + struct CheckVisitor<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, used_trait_imports: DefIdSet, @@ -66,10 +68,9 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { let mut used_trait_imports = DefIdSet(); for &body_id in tcx.hir.krate().bodies.keys() { let item_def_id = tcx.hir.body_owner_def_id(body_id); - let tables = tcx.typeck_tables_of(item_def_id); - let imports = &tables.used_trait_imports; + let imports = get_used_trait_imports(tcx, item_def_id); debug!("GatherVisitor: item_def_id={:?} with imports {:#?}", item_def_id, imports); - used_trait_imports.extend(imports); + used_trait_imports.extend(imports.borrow().iter()); } let mut visitor = CheckVisitor { tcx, used_trait_imports }; From b815ecc597a2457014dd9195e078ebe81c69a588 Mon Sep 17 00:00:00 2001 From: cjkenn Date: Mon, 16 Oct 2017 23:41:51 -0700 Subject: [PATCH 2/4] Add used_trait_imports query --- src/librustc/dep_graph/dep_node.rs | 1 + src/librustc/ty/context.rs | 9 +++++---- src/librustc/ty/maps/mod.rs | 2 ++ src/librustc/ty/maps/plumbing.rs | 1 + src/librustc_typeck/check/method/mod.rs | 13 ++++++------- src/librustc_typeck/check/mod.rs | 13 +++++++------ src/librustc_typeck/check/writeback.rs | 3 +-- src/librustc_typeck/check_unused.rs | 7 +++---- 8 files changed, 26 insertions(+), 23 deletions(-) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index 92bbb745bb214..7de557ad0ed0d 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -500,6 +500,7 @@ define_dep_nodes!( <'tcx> [] InherentImpls(DefId), [] TypeckBodiesKrate, [] TypeckTables(DefId), + [] UsedTraitImports(DefId), [] HasTypeckTables(DefId), [] ConstEval { param_env: ParamEnvAnd<'tcx, (DefId, &'tcx Substs<'tcx>)> }, [] SymbolName(DefId), diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 77889b7c10e86..8232c0ed46096 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -387,8 +387,9 @@ pub struct TypeckTables<'tcx> { cast_kinds: ItemLocalMap, /// Set of trait imports actually used in the method resolution. - /// This is used for warning unused imports. - pub used_trait_imports: Rc>, + /// This is used for warning unused imports. During type + /// checking, this field should not be cloned. + pub used_trait_imports: Rc, /// If any errors occurred while type-checking this body, /// this field will be set to `true`. @@ -418,7 +419,7 @@ impl<'tcx> TypeckTables<'tcx> { liberated_fn_sigs: ItemLocalMap(), fru_field_types: ItemLocalMap(), cast_kinds: ItemLocalMap(), - used_trait_imports: Rc::new(RefCell::new(DefIdSet())), + used_trait_imports: Rc::new(DefIdSet()), tainted_by_errors: false, free_region_map: FreeRegionMap::new(), } @@ -782,7 +783,7 @@ impl<'gcx> HashStable> for TypeckTables<'gcx> { cast_kinds.hash_stable(hcx, hasher); generator_sigs.hash_stable(hcx, hasher); generator_interiors.hash_stable(hcx, hasher); - used_trait_imports.borrow_mut().hash_stable(hcx, hasher); + used_trait_imports.hash_stable(hcx, hasher); tainted_by_errors.hash_stable(hcx, hasher); free_region_map.hash_stable(hcx, hasher); }) diff --git a/src/librustc/ty/maps/mod.rs b/src/librustc/ty/maps/mod.rs index f54391ebb07eb..f6a0194913046 100644 --- a/src/librustc/ty/maps/mod.rs +++ b/src/librustc/ty/maps/mod.rs @@ -183,6 +183,8 @@ define_maps! { <'tcx> [] fn typeck_tables_of: TypeckTables(DefId) -> &'tcx ty::TypeckTables<'tcx>, + [] fn used_trait_imports: UsedTraitImports(DefId) -> Rc, + [] fn has_typeck_tables: HasTypeckTables(DefId) -> bool, [] fn coherent_trait: coherent_trait_dep_node((CrateNum, DefId)) -> (), diff --git a/src/librustc/ty/maps/plumbing.rs b/src/librustc/ty/maps/plumbing.rs index 4e301342ee079..767e9a8436c2e 100644 --- a/src/librustc/ty/maps/plumbing.rs +++ b/src/librustc/ty/maps/plumbing.rs @@ -757,6 +757,7 @@ pub fn force_from_dep_node<'a, 'gcx, 'lcx>(tcx: TyCtxt<'a, 'gcx, 'lcx>, DepKind::InherentImpls => { force!(inherent_impls, def_id!()); } DepKind::TypeckBodiesKrate => { force!(typeck_item_bodies, LOCAL_CRATE); } DepKind::TypeckTables => { force!(typeck_tables_of, def_id!()); } + DepKind::UsedTraitImports => { force!(used_trait_imports, def_id!()); } DepKind::HasTypeckTables => { force!(has_typeck_tables, def_id!()); } DepKind::SymbolName => { force!(def_symbol_name, def_id!()); } DepKind::SpecializationGraph => { force!(specialization_graph_of, def_id!()); } diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index a0c93df80689c..58d72e37d51cf 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -25,6 +25,8 @@ use syntax_pos::Span; use rustc::hir; +use std::rc::Rc; + pub use self::MethodError::*; pub use self::CandidateSource::*; @@ -163,10 +165,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if let Some(import_id) = pick.import_id { let import_def_id = self.tcx.hir.local_def_id(import_id); debug!("used_trait_import: {:?}", import_def_id); - - let tables = self.tables.borrow_mut(); - let mut ut = tables.used_trait_imports.borrow_mut(); - ut.insert(import_def_id); + Rc::get_mut(&mut self.tables.borrow_mut().used_trait_imports) + .unwrap().insert(import_def_id); } self.tcx.check_stability(pick.item.def_id, call_expr.id, span); @@ -364,9 +364,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if let Some(import_id) = pick.import_id { let import_def_id = self.tcx.hir.local_def_id(import_id); debug!("used_trait_import: {:?}", import_def_id); - let tables = self.tables.borrow_mut(); - let mut ut = tables.used_trait_imports.borrow_mut(); - ut.insert(import_def_id); + Rc::get_mut(&mut self.tables.borrow_mut().used_trait_imports) + .unwrap().insert(import_def_id); } let def = pick.item.def(); diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index f6b4dd8470c0c..c4c8f65a994a8 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -743,6 +743,7 @@ pub fn provide(providers: &mut Providers) { closure_kind, generator_sig, adt_destructor, + used_trait_imports, ..*providers }; } @@ -846,6 +847,12 @@ fn has_typeck_tables<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, primary_body_of(tcx, id).is_some() } +fn used_trait_imports<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + def_id: DefId) + -> Rc { + Rc::clone(&tcx.typeck_tables_of(def_id).used_trait_imports) +} + fn typeck_tables_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx ty::TypeckTables<'tcx> { @@ -922,12 +929,6 @@ fn typeck_tables_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, tables } -pub fn get_used_trait_imports<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, - def_id: DefId) - -> Rc> { - Rc::clone(&tcx.typeck_tables_of(def_id).used_trait_imports) -} - fn check_abi<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, span: Span, abi: Abi) { if !tcx.sess.target.target.is_abi_supported(abi) { struct_span_err!(tcx.sess, span, E0570, diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs index b6f2551da2077..ce2ac73a27e0c 100644 --- a/src/librustc_typeck/check/writeback.rs +++ b/src/librustc_typeck/check/writeback.rs @@ -24,7 +24,6 @@ use syntax::ast; use syntax_pos::Span; use std::mem; use std::rc::Rc; -use std::cell::RefCell; /////////////////////////////////////////////////////////////////////////// // Entry point @@ -51,7 +50,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { wbcx.visit_generator_interiors(); let used_trait_imports = mem::replace(&mut self.tables.borrow_mut().used_trait_imports, - Rc::new(RefCell::new(DefIdSet()))); + Rc::new(DefIdSet())); debug!("used_trait_imports({:?}) = {:?}", item_def_id, used_trait_imports); wbcx.tables.used_trait_imports = used_trait_imports; diff --git a/src/librustc_typeck/check_unused.rs b/src/librustc_typeck/check_unused.rs index cd2bd8499cf38..29df70a4679c0 100644 --- a/src/librustc_typeck/check_unused.rs +++ b/src/librustc_typeck/check_unused.rs @@ -19,8 +19,6 @@ use rustc::hir::itemlikevisit::ItemLikeVisitor; use rustc::hir; use rustc::util::nodemap::DefIdSet; -use check::get_used_trait_imports; - struct CheckVisitor<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, used_trait_imports: DefIdSet, @@ -68,9 +66,10 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { let mut used_trait_imports = DefIdSet(); for &body_id in tcx.hir.krate().bodies.keys() { let item_def_id = tcx.hir.body_owner_def_id(body_id); - let imports = get_used_trait_imports(tcx, item_def_id); + // let tables = tcx.typeck_tables_of(item_def_id); + let imports = tcx.used_trait_imports(item_def_id); debug!("GatherVisitor: item_def_id={:?} with imports {:#?}", item_def_id, imports); - used_trait_imports.extend(imports.borrow().iter()); + used_trait_imports.extend(imports.iter()); } let mut visitor = CheckVisitor { tcx, used_trait_imports }; From 24b4dfcf1b9862d3a8fa15a36f1d39ae0971d9f9 Mon Sep 17 00:00:00 2001 From: cjkenn Date: Tue, 17 Oct 2017 18:18:07 -0700 Subject: [PATCH 3/4] Remove commented out line. --- src/librustc_typeck/check_unused.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc_typeck/check_unused.rs b/src/librustc_typeck/check_unused.rs index 29df70a4679c0..b867a655b4a6a 100644 --- a/src/librustc_typeck/check_unused.rs +++ b/src/librustc_typeck/check_unused.rs @@ -66,7 +66,6 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { let mut used_trait_imports = DefIdSet(); for &body_id in tcx.hir.krate().bodies.keys() { let item_def_id = tcx.hir.body_owner_def_id(body_id); - // let tables = tcx.typeck_tables_of(item_def_id); let imports = tcx.used_trait_imports(item_def_id); debug!("GatherVisitor: item_def_id={:?} with imports {:#?}", item_def_id, imports); used_trait_imports.extend(imports.iter()); From 6f30ce0be2240758889224781a9f727e7bc325e1 Mon Sep 17 00:00:00 2001 From: cjkenn Date: Thu, 19 Oct 2017 23:06:22 -0700 Subject: [PATCH 4/4] Misc code review changes --- src/librustc/ty/context.rs | 3 ++- src/librustc_typeck/check/mod.rs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 8232c0ed46096..80a9cda1e1832 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -388,7 +388,8 @@ pub struct TypeckTables<'tcx> { /// Set of trait imports actually used in the method resolution. /// This is used for warning unused imports. During type - /// checking, this field should not be cloned. + /// checking, this `Rc` should not be cloned: it must have a ref-count + /// of 1 so that we can insert things into the set mutably. pub used_trait_imports: Rc, /// If any errors occurred while type-checking this body, diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index c4c8f65a994a8..7807c4c735226 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -850,7 +850,7 @@ fn has_typeck_tables<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, fn used_trait_imports<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Rc { - Rc::clone(&tcx.typeck_tables_of(def_id).used_trait_imports) + tcx.typeck_tables_of(def_id).used_trait_imports.clone() } fn typeck_tables_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,