Skip to content

incr.comp.: Don't use Ident in DefPath because that's unstable across compilation sessions #42627

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 13, 2017
Merged

incr.comp.: Don't use Ident in DefPath because that's unstable across compilation sessions #42627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/librustc/hir/lowering.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2686,7 +2686,7 @@ impl<'a> LoweringContext<'a> {
let parent_def = self.parent_def.unwrap();
let def_id = {
let defs = self.resolver.definitions();
let def_path_data = DefPathData::Binding(Ident::with_empty_ctxt(name));
let def_path_data = DefPathData::Binding(name);
let def_index = defs
.create_def_with_parent(parent_def, id, def_path_data, REGULAR_SPACE, Mark::root());
DefId::local(def_index)
Expand Down
39 changes: 20 additions & 19 deletions src/librustc/hir/map/def_collector.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use syntax::ast::*;
use syntax::ext::hygiene::Mark;
use syntax::visit;
use syntax::symbol::keywords;
use syntax::symbol::Symbol;

use hir::map::{ITEM_LIKE_SPACE, REGULAR_SPACE};

Expand Down Expand Up @@ -103,14 +104,14 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> {
DefPathData::Impl,
ItemKind::Enum(..) | ItemKind::Struct(..) | ItemKind::Union(..) | ItemKind::Trait(..) |
ItemKind::ExternCrate(..) | ItemKind::ForeignMod(..) | ItemKind::Ty(..) =>
DefPathData::TypeNs(i.ident.modern()),
DefPathData::TypeNs(i.ident.name),
ItemKind::Mod(..) if i.ident == keywords::Invalid.ident() => {
return visit::walk_item(self, i);
}
ItemKind::Mod(..) => DefPathData::Module(i.ident.modern()),
ItemKind::Mod(..) => DefPathData::Module(i.ident.name),
ItemKind::Static(..) | ItemKind::Const(..) | ItemKind::Fn(..) =>
DefPathData::ValueNs(i.ident.modern()),
ItemKind::MacroDef(..) => DefPathData::MacroDef(i.ident.modern()),
DefPathData::ValueNs(i.ident.name),
ItemKind::MacroDef(..) => DefPathData::MacroDef(i.ident.name),
ItemKind::Mac(..) => return self.visit_macro_invoc(i.id, false),
ItemKind::GlobalAsm(..) => DefPathData::Misc,
ItemKind::Use(ref view_path) => {
Expand Down Expand Up @@ -138,13 +139,13 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> {
for v in &enum_definition.variants {
let variant_def_index =
this.create_def(v.node.data.id(),
DefPathData::EnumVariant(v.node.name.modern()),
DefPathData::EnumVariant(v.node.name.name),
REGULAR_SPACE);
this.with_parent(variant_def_index, |this| {
for (index, field) in v.node.data.fields().iter().enumerate() {
let ident = field.ident.map(Ident::modern)
.unwrap_or_else(|| Ident::from_str(&index.to_string()));
this.create_def(field.id, DefPathData::Field(ident), REGULAR_SPACE);
let name = field.ident.map(|ident| ident.name)
.unwrap_or_else(|| Symbol::intern(&index.to_string()));
this.create_def(field.id, DefPathData::Field(name), REGULAR_SPACE);
}

if let Some(ref expr) = v.node.disr_expr {
Expand All @@ -162,9 +163,9 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> {
}

for (index, field) in struct_def.fields().iter().enumerate() {
let ident = field.ident.map(Ident::modern)
.unwrap_or_else(|| Ident::from_str(&index.to_string()));
this.create_def(field.id, DefPathData::Field(ident), REGULAR_SPACE);
let name = field.ident.map(|ident| ident.name)
.unwrap_or_else(|| Symbol::intern(&index.to_string()));
this.create_def(field.id, DefPathData::Field(name), REGULAR_SPACE);
}
}
_ => {}
Expand All @@ -175,7 +176,7 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> {

fn visit_foreign_item(&mut self, foreign_item: &'a ForeignItem) {
let def = self.create_def(foreign_item.id,
DefPathData::ValueNs(foreign_item.ident.modern()),
DefPathData::ValueNs(foreign_item.ident.name),
REGULAR_SPACE);

self.with_parent(def, |this| {
Expand All @@ -186,7 +187,7 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> {
fn visit_generics(&mut self, generics: &'a Generics) {
for ty_param in generics.ty_params.iter() {
self.create_def(ty_param.id,
DefPathData::TypeParam(ty_param.ident.modern()),
DefPathData::TypeParam(ty_param.ident.name),
REGULAR_SPACE);
}

Expand All @@ -196,8 +197,8 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> {
fn visit_trait_item(&mut self, ti: &'a TraitItem) {
let def_data = match ti.node {
TraitItemKind::Method(..) | TraitItemKind::Const(..) =>
DefPathData::ValueNs(ti.ident.modern()),
TraitItemKind::Type(..) => DefPathData::TypeNs(ti.ident.modern()),
DefPathData::ValueNs(ti.ident.name),
TraitItemKind::Type(..) => DefPathData::TypeNs(ti.ident.name),
TraitItemKind::Macro(..) => return self.visit_macro_invoc(ti.id, false),
};

Expand All @@ -214,8 +215,8 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> {
fn visit_impl_item(&mut self, ii: &'a ImplItem) {
let def_data = match ii.node {
ImplItemKind::Method(..) | ImplItemKind::Const(..) =>
DefPathData::ValueNs(ii.ident.modern()),
ImplItemKind::Type(..) => DefPathData::TypeNs(ii.ident.modern()),
DefPathData::ValueNs(ii.ident.name),
ImplItemKind::Type(..) => DefPathData::TypeNs(ii.ident.name),
ImplItemKind::Macro(..) => return self.visit_macro_invoc(ii.id, false),
};

Expand All @@ -236,7 +237,7 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> {
PatKind::Mac(..) => return self.visit_macro_invoc(pat.id, false),
PatKind::Ident(_, id, _) => {
let def = self.create_def(pat.id,
DefPathData::Binding(id.node.modern()),
DefPathData::Binding(id.node.name),
REGULAR_SPACE);
self.parent_def = Some(def);
}
Expand Down Expand Up @@ -281,7 +282,7 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> {

fn visit_lifetime_def(&mut self, def: &'a LifetimeDef) {
self.create_def(def.lifetime.id,
DefPathData::LifetimeDef(def.lifetime.ident.modern()),
DefPathData::LifetimeDef(def.lifetime.ident.name),
REGULAR_SPACE);
}

Expand Down
139 changes: 72 additions & 67 deletions src/librustc/hir/map/definitions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ use rustc_data_structures::stable_hasher::StableHasher;
use serialize::{Encodable, Decodable, Encoder, Decoder};
use std::fmt::Write;
use std::hash::Hash;
use syntax::ast::{self, Ident};
use syntax::ext::hygiene::{Mark, SyntaxContext};
use syntax::ast;
use syntax::ext::hygiene::Mark;
use syntax::symbol::{Symbol, InternedString};
use ty::TyCtxt;
use util::nodemap::NodeMap;
Expand Down Expand Up @@ -248,7 +248,39 @@ impl DefKey {
// and the special "root_parent" below.
0u8.hash(&mut hasher);
parent_hash.hash(&mut hasher);
self.disambiguated_data.hash(&mut hasher);

let DisambiguatedDefPathData {
ref data,
disambiguator,
} = self.disambiguated_data;

::std::mem::discriminant(data).hash(&mut hasher);
match *data {
DefPathData::TypeNs(name) |
DefPathData::ValueNs(name) |
DefPathData::Module(name) |
DefPathData::MacroDef(name) |
DefPathData::TypeParam(name) |
DefPathData::LifetimeDef(name) |
DefPathData::EnumVariant(name) |
DefPathData::Binding(name) |
DefPathData::Field(name) |
DefPathData::GlobalMetaData(name) => {
(*name.as_str()).hash(&mut hasher);
}

DefPathData::Impl |
DefPathData::CrateRoot |
DefPathData::Misc |
DefPathData::ClosureExpr |
DefPathData::StructCtor |
DefPathData::Initializer |
DefPathData::ImplTrait |
DefPathData::Typeof => {}
};

disambiguator.hash(&mut hasher);

DefPathHash(hasher.finish())
}

Expand Down Expand Up @@ -354,7 +386,7 @@ impl DefPath {
}
}

#[derive(Clone, Debug, RustcEncodable, RustcDecodable)]
#[derive(Clone, Debug, Eq, PartialEq, Hash, RustcEncodable, RustcDecodable)]
pub enum DefPathData {
// Root: these should only be used for the root nodes, because
// they are treated specially by the `def_path` function.
Expand All @@ -368,31 +400,31 @@ pub enum DefPathData {
/// An impl
Impl,
/// Something in the type NS
TypeNs(Ident),
TypeNs(Symbol),
/// Something in the value NS
ValueNs(Ident),
ValueNs(Symbol),
/// A module declaration
Module(Ident),
Module(Symbol),
/// A macro rule
MacroDef(Ident),
MacroDef(Symbol),
/// A closure expression
ClosureExpr,

// Subportions of items
/// A type parameter (generic parameter)
TypeParam(Ident),
TypeParam(Symbol),
/// A lifetime definition
LifetimeDef(Ident),
LifetimeDef(Symbol),
/// A variant of a enum
EnumVariant(Ident),
EnumVariant(Symbol),
/// A struct field
Field(Ident),
Field(Symbol),
/// Implicit ctor for a tuple-like struct
StructCtor,
/// Initializer for a const
Initializer,
/// Pattern binding
Binding(Ident),
Binding(Symbol),
/// An `impl Trait` type node.
ImplTrait,
/// A `typeof` type node.
Expand All @@ -401,7 +433,7 @@ pub enum DefPathData {
/// GlobalMetaData identifies a piece of crate metadata that is global to
/// a whole crate (as opposed to just one item). GlobalMetaData components
/// are only supposed to show up right below the crate root.
GlobalMetaData(Ident)
GlobalMetaData(Symbol)
}

#[derive(Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Debug,
Expand Down Expand Up @@ -604,19 +636,19 @@ impl Definitions {
}

impl DefPathData {
pub fn get_opt_ident(&self) -> Option<Ident> {
pub fn get_opt_name(&self) -> Option<Symbol> {
use self::DefPathData::*;
match *self {
TypeNs(ident) |
ValueNs(ident) |
Module(ident) |
MacroDef(ident) |
TypeParam(ident) |
LifetimeDef(ident) |
EnumVariant(ident) |
Binding(ident) |
Field(ident) |
GlobalMetaData(ident) => Some(ident),
TypeNs(name) |
ValueNs(name) |
Module(name) |
MacroDef(name) |
TypeParam(name) |
LifetimeDef(name) |
EnumVariant(name) |
Binding(name) |
Field(name) |
GlobalMetaData(name) => Some(name),

Impl |
CrateRoot |
Expand All @@ -629,24 +661,20 @@ impl DefPathData {
}
}

pub fn get_opt_name(&self) -> Option<ast::Name> {
self.get_opt_ident().map(|ident| ident.name)
}

pub fn as_interned_str(&self) -> InternedString {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice not to have the duplication between this and get_opt_name.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice, but they are not quite the same...

use self::DefPathData::*;
let s = match *self {
TypeNs(ident) |
ValueNs(ident) |
Module(ident) |
MacroDef(ident) |
TypeParam(ident) |
LifetimeDef(ident) |
EnumVariant(ident) |
Binding(ident) |
Field(ident) |
GlobalMetaData(ident) => {
return ident.name.as_str();
TypeNs(name) |
ValueNs(name) |
Module(name) |
MacroDef(name) |
TypeParam(name) |
LifetimeDef(name) |
EnumVariant(name) |
Binding(name) |
Field(name) |
GlobalMetaData(name) => {
return name.as_str();
}

// note that this does not show up in user printouts
Expand All @@ -669,29 +697,6 @@ impl DefPathData {
}
}

impl Eq for DefPathData {}
impl PartialEq for DefPathData {
fn eq(&self, other: &DefPathData) -> bool {
::std::mem::discriminant(self) == ::std::mem::discriminant(other) &&
self.get_opt_ident() == other.get_opt_ident()
}
}

impl ::std::hash::Hash for DefPathData {
fn hash<H: ::std::hash::Hasher>(&self, hasher: &mut H) {
::std::mem::discriminant(self).hash(hasher);
if let Some(ident) = self.get_opt_ident() {
if ident.ctxt == SyntaxContext::empty() && ident.name == ident.name.interned() {
ident.name.as_str().hash(hasher)
} else {
// FIXME(jseyfried) implement stable hashing for idents with macros 2.0 hygiene info
ident.hash(hasher)
}
}
}
}


// We define the GlobalMetaDataKind enum with this macro because we want to
// make sure that we exhaustively iterate over all variants when registering
// the corresponding DefIndices in the DefTable.
Expand All @@ -712,7 +717,7 @@ macro_rules! define_global_metadata_kind {
definitions.create_def_with_parent(
CRATE_DEF_INDEX,
ast::DUMMY_NODE_ID,
DefPathData::GlobalMetaData(instance.ident()),
DefPathData::GlobalMetaData(instance.name()),
DefIndexAddressSpace::High,
Mark::root()
);
Expand All @@ -726,15 +731,15 @@ macro_rules! define_global_metadata_kind {
let def_key = DefKey {
parent: Some(CRATE_DEF_INDEX),
disambiguated_data: DisambiguatedDefPathData {
data: DefPathData::GlobalMetaData(self.ident()),
data: DefPathData::GlobalMetaData(self.name()),
disambiguator: 0,
}
};

def_path_table.key_to_index[&def_key]
}

fn ident(&self) -> Ident {
fn name(&self) -> Symbol {

let string = match *self {
$(
Expand All @@ -744,7 +749,7 @@ macro_rules! define_global_metadata_kind {
)*
};

Ident::from_str(string)
Symbol::intern(string)
}
}
)
Expand Down