Skip to content

Commit

Permalink
rejigger how we handle used trait imports
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
nikomatsakis committed Feb 3, 2017
1 parent fdd7e3c commit 78f7ac5
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 11 deletions.
3 changes: 3 additions & 0 deletions src/librustc/dep_graph/dep_node.rs
Expand Up @@ -113,6 +113,7 @@ pub enum DepNode<D: Clone + Debug> {
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
Expand Down Expand Up @@ -162,6 +163,7 @@ impl<D: Clone + Debug> DepNode<D> {
AssociatedItemDefIds,
InherentImpls,
TypeckTables,
UsedTraitImports,
TraitImpls,
ReprHints,
}
Expand Down Expand Up @@ -230,6 +232,7 @@ impl<D: Clone + Debug> DepNode<D> {
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),
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/context.rs
Expand Up @@ -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<DefIdSet>,
pub used_trait_imports: RefCell<DepTrackingMap<maps::UsedTraitImports<'tcx>>>,

/// The set of external nominal types whose implementations have been read.
/// This is used for lazy resolution of methods.
Expand Down Expand Up @@ -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),
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/ty/maps.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -49,3 +50,4 @@ dep_map_ty! { Mir: Mir(DefId) -> &'tcx RefCell<mir::Mir<'tcx>> }
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 }
6 changes: 4 additions & 2 deletions src/librustc_typeck/check/method/mod.rs
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
8 changes: 7 additions & 1 deletion src/librustc_typeck/check/mod.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Vec<traits::DeferredObligation<'tcx>>>,

// 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<FxHashSet<DefId>>,
}

impl<'a, 'gcx, 'tcx> Deref for Inherited<'a, 'gcx, 'tcx> {
Expand Down Expand Up @@ -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()),
}
}

Expand Down
8 changes: 7 additions & 1 deletion src/librustc_typeck/check/writeback.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down
27 changes: 22 additions & 5 deletions src/librustc_typeck/check_unused.rs
Expand Up @@ -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;
}

Expand All @@ -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;
Expand All @@ -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);
}
1 change: 1 addition & 0 deletions src/librustc_typeck/lib.rs
Expand Up @@ -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)]
Expand Down

0 comments on commit 78f7ac5

Please sign in to comment.