Skip to content

Commit

Permalink
Rollup merge of rust-lang#38418 - michaelwoerister:def_path_cleanup, …
Browse files Browse the repository at this point in the history 
…r=eddyb

Cleanup refactoring around DefPath handling

This PR makes two big changes:
* All DefPaths of a crate are now stored in metadata in their own table (as opposed to `DefKey`s as part of metadata `Entry`s.
* The compiler will no longer allocate a pseudo-local DefId for inlined HIR nodes (because those are gross). Inlined HIR nodes will have a NodeId but they don't have there own DefId anymore. Turns out they were not needed anymore either. Hopefully HIR inlining will be gone completely one day but if until then we start needing to be able to map inlined NodeIds to original DefIds, we can add an additional table to metadata that allows for reconstructing this.

Overall this makes for some nice simplifications and removal of special cases.

r? @eddyb

cc @rust-lang/compiler
  • Loading branch information
@alexcrichton
alexcrichton committed Dec 20, 2016
2 parents 2104111 + 3a82b0d commit 21f33db
Show file tree
Hide file tree
Showing 23 changed files with 238 additions and 532 deletions.
6 changes: 2 additions & 4 deletions src/librustc/dep_graph/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ to see something like:

Hir(foo) -> Collect(bar)
Collect(bar) -> TypeckItemBody(bar)

That first edge looks suspicious to you. So you set
`RUST_FORBID_DEP_GRAPH_EDGE` to `Hir&foo -> Collect&bar`, re-run, and
then observe the backtrace. Voila, bug fixed!
Expand All @@ -440,6 +440,4 @@ To achieve this, the HIR map will detect if the def-id originates in
an inlined node and add a dependency to a suitable `MetaData` node
instead. If you are reading a HIR node and are not sure if it may be
inlined or not, you can use `tcx.map.read(node_id)` and it will detect
whether the node is inlined or not and do the right thing. You can
also use `tcx.map.is_inlined_def_id()` and
`tcx.map.is_inlined_node_id()` to test.
whether the node is inlined or not and do the right thing.
2 changes: 0 additions & 2 deletions src/librustc/dep_graph/visit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ pub fn visit_all_item_likes_in_krate<'a, 'tcx, V, F>(tcx: TyCtxt<'a, 'tcx, 'tcx>
let task_id = (self.dep_node_fn)(item_def_id);
let _task = self.tcx.dep_graph.in_task(task_id.clone());
debug!("Started task {:?}", task_id);
assert!(!self.tcx.map.is_inlined_def_id(item_def_id));
self.tcx.dep_graph.read(DepNode::Hir(item_def_id));
self.visitor.visit_item(i);
debug!("Ended task {:?}", task_id);
Expand All @@ -51,7 +50,6 @@ pub fn visit_all_item_likes_in_krate<'a, 'tcx, V, F>(tcx: TyCtxt<'a, 'tcx, 'tcx>
let task_id = (self.dep_node_fn)(impl_item_def_id);
let _task = self.tcx.dep_graph.in_task(task_id.clone());
debug!("Started task {:?}", task_id);
assert!(!self.tcx.map.is_inlined_def_id(impl_item_def_id));
self.tcx.dep_graph.read(DepNode::Hir(impl_item_def_id));
self.visitor.visit_impl_item(i);
debug!("Ended task {:?}", task_id);
Expand Down
4 changes: 1 addition & 3 deletions src/librustc/hir/def_id.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,7 @@ impl fmt::Debug for DefId {

ty::tls::with_opt(|opt_tcx| {
if let Some(tcx) = opt_tcx {
if let Some(def_path) = tcx.opt_def_path(*self) {
write!(f, " => {}", def_path.to_string(tcx))?;
}
write!(f, " => {}", tcx.def_path(*self).to_string(tcx))?;
}
Ok(())
})?;
Expand Down
4 changes: 0 additions & 4 deletions src/librustc/hir/map/collector.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
use super::*;

use hir::intravisit::{Visitor, NestedVisitorMap};
use hir::def_id::DefId;
use middle::cstore::InlinedItem;
use std::iter::repeat;
use syntax::ast::{NodeId, CRATE_NODE_ID};
Expand Down Expand Up @@ -47,8 +46,6 @@ impl<'ast> NodeCollector<'ast> {
pub fn extend(krate: &'ast Crate,
parent: &'ast InlinedItem,
parent_node: NodeId,
parent_def_path: DefPath,
parent_def_id: DefId,
map: Vec<MapEntry<'ast>>)
-> NodeCollector<'ast> {
let mut collector = NodeCollector {
Expand All @@ -58,7 +55,6 @@ impl<'ast> NodeCollector<'ast> {
ignore_nested_items: true
};

assert_eq!(parent_def_path.krate, parent_def_id.krate);
collector.insert_entry(parent_node, RootInlinedParent(parent));

collector
Expand Down
213 changes: 1 addition & 212 deletions src/librustc/hir/map/def_collector.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,7 @@
// except according to those terms.

use hir::map::definitions::*;

use hir;
use hir::intravisit::{self, Visitor, NestedVisitorMap};
use hir::def_id::{CRATE_DEF_INDEX, DefId, DefIndex};

use middle::cstore::InlinedItem;
use hir::def_id::{CRATE_DEF_INDEX, DefIndex};

use syntax::ast::*;
use syntax::ext::hygiene::Mark;
Expand All @@ -23,9 +18,6 @@ use syntax::symbol::{Symbol, keywords};

/// Creates def ids for nodes in the HIR.
pub struct DefCollector<'a> {
// If we are walking HIR (c.f., AST), we need to keep a reference to the
// crate.
hir_crate: Option<&'a hir::Crate>,
definitions: &'a mut Definitions,
parent_def: Option<DefIndex>,
pub visit_macro_invoc: Option<&'a mut FnMut(MacroInvocationData)>,
Expand All @@ -40,43 +32,16 @@ pub struct MacroInvocationData {
impl<'a> DefCollector<'a> {
pub fn new(definitions: &'a mut Definitions) -> Self {
DefCollector {
hir_crate: None,
definitions: definitions,
parent_def: None,
visit_macro_invoc: None,
}
}

pub fn extend(parent_node: NodeId,
parent_def_path: DefPath,
parent_def_id: DefId,
definitions: &'a mut Definitions)
-> Self {
let mut collector = DefCollector::new(definitions);

assert_eq!(parent_def_path.krate, parent_def_id.krate);
let root_path = Box::new(InlinedRootPath {
data: parent_def_path.data,
def_id: parent_def_id,
});

let def = collector.create_def(parent_node, DefPathData::InlinedRoot(root_path));
collector.parent_def = Some(def);

collector
}

pub fn collect_root(&mut self) {
let root = self.create_def_with_parent(None, CRATE_NODE_ID, DefPathData::CrateRoot);
assert_eq!(root, CRATE_DEF_INDEX);
self.parent_def = Some(root);

self.create_def_with_parent(Some(CRATE_DEF_INDEX), DUMMY_NODE_ID, DefPathData::Misc);
}

pub fn walk_item(&mut self, ii: &'a InlinedItem, krate: &'a hir::Crate) {
self.hir_crate = Some(krate);
ii.visit(self);
}

fn create_def(&mut self, node_id: NodeId, data: DefPathData) -> DefIndex {
Expand Down Expand Up @@ -114,16 +79,6 @@ impl<'a> DefCollector<'a> {
self.create_def(expr.id, DefPathData::Initializer);
}

fn visit_hir_const_integer(&mut self, expr: &hir::Expr) {
// FIXME(eddyb) Closures should have separate
// function definition IDs and expression IDs.
if let hir::ExprClosure(..) = expr.node {
return;
}

self.create_def(expr.id, DefPathData::Initializer);
}

fn visit_macro_invoc(&mut self, id: NodeId, const_integer: bool) {
if let Some(ref mut visit) = self.visit_macro_invoc {
visit(MacroInvocationData {
Expand Down Expand Up @@ -324,169 +279,3 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> {
}
}
}

// We walk the HIR rather than the AST when reading items from metadata.
impl<'ast> Visitor<'ast> for DefCollector<'ast> {
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'ast> {
// note however that we override `visit_body` below
NestedVisitorMap::None
}

fn visit_body(&mut self, id: hir::ExprId) {
if let Some(krate) = self.hir_crate {
self.visit_expr(krate.expr(id));
}
}

fn visit_item(&mut self, i: &'ast hir::Item) {
debug!("visit_item: {:?}", i);

// Pick the def data. This need not be unique, but the more
// information we encapsulate into
let def_data = match i.node {
hir::ItemDefaultImpl(..) | hir::ItemImpl(..) =>
DefPathData::Impl,
hir::ItemEnum(..) | hir::ItemStruct(..) | hir::ItemUnion(..) |
hir::ItemTrait(..) | hir::ItemExternCrate(..) | hir::ItemMod(..) |
hir::ItemForeignMod(..) | hir::ItemTy(..) =>
DefPathData::TypeNs(i.name.as_str()),
hir::ItemStatic(..) | hir::ItemConst(..) | hir::ItemFn(..) =>
DefPathData::ValueNs(i.name.as_str()),
hir::ItemUse(..) => DefPathData::Misc,
};
let def = self.create_def(i.id, def_data);

self.with_parent(def, |this| {
match i.node {
hir::ItemEnum(ref enum_definition, _) => {
for v in &enum_definition.variants {
let variant_def_index =
this.create_def(v.node.data.id(),
DefPathData::EnumVariant(v.node.name.as_str()));

this.with_parent(variant_def_index, |this| {
for field in v.node.data.fields() {
this.create_def(field.id,
DefPathData::Field(field.name.as_str()));
}
if let Some(ref expr) = v.node.disr_expr {
this.visit_hir_const_integer(expr);
}
});
}
}
hir::ItemStruct(ref struct_def, _) |
hir::ItemUnion(ref struct_def, _) => {
// If this is a tuple-like struct, register the constructor.
if !struct_def.is_struct() {
this.create_def(struct_def.id(),
DefPathData::StructCtor);
}

for field in struct_def.fields() {
this.create_def(field.id, DefPathData::Field(field.name.as_str()));
}
}
_ => {}
}
intravisit::walk_item(this, i);
});
}

fn visit_foreign_item(&mut self, foreign_item: &'ast hir::ForeignItem) {
let def = self.create_def(foreign_item.id,
DefPathData::ValueNs(foreign_item.name.as_str()));

self.with_parent(def, |this| {
intravisit::walk_foreign_item(this, foreign_item);
});
}

fn visit_generics(&mut self, generics: &'ast hir::Generics) {
for ty_param in generics.ty_params.iter() {
self.create_def(ty_param.id, DefPathData::TypeParam(ty_param.name.as_str()));
}

intravisit::walk_generics(self, generics);
}

fn visit_trait_item(&mut self, ti: &'ast hir::TraitItem) {
let def_data = match ti.node {
hir::MethodTraitItem(..) | hir::ConstTraitItem(..) =>
DefPathData::ValueNs(ti.name.as_str()),
hir::TypeTraitItem(..) => DefPathData::TypeNs(ti.name.as_str()),
};

let def = self.create_def(ti.id, def_data);
self.with_parent(def, |this| {
if let hir::ConstTraitItem(_, Some(ref expr)) = ti.node {
this.create_def(expr.id, DefPathData::Initializer);
}

intravisit::walk_trait_item(this, ti);
});
}

fn visit_impl_item(&mut self, ii: &'ast hir::ImplItem) {
let def_data = match ii.node {
hir::ImplItemKind::Method(..) | hir::ImplItemKind::Const(..) =>
DefPathData::ValueNs(ii.name.as_str()),
hir::ImplItemKind::Type(..) => DefPathData::TypeNs(ii.name.as_str()),
};

let def = self.create_def(ii.id, def_data);
self.with_parent(def, |this| {
if let hir::ImplItemKind::Const(_, ref expr) = ii.node {
this.create_def(expr.id, DefPathData::Initializer);
}

intravisit::walk_impl_item(this, ii);
});
}

fn visit_pat(&mut self, pat: &'ast hir::Pat) {
let parent_def = self.parent_def;

if let hir::PatKind::Binding(_, _, name, _) = pat.node {
let def = self.create_def(pat.id, DefPathData::Binding(name.node.as_str()));
self.parent_def = Some(def);
}

intravisit::walk_pat(self, pat);
self.parent_def = parent_def;
}

fn visit_expr(&mut self, expr: &'ast hir::Expr) {
let parent_def = self.parent_def;

if let hir::ExprRepeat(_, ref count) = expr.node {
self.visit_hir_const_integer(count);
}

if let hir::ExprClosure(..) = expr.node {
let def = self.create_def(expr.id, DefPathData::ClosureExpr);
self.parent_def = Some(def);
}

intravisit::walk_expr(self, expr);
self.parent_def = parent_def;
}

fn visit_ty(&mut self, ty: &'ast hir::Ty) {
if let hir::TyArray(_, ref length) = ty.node {
self.visit_hir_const_integer(length);
}
if let hir::TyImplTrait(..) = ty.node {
self.create_def(ty.id, DefPathData::ImplTrait);
}
intravisit::walk_ty(self, ty);
}

fn visit_lifetime_def(&mut self, def: &'ast hir::LifetimeDef) {
self.create_def(def.lifetime.id, DefPathData::LifetimeDef(def.lifetime.name.as_str()));
}

fn visit_macro_def(&mut self, macro_def: &'ast hir::MacroDef) {
self.create_def(macro_def.id, DefPathData::MacroDef(macro_def.name.as_str()));
}
}
Loading

0 comments on commit 21f33db

Please sign in to comment.