From 78f7ac561c0b360650b36a175f39475bc237230f Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 28 Jan 2017 06:34:07 -0500 Subject: [PATCH] rejigger how we handle used trait imports The previous way was not friendly to incremental compilation. The new plan is to compute, for each body, a set of trait imports used in that body (slightly subtle: for a closure, we assign the trait imports to the enclosing fn). Then we walk all bodies and union these sets. The reason we do this is that we can save the individual sets in the incremental state, and then recompute only those sets that are needed. Before we were planning to save only the final union, but in that case if some components are invalidated we have to recompute *all* of them since we don't have enough information to "partly" invalidate a result. In truth, this set probably ought to be part of the `TypeckTables`; however, I opted not to do that because I don't want to have to save/restore the entire tables in the incremental state yet (since it contains a lot of `NodeId` references, and removing those is a significant refactoring). --- src/librustc/dep_graph/dep_node.rs | 3 +++ src/librustc/ty/context.rs | 4 ++-- src/librustc/ty/maps.rs | 2 ++ src/librustc_typeck/check/method/mod.rs | 6 ++++-- src/librustc_typeck/check/mod.rs | 8 +++++++- src/librustc_typeck/check/writeback.rs | 8 +++++++- src/librustc_typeck/check_unused.rs | 27 ++++++++++++++++++++----- src/librustc_typeck/lib.rs | 1 + 8 files changed, 48 insertions(+), 11 deletions(-) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index df6db366df5b3..006de1c06e2d9 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -113,6 +113,7 @@ pub enum DepNode { AssociatedItemDefIds(D), InherentImpls(D), TypeckTables(D), + UsedTraitImports(D), // The set of impls for a given trait. Ultimately, it would be // nice to get more fine-grained here (e.g., to include a @@ -162,6 +163,7 @@ impl DepNode { AssociatedItemDefIds, InherentImpls, TypeckTables, + UsedTraitImports, TraitImpls, ReprHints, } @@ -230,6 +232,7 @@ impl DepNode { AssociatedItemDefIds(ref d) => op(d).map(AssociatedItemDefIds), InherentImpls(ref d) => op(d).map(InherentImpls), TypeckTables(ref d) => op(d).map(TypeckTables), + UsedTraitImports(ref d) => op(d).map(UsedTraitImports), TraitImpls(ref d) => op(d).map(TraitImpls), TraitItems(ref d) => op(d).map(TraitItems), ReprHints(ref d) => op(d).map(ReprHints), diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index a3b81586738b5..7dcbe04caf82b 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -499,7 +499,7 @@ pub struct GlobalCtxt<'tcx> { /// Set of trait imports actually used in the method resolution. /// This is used for warning unused imports. - pub used_trait_imports: RefCell, + pub used_trait_imports: RefCell>>, /// The set of external nominal types whose implementations have been read. /// This is used for lazy resolution of methods. @@ -788,7 +788,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { inherent_impls: RefCell::new(DepTrackingMap::new(dep_graph.clone())), used_unsafe: RefCell::new(NodeSet()), used_mut_nodes: RefCell::new(NodeSet()), - used_trait_imports: RefCell::new(DefIdSet()), + used_trait_imports: RefCell::new(DepTrackingMap::new(dep_graph.clone())), populated_external_types: RefCell::new(DefIdSet()), populated_external_primitive_impls: RefCell::new(DefIdSet()), stability: RefCell::new(stability), diff --git a/src/librustc/ty/maps.rs b/src/librustc/ty/maps.rs index c0cf1d724273e..d7341d148b720 100644 --- a/src/librustc/ty/maps.rs +++ b/src/librustc/ty/maps.rs @@ -12,6 +12,7 @@ use dep_graph::{DepNode, DepTrackingMapConfig}; use hir::def_id::DefId; use mir; use ty::{self, Ty}; +use util::nodemap::DefIdSet; use std::cell::RefCell; use std::marker::PhantomData; @@ -49,3 +50,4 @@ dep_map_ty! { Mir: Mir(DefId) -> &'tcx RefCell> } dep_map_ty! { ClosureKinds: ItemSignature(DefId) -> ty::ClosureKind } dep_map_ty! { ClosureTypes: ItemSignature(DefId) -> ty::ClosureTy<'tcx> } dep_map_ty! { TypeckTables: TypeckTables(DefId) -> &'tcx ty::TypeckTables<'tcx> } +dep_map_ty! { UsedTraitImports: UsedTraitImports(DefId) -> DefIdSet } diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index f8b6fbc9e816b..eae8989bd342e 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -138,7 +138,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); - self.tcx.used_trait_imports.borrow_mut().insert(import_def_id); + debug!("used_trait_import: {:?}", import_def_id); + self.used_trait_imports.borrow_mut().insert(import_def_id); } self.tcx.check_stability(pick.item.def_id, call_expr.id, span); @@ -338,7 +339,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); - self.tcx.used_trait_imports.borrow_mut().insert(import_def_id); + debug!("used_trait_import: {:?}", import_def_id); + self.used_trait_imports.borrow_mut().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 573cbfcc3b062..1d9913cd96dc3 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -102,7 +102,7 @@ use CrateCtxt; use TypeAndSubsts; use lint; use util::common::{ErrorReported, indenter}; -use util::nodemap::{DefIdMap, FxHashMap, FxHashSet, NodeMap}; +use util::nodemap::{DefIdMap, DefIdSet, FxHashMap, FxHashSet, NodeMap}; use std::cell::{Cell, RefCell}; use std::cmp; @@ -179,6 +179,11 @@ pub struct Inherited<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { // Obligations which will have to be checked at the end of // type-checking, after all functions have been inferred. deferred_obligations: RefCell>>, + + // a set of trait import def-ids that we use during method + // resolution; during writeback, this is written into + // `tcx.used_trait_imports` for this item def-id + used_trait_imports: RefCell>, } impl<'a, 'gcx, 'tcx> Deref for Inherited<'a, 'gcx, 'tcx> { @@ -513,6 +518,7 @@ impl<'a, 'gcx, 'tcx> Inherited<'a, 'gcx, 'tcx> { deferred_cast_checks: RefCell::new(Vec::new()), anon_types: RefCell::new(DefIdMap()), deferred_obligations: RefCell::new(Vec::new()), + used_trait_imports: RefCell::new(DefIdSet()), } } diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs index 3a467c0296a52..9df0542f51fa1 100644 --- a/src/librustc_typeck/check/writeback.rs +++ b/src/librustc_typeck/check/writeback.rs @@ -19,9 +19,10 @@ use rustc::ty::{self, Ty, TyCtxt, MethodCall, MethodCallee}; use rustc::ty::adjustment; use rustc::ty::fold::{TypeFolder,TypeFoldable}; use rustc::infer::{InferCtxt, FixupError}; -use rustc::util::nodemap::DefIdMap; +use rustc::util::nodemap::{DefIdMap, DefIdSet}; use std::cell::Cell; +use std::mem; use syntax::ast; use syntax_pos::Span; @@ -56,6 +57,11 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let tables = self.tcx.alloc_tables(wbcx.tables); self.tcx.tables.borrow_mut().insert(item_def_id, tables); + + let used_trait_imports = mem::replace(&mut *self.used_trait_imports.borrow_mut(), + DefIdSet()); + debug!("used_trait_imports({:?}) = {:?}", item_def_id, used_trait_imports); + self.tcx.used_trait_imports.borrow_mut().insert(item_def_id, used_trait_imports); } } diff --git a/src/librustc_typeck/check_unused.rs b/src/librustc_typeck/check_unused.rs index c1ba53afad4e9..6dff6d57e4fac 100644 --- a/src/librustc_typeck/check_unused.rs +++ b/src/librustc_typeck/check_unused.rs @@ -17,19 +17,21 @@ use syntax_pos::{Span, DUMMY_SP}; use rustc::hir; use rustc::hir::itemlikevisit::ItemLikeVisitor; +use rustc::util::nodemap::DefIdSet; -struct UnusedTraitImportVisitor<'a, 'tcx: 'a> { +struct CheckVisitor<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, + used_trait_imports: DefIdSet, } -impl<'a, 'tcx> UnusedTraitImportVisitor<'a, 'tcx> { +impl<'a, 'tcx> CheckVisitor<'a, 'tcx> { fn check_import(&self, id: ast::NodeId, span: Span) { if !self.tcx.maybe_unused_trait_imports.contains(&id) { return; } let import_def_id = self.tcx.hir.local_def_id(id); - if self.tcx.used_trait_imports.borrow().contains(&import_def_id) { + if self.used_trait_imports.contains(&import_def_id) { return; } @@ -42,7 +44,7 @@ impl<'a, 'tcx> UnusedTraitImportVisitor<'a, 'tcx> { } } -impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for UnusedTraitImportVisitor<'a, 'tcx> { +impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for CheckVisitor<'a, 'tcx> { fn visit_item(&mut self, item: &hir::Item) { if item.vis == hir::Public || item.span == DUMMY_SP { return; @@ -61,6 +63,21 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for UnusedTraitImportVisitor<'a, 'tcx> { pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { let _task = tcx.dep_graph.in_task(DepNode::UnusedTraitCheck); - let mut visitor = UnusedTraitImportVisitor { tcx: tcx }; + + let mut used_trait_imports = DefIdSet(); + for &body_id in tcx.hir.krate().bodies.keys() { + let item_id = tcx.hir.body_owner(body_id); + let item_def_id = tcx.hir.local_def_id(item_id); + + // this will have been written by the main typeck pass + if let Some(imports) = tcx.used_trait_imports.borrow().get(&item_def_id) { + debug!("GatherVisitor: item_def_id={:?} with imports {:#?}", item_def_id, imports); + used_trait_imports.extend(imports); + } else { + debug!("GatherVisitor: item_def_id={:?} with no imports", item_def_id); + } + } + + let mut visitor = CheckVisitor { tcx, used_trait_imports }; tcx.hir.krate().visit_all_item_likes(&mut visitor); } diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs index 4ed116b88f6d9..f19a59a5d38ae 100644 --- a/src/librustc_typeck/lib.rs +++ b/src/librustc_typeck/lib.rs @@ -77,6 +77,7 @@ This API is completely unstable and subject to change. #![feature(box_patterns)] #![feature(box_syntax)] #![feature(conservative_impl_trait)] +#![feature(field_init_shorthand)] #![feature(loop_break_value)] #![feature(quote)] #![feature(rustc_diagnostic_macros)]