Skip to content

Commit

Permalink
Store idents for DefPathData into crate metadata
Browse files Browse the repository at this point in the history 
Previously, we threw away the `Span` associated with a definition's
identifier when we encoded crate metadata, causing us to lose location
and hygiene information.

We now store the identifier's `Span` in the crate metadata.
When we decode items from the metadata, we combine
the name and span back into an `Ident`.

This improves the output of several tests, which previously had messages
suppressed due to dummy spans.

This is a prerequisite for rust-lang#68686, since throwing away a `Span` means
that we lose hygiene information.
  • Loading branch information
@Aaron1011
Aaron1011 committed Mar 20, 2020
1 parent f4c675c commit 1f13a4f
Show file tree
Hide file tree
Showing 28 changed files with 224 additions and 39 deletions.
56 changes: 31 additions & 25 deletions src/librustc_metadata/rmeta/decoder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -509,14 +509,6 @@ impl<'a, 'tcx> SpecializedDecoder<Span> for DecodeContext<'a, 'tcx> {
}
}

impl SpecializedDecoder<Ident> for DecodeContext<'_, '_> {
fn specialized_decode(&mut self) -> Result<Ident, Self::Error> {
// FIXME(jseyfried): intercrate hygiene

Ok(Ident::with_dummy_span(Symbol::decode(self)?))
}
}

impl<'a, 'tcx> SpecializedDecoder<Fingerprint> for DecodeContext<'a, 'tcx> {
fn specialized_decode(&mut self) -> Result<Fingerprint, Self::Error> {
Fingerprint::decode_opaque(&mut self.opaque)
Expand Down Expand Up @@ -663,15 +655,27 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
&self.raw_proc_macros.unwrap()[pos]
}

fn item_name(&self, item_index: DefIndex) -> Symbol {
fn item_ident(&self, item_index: DefIndex, sess: &Session) -> Ident {
if !self.is_proc_macro(item_index) {
self.def_key(item_index)
let name = self
.def_key(item_index)
.disambiguated_data
.data
.get_opt_name()
.expect("no name in item_name")
.expect("no name in item_ident");
let span = self
.root
.per_def
.ident_span
.get(self, item_index)
.map(|data| data.decode((self, sess)))
.unwrap_or_else(|| panic!("Missing ident span for {:?} ({:?})", name, item_index));
Ident::new(name, span)
} else {
Symbol::intern(self.raw_proc_macro(item_index).name())
Ident::new(
Symbol::intern(self.raw_proc_macro(item_index).name()),
self.get_span(item_index, sess),
)
}
}

Expand Down Expand Up @@ -750,6 +754,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
kind: &EntryKind,
index: DefIndex,
parent_did: DefId,
sess: &Session,
) -> ty::VariantDef {
let data = match kind {
EntryKind::Variant(data) | EntryKind::Struct(data, _) | EntryKind::Union(data, _) => {
Expand All @@ -771,7 +776,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {

ty::VariantDef::new(
tcx,
Ident::with_dummy_span(self.item_name(index)),
self.item_ident(index, sess),
variant_did,
ctor_did,
data.discr,
Expand All @@ -783,7 +788,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
.decode(self)
.map(|index| ty::FieldDef {
did: self.local_def_id(index),
ident: Ident::with_dummy_span(self.item_name(index)),
ident: self.item_ident(index, sess),
vis: self.get_visibility(index),
})
.collect(),
Expand Down Expand Up @@ -812,10 +817,10 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
.get(self, item_id)
.unwrap_or(Lazy::empty())
.decode(self)
.map(|index| self.get_variant(tcx, &self.kind(index), index, did))
.map(|index| self.get_variant(tcx, &self.kind(index), index, did, tcx.sess))
.collect()
} else {
std::iter::once(self.get_variant(tcx, &kind, item_id, did)).collect()
std::iter::once(self.get_variant(tcx, &kind, item_id, did, tcx.sess)).collect()
};

tcx.alloc_adt_def(did, adt_kind, variants, repr)
Expand Down Expand Up @@ -1007,7 +1012,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
if let Some(kind) = self.def_kind(child_index) {
callback(Export {
res: Res::Def(kind, self.local_def_id(child_index)),
ident: Ident::with_dummy_span(self.item_name(child_index)),
ident: self.item_ident(child_index, sess),
vis: self.get_visibility(child_index),
span: self
.root
Expand All @@ -1028,10 +1033,11 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {

let def_key = self.def_key(child_index);
let span = self.get_span(child_index, sess);
if let (Some(kind), Some(name)) =
(self.def_kind(child_index), def_key.disambiguated_data.data.get_opt_name())
{
let ident = Ident::with_dummy_span(name);
if let (Some(kind), true) = (
self.def_kind(child_index),
def_key.disambiguated_data.data.get_opt_name().is_some(),
) {
let ident = self.item_ident(child_index, sess);
let vis = self.get_visibility(child_index);
let def_id = self.local_def_id(child_index);
let res = Res::Def(kind, def_id);
Expand Down Expand Up @@ -1138,10 +1144,10 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
}
}

fn get_associated_item(&self, id: DefIndex) -> ty::AssocItem {
fn get_associated_item(&self, id: DefIndex, sess: &Session) -> ty::AssocItem {
let def_key = self.def_key(id);
let parent = self.local_def_id(def_key.parent.unwrap());
let name = def_key.disambiguated_data.data.get_opt_name().unwrap();
let ident = self.item_ident(id, sess);

let (kind, container, has_self) = match self.kind(id) {
EntryKind::AssocConst(container, _, _) => (ty::AssocKind::Const, container, false),
Expand All @@ -1155,7 +1161,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
};

ty::AssocItem {
ident: Ident::with_dummy_span(name),
ident,
kind,
vis: self.get_visibility(id),
defaultness: container.defaultness(),
Expand Down Expand Up @@ -1219,7 +1225,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
.get(self, id)
.unwrap_or(Lazy::empty())
.decode(self)
.map(|index| respan(self.get_span(index, sess), self.item_name(index)))
.map(|index| respan(self.get_span(index, sess), self.item_ident(index, sess).name))
.collect()
}

Expand Down
6 changes: 3 additions & 3 deletions src/librustc_metadata/rmeta/decoder/cstore_impl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ provide! { <'tcx> tcx, def_id, other, cdata,
|child| result.push(child.res.def_id()), tcx.sess);
tcx.arena.alloc_slice(&result)
}
associated_item => { cdata.get_associated_item(def_id.index) }
associated_item => { cdata.get_associated_item(def_id.index, tcx.sess) }
impl_trait_ref => { cdata.get_impl_trait(def_id.index, tcx) }
impl_polarity => { cdata.get_impl_polarity(def_id.index) }
coerce_unsized_info => {
Expand Down Expand Up @@ -442,8 +442,8 @@ impl CStore {
)
}

pub fn associated_item_cloned_untracked(&self, def: DefId) -> ty::AssocItem {
self.get_crate_data(def.krate).get_associated_item(def.index)
pub fn associated_item_cloned_untracked(&self, def: DefId, sess: &Session) -> ty::AssocItem {
self.get_crate_data(def.krate).get_associated_item(def.index, sess)
}

pub fn crate_source_untracked(&self, cnum: CrateNum) -> CrateSource {
Expand Down
23 changes: 14 additions & 9 deletions src/librustc_metadata/rmeta/encoder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc::traits::specialization_graph;
use rustc::ty::codec::{self as ty_codec, TyEncoder};
use rustc::ty::layout::VariantIdx;
use rustc::ty::{self, SymbolName, Ty, TyCtxt};
use rustc_ast::ast;
use rustc_ast::ast::{self, Ident};
use rustc_ast::attr;
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::fx::FxHashMap;
Expand All @@ -29,7 +29,7 @@ use rustc_index::vec::Idx;
use rustc_serialize::{opaque, Encodable, Encoder, SpecializedEncoder};
use rustc_session::config::{self, CrateType};
use rustc_span::source_map::Spanned;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::symbol::{kw, sym, Symbol};
use rustc_span::{self, ExternalSource, FileName, SourceFile, Span};
use std::hash::Hash;
use std::num::NonZeroUsize;
Expand Down Expand Up @@ -219,13 +219,6 @@ impl<'tcx> SpecializedEncoder<Span> for EncodeContext<'tcx> {
}
}

impl SpecializedEncoder<Ident> for EncodeContext<'tcx> {
fn specialized_encode(&mut self, ident: &Ident) -> Result<(), Self::Error> {
// FIXME(jseyfried): intercrate hygiene
ident.name.encode(self)
}
}

impl<'tcx> SpecializedEncoder<LocalDefId> for EncodeContext<'tcx> {
#[inline]
fn specialized_encode(&mut self, def_id: &LocalDefId) -> Result<(), Self::Error> {
Expand Down Expand Up @@ -631,6 +624,7 @@ impl EncodeContext<'tcx> {
assert!(f.did.is_local());
f.did.index
}));
self.encode_ident_span(def_id, variant.ident);
self.encode_stability(def_id);
self.encode_deprecation(def_id);
self.encode_item_type(def_id);
Expand Down Expand Up @@ -733,6 +727,7 @@ impl EncodeContext<'tcx> {
record!(self.per_def.visibility[def_id] <- field.vis);
record!(self.per_def.span[def_id] <- self.tcx.def_span(def_id));
record!(self.per_def.attributes[def_id] <- variant_data.fields()[field_index].attrs);
self.encode_ident_span(def_id, field.ident);
self.encode_stability(def_id);
self.encode_deprecation(def_id);
self.encode_item_type(def_id);
Expand Down Expand Up @@ -867,6 +862,7 @@ impl EncodeContext<'tcx> {
record!(self.per_def.visibility[def_id] <- trait_item.vis);
record!(self.per_def.span[def_id] <- ast_item.span);
record!(self.per_def.attributes[def_id] <- ast_item.attrs);
self.encode_ident_span(def_id, ast_item.ident);
self.encode_stability(def_id);
self.encode_const_stability(def_id);
self.encode_deprecation(def_id);
Expand Down Expand Up @@ -948,6 +944,7 @@ impl EncodeContext<'tcx> {
record!(self.per_def.visibility[def_id] <- impl_item.vis);
record!(self.per_def.span[def_id] <- ast_item.span);
record!(self.per_def.attributes[def_id] <- ast_item.attrs);
self.encode_ident_span(def_id, impl_item.ident);
self.encode_stability(def_id);
self.encode_const_stability(def_id);
self.encode_deprecation(def_id);
Expand Down Expand Up @@ -1051,6 +1048,8 @@ impl EncodeContext<'tcx> {

debug!("EncodeContext::encode_info_for_item({:?})", def_id);

self.encode_ident_span(def_id, item.ident);

record!(self.per_def.kind[def_id] <- match item.kind {
hir::ItemKind::Static(_, hir::Mutability::Mut, _) => EntryKind::MutStatic,
hir::ItemKind::Static(_, hir::Mutability::Not, _) => EntryKind::ImmStatic,
Expand Down Expand Up @@ -1275,6 +1274,7 @@ impl EncodeContext<'tcx> {
record!(self.per_def.visibility[def_id] <- ty::Visibility::Public);
record!(self.per_def.span[def_id] <- macro_def.span);
record!(self.per_def.attributes[def_id] <- macro_def.attrs);
self.encode_ident_span(def_id, macro_def.ident);
self.encode_stability(def_id);
self.encode_deprecation(def_id);
}
Expand Down Expand Up @@ -1519,6 +1519,7 @@ impl EncodeContext<'tcx> {
ty::Visibility::from_hir(&nitem.vis, nitem.hir_id, self.tcx));
record!(self.per_def.span[def_id] <- nitem.span);
record!(self.per_def.attributes[def_id] <- nitem.attrs);
self.encode_ident_span(def_id, nitem.ident);
self.encode_stability(def_id);
self.encode_const_stability(def_id);
self.encode_deprecation(def_id);
Expand Down Expand Up @@ -1613,6 +1614,10 @@ impl EncodeContext<'tcx> {
}
}

fn encode_ident_span(&mut self, def_id: DefId, ident: Ident) {
record!(self.per_def.ident_span[def_id] <- ident.span);
}

/// In some cases, along with the item itself, we also
/// encode some sub-items. Usually we want some info from the item
/// so it's easier to do that here then to wait until we would encounter
Expand Down
1 change: 1 addition & 0 deletions src/librustc_metadata/rmeta/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ define_per_def_tables! {
kind: Table<DefIndex, Lazy<EntryKind>>,
visibility: Table<DefIndex, Lazy<ty::Visibility>>,
span: Table<DefIndex, Lazy<Span>>,
ident_span: Table<DefIndex, Lazy<Span>>,
attributes: Table<DefIndex, Lazy<[ast::Attribute]>>,
children: Table<DefIndex, Lazy<[DefIndex]>>,
stability: Table<DefIndex, Lazy<attr::Stability>>,
Expand Down
5 changes: 4 additions & 1 deletion src/librustc_resolve/build_reduced_graph.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,10 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
self.insert_field_names(def_id, field_names);
}
Res::Def(DefKind::AssocFn, def_id) => {
if cstore.associated_item_cloned_untracked(def_id).method_has_self_argument {
if cstore
.associated_item_cloned_untracked(def_id, self.r.session)
.method_has_self_argument
{
self.r.has_self.insert(def_id);
}
}
Expand Down
8 changes: 8 additions & 0 deletions src/test/ui/copy-a-resource.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ LL | struct Foo {
...
LL | let _y = x.clone();
| ^^^^^ method not found in `Foo`
|
::: $SRC_DIR/libcore/clone.rs:LL:COL
|
LL | fn clone(&self) -> Self;
| -----
| |
| the method is available for `std::sync::Arc<Foo>` here
| the method is available for `std::rc::Rc<Foo>` here
|
= help: items from traits can only be used if the trait is implemented and in scope
= note: the following trait defines an item `clone`, perhaps you need to implement it:
Expand Down
8 changes: 8 additions & 0 deletions src/test/ui/derives/derive-assoc-type-not-impl.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ LL | struct NotClone;
...
LL | Bar::<NotClone> { x: 1 }.clone();
| ^^^^^ method not found in `Bar<NotClone>`
|
::: $SRC_DIR/libcore/clone.rs:LL:COL
|
LL | fn clone(&self) -> Self;
| -----
| |
| the method is available for `std::sync::Arc<Bar<NotClone>>` here
| the method is available for `std::rc::Rc<Bar<NotClone>>` here
|
= note: the method `clone` exists but the following trait bounds were not satisfied:
`NotClone: std::clone::Clone`
Expand Down
8 changes: 8 additions & 0 deletions src/test/ui/error-codes/E0004-2.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ error[E0004]: non-exhaustive patterns: `None` and `Some(_)` not covered
|
LL | match x { }
| ^ patterns `None` and `Some(_)` not covered
|
::: $SRC_DIR/libcore/option.rs:LL:COL
|
LL | None,
| ---- not covered
...
LL | Some(#[stable(feature = "rust1", since = "1.0.0")] T),
| ---- not covered
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

Expand Down
5 changes: 5 additions & 0 deletions src/test/ui/error-codes/E0005.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ error[E0005]: refutable pattern in local binding: `None` not covered
|
LL | let Some(y) = x;
| ^^^^^^^ pattern `None` not covered
|
::: $SRC_DIR/libcore/option.rs:LL:COL
|
LL | None,
| ---- not covered
|
= note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
= note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
Expand Down
5 changes: 5 additions & 0 deletions src/test/ui/error-codes/E0297.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ error[E0005]: refutable pattern in `for` loop binding: `None` not covered
|
LL | for Some(x) in xs {}
| ^^^^^^^ pattern `None` not covered
|
::: $SRC_DIR/libcore/option.rs:LL:COL
|
LL | None,
| ---- not covered

error: aborting due to previous error

Expand Down
5 changes: 5 additions & 0 deletions src/test/ui/feature-gates/feature-gate-exhaustive-patterns.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ error[E0005]: refutable pattern in local binding: `Err(_)` not covered
|
LL | let Ok(_x) = foo();
| ^^^^^^ pattern `Err(_)` not covered
|
::: $SRC_DIR/libcore/result.rs:LL:COL
|
LL | Err(#[stable(feature = "rust1", since = "1.0.0")] E),
| --- not covered
|
= note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
= note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
Expand Down
10 changes: 10 additions & 0 deletions src/test/ui/generic-associated-types/iterable.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ LL | impl<T> Iterable for Vec<T> {
| --------------------------- in this `impl` item
LL | type Item<'a> where T: 'a = <std::slice::Iter<'a, T> as Iterator>::Item;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected reference, found associated type
|
::: $SRC_DIR/libcore/iter/traits/iterator.rs:LL:COL
|
LL | type Item;
| ---- associated type defined here
|
= note: expected reference `&T`
found associated type `<std::vec::Vec<T> as Iterable>::Item<'_>`
Expand All @@ -18,6 +23,11 @@ LL | impl<T> Iterable for [T] {
| ------------------------ in this `impl` item
LL | type Item<'a> where T: 'a = <std::slice::Iter<'a, T> as Iterator>::Item;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected reference, found associated type
|
::: $SRC_DIR/libcore/iter/traits/iterator.rs:LL:COL
|
LL | type Item;
| ---- associated type defined here
|
= note: expected reference `&T`
found associated type `<[T] as Iterable>::Item<'_>`
Expand Down
Loading

0 comments on commit 1f13a4f

Please sign in to comment.