Skip to content

Commit

Permalink
Store Ident in DefPathData instead of Symbol
Browse files Browse the repository at this point in the history
This allows us to recover span information when emitting cross crate
errors. A number of test cases benefit from this change, since we were
previously not emitting messages due to invalid spans.

There are four main parts to this commit:

1. Adding `Ident` to `DefPathData`, and updating the affected code. This
mostly consists of mechanical changes.

2. Updating how we determine the disambiguator for a `DefPath`. Since
`DefPathData` now stores a `Span` (inside the `Ident`), we need to
make sure we ignore this span we determining if a path needs to be
disambiguated. This ensure that two paths with the same symbols but
different spans are considered equal (this can occur when a macro is
repeatedly expanded to a definition).

3. Ensuring that we are able to decode `Spans` when decoding the
`DefPathTable`. Since the `DefPathTable` is stored in `CrateMetadata`, we
must decode it before we have a `CrateMetadata` available. Since
decoding a `Span` requires access to several fields from
`CrateMetadata`, this commit adds a new struct `InitialCrateMetadata`,
which implements `Metadata` and stores all of the necessary fields.

4. The specialized metadata encoder/decoder impls for `Ident` are
removed. This causes us to fall back to the default encoder/decoder
implementations for `Ident`, which simply serializes and deserializes
the fields of `Ident`. This is strictly an improvement - we still don't
have any hygiene information, but we now have a non-dummy Span.

This should hopefully allow us to test PR rust-lang#68941, since we will now use
cross-crate spans in more places.
  • Loading branch information
Aaron1011 committed Mar 17, 2020
1 parent 5e9ebf4 commit 5e98189
Show file tree
Hide file tree
Showing 46 changed files with 517 additions and 224 deletions.
63 changes: 42 additions & 21 deletions src/librustc/hir/map/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_hir::def_id::{CrateNum, DefId, DefIndex, CRATE_DEF_INDEX, LOCAL_CRATE}
use rustc_index::vec::IndexVec;
use rustc_session::CrateDisambiguator;
use rustc_span::hygiene::ExpnId;
use rustc_span::symbol::{sym, Symbol};
use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_span::Span;

use std::fmt::Write;
Expand Down Expand Up @@ -90,6 +90,11 @@ pub struct Definitions {
parent_modules_of_macro_defs: FxHashMap<ExpnId, DefId>,
/// Item with a given `DefIndex` was defined during macro expansion with ID `ExpnId`.
expansions_that_defined: FxHashMap<DefIndex, ExpnId>,
// Keeps track of the current disambiguator for a given (DefIndex, DefPathData)
// Note that we replace any `Spans` (e.g. in `Ident`) with `DUMMY_SP` before
// inserting a `DefPathData`. This ensures that we create a new disambiguator
// for definitions that have the same name, but different spans (e.g. definitions
// created by a macro invocation).
next_disambiguator: FxHashMap<(DefIndex, DefPathData), u32>,
def_index_to_span: FxHashMap<DefIndex, Span>,
/// When collecting definitions from an AST fragment produced by a macro invocation `ExpnId`
Expand Down Expand Up @@ -206,7 +211,7 @@ impl DefPath {
let mut s = String::with_capacity(self.data.len() * 16);

for component in &self.data {
write!(s, "::{}[{}]", component.data.as_symbol(), component.disambiguator).unwrap();
write!(s, "::{}[{}]", component.data.as_ident(), component.disambiguator).unwrap();
}

s
Expand All @@ -225,9 +230,9 @@ impl DefPath {

for component in &self.data {
if component.disambiguator == 0 {
write!(s, "::{}", component.data.as_symbol()).unwrap();
write!(s, "::{}", component.data.as_ident()).unwrap();
} else {
write!(s, "{}[{}]", component.data.as_symbol(), component.disambiguator).unwrap();
write!(s, "{}[{}]", component.data.as_ident(), component.disambiguator).unwrap();
}
}

Expand All @@ -245,9 +250,9 @@ impl DefPath {
opt_delimiter.map(|d| s.push(d));
opt_delimiter = Some('-');
if component.disambiguator == 0 {
write!(s, "{}", component.data.as_symbol()).unwrap();
write!(s, "{}", component.data.as_ident()).unwrap();
} else {
write!(s, "{}[{}]", component.data.as_symbol(), component.disambiguator).unwrap();
write!(s, "{}[{}]", component.data.as_ident(), component.disambiguator).unwrap();
}
}
s
Expand All @@ -267,13 +272,13 @@ pub enum DefPathData {
/// An impl.
Impl,
/// Something in the type namespace.
TypeNs(Symbol),
TypeNs(Ident),
/// Something in the value namespace.
ValueNs(Symbol),
ValueNs(Ident),
/// Something in the macro namespace.
MacroNs(Symbol),
MacroNs(Ident),
/// Something in the lifetime namespace.
LifetimeNs(Symbol),
LifetimeNs(Ident),
/// A closure expression.
ClosureExpr,

Expand Down Expand Up @@ -432,7 +437,10 @@ impl Definitions {

// Find the next free disambiguator for this key.
let disambiguator = {
let next_disamb = self.next_disambiguator.entry((parent, data)).or_insert(0);
// Replace any span with `DUMMY_SP` - two definitions which differ
// only in their spans still need to be disambiguated.
let data_dummy_span = data.with_dummy_span();
let next_disamb = self.next_disambiguator.entry((parent, data_dummy_span)).or_insert(0);
let disambiguator = *next_disamb;
*next_disamb = next_disamb.checked_add(1).expect("disambiguator overflow");
disambiguator
Expand Down Expand Up @@ -522,7 +530,20 @@ impl Definitions {
}

impl DefPathData {
pub fn get_opt_name(&self) -> Option<Symbol> {
/// Replaces any `Spans` contains in this `DefPathData` with `DUMMY_SP`.
/// Useful when using a `DefPathData` as a cache key.
pub fn with_dummy_span(&self) -> DefPathData {
use self::DefPathData::*;
match *self {
TypeNs(name) => TypeNs(Ident::with_dummy_span(name.name)),
ValueNs(name) => ValueNs(Ident::with_dummy_span(name.name)),
MacroNs(name) => MacroNs(Ident::with_dummy_span(name.name)),
LifetimeNs(name) => LifetimeNs(Ident::with_dummy_span(name.name)),
_ => *self,
}
}

pub fn get_opt_name(&self) -> Option<Ident> {
use self::DefPathData::*;
match *self {
TypeNs(name) | ValueNs(name) | MacroNs(name) | LifetimeNs(name) => Some(name),
Expand All @@ -531,22 +552,22 @@ impl DefPathData {
}
}

pub fn as_symbol(&self) -> Symbol {
pub fn as_ident(&self) -> Ident {
use self::DefPathData::*;
match *self {
TypeNs(name) | ValueNs(name) | MacroNs(name) | LifetimeNs(name) => name,
// Note that this does not show up in user print-outs.
CrateRoot => sym::double_braced_crate,
Impl => sym::double_braced_impl,
Misc => sym::double_braced_misc,
ClosureExpr => sym::double_braced_closure,
Ctor => sym::double_braced_constructor,
AnonConst => sym::double_braced_constant,
ImplTrait => sym::double_braced_opaque,
CrateRoot => Ident::with_dummy_span(sym::double_braced_crate),
Impl => Ident::with_dummy_span(sym::double_braced_impl),
Misc => Ident::with_dummy_span(sym::double_braced_misc),
ClosureExpr => Ident::with_dummy_span(sym::double_braced_closure),
Ctor => Ident::with_dummy_span(sym::double_braced_constructor),
AnonConst => Ident::with_dummy_span(sym::double_braced_constant),
ImplTrait => Ident::with_dummy_span(sym::double_braced_opaque),
}
}

pub fn to_string(&self) -> String {
self.as_symbol().to_string()
self.as_ident().to_string()
}
}
13 changes: 10 additions & 3 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2996,9 +2996,16 @@ impl<'tcx> TyCtxt<'tcx> {
hir_map::DefPathData::Ctor => {
self.item_name(DefId { krate: id.krate, index: def_key.parent.unwrap() })
}
_ => def_key.disambiguated_data.data.get_opt_name().unwrap_or_else(|| {
bug!("item_name: no name for {:?}", self.def_path(id));
}),
_ => {
def_key
.disambiguated_data
.data
.get_opt_name()
.unwrap_or_else(|| {
bug!("item_name: no name for {:?}", self.def_path(id));
})
.name
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/print/obsolete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,9 @@ impl DefPathBasedNames<'tcx> {
// foo::bar::ItemName::
for part in self.tcx.def_path(def_id).data {
if self.omit_disambiguators {
write!(output, "{}::", part.data.as_symbol()).unwrap();
write!(output, "{}::", part.data.as_ident()).unwrap();
} else {
write!(output, "{}[{}]::", part.data.as_symbol(), part.disambiguator).unwrap();
write!(output, "{}[{}]::", part.data.as_ident(), part.disambiguator).unwrap();
}
}

Expand Down
10 changes: 6 additions & 4 deletions src/librustc/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_apfloat::ieee::{Double, Single};
use rustc_apfloat::Float;
use rustc_ast::ast;
use rustc_attr::{SignedInt, UnsignedInt};
use rustc_span::symbol::{kw, Symbol};
use rustc_span::symbol::{kw, Ident, Symbol};
use rustc_target::spec::abi::Abi;

use std::cell::Cell;
Expand Down Expand Up @@ -402,12 +402,14 @@ pub trait PrettyPrinter<'tcx>:
.find(|child| child.res.def_id() == def_id)
.map(|child| child.ident.name);
if let Some(reexport) = reexport {
*name = reexport;
*name = Ident::with_dummy_span(reexport);
}
}
// Re-exported `extern crate` (#43189).
DefPathData::CrateRoot => {
data = DefPathData::TypeNs(self.tcx().original_crate_name(def_id.krate));
data = DefPathData::TypeNs(Ident::with_dummy_span(
self.tcx().original_crate_name(def_id.krate),
));
}
_ => {}
}
Expand Down Expand Up @@ -1401,7 +1403,7 @@ impl<F: fmt::Write> Printer<'tcx> for FmtPrinter<'_, 'tcx, F> {

// FIXME(eddyb) `name` should never be empty, but it
// currently is for `extern { ... }` "foreign modules".
let name = disambiguated_data.data.as_symbol().as_str();
let name = disambiguated_data.data.to_string();
if !name.is_empty() {
if !self.empty_path {
write!(self, "::")?;
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/query/profiling_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@ impl<'p, 'c, 'tcx> QueryKeyStringBuilder<'p, 'c, 'tcx> {

match def_key.disambiguated_data.data {
DefPathData::CrateRoot => {
name = self.tcx.original_crate_name(def_id.krate).as_str();
name = self.tcx.original_crate_name(def_id.krate).as_str().to_string();
dis = "";
end_index = 3;
}
other => {
name = other.as_symbol().as_str();
name = other.to_string();
if def_key.disambiguated_data.disambiguator == 0 {
dis = "";
end_index = 3;
Expand Down
16 changes: 10 additions & 6 deletions src/librustc_ast_lowering/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -764,17 +764,21 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
// Get the name we'll use to make the def-path. Note
// that collisions are ok here and this shouldn't
// really show up for end-user.
let (str_name, kind) = match hir_name {
ParamName::Plain(ident) => (ident.name, hir::LifetimeParamKind::InBand),
ParamName::Fresh(_) => (kw::UnderscoreLifetime, hir::LifetimeParamKind::Elided),
ParamName::Error => (kw::UnderscoreLifetime, hir::LifetimeParamKind::Error),
let (name, kind) = match hir_name {
ParamName::Plain(ident) => (ident, hir::LifetimeParamKind::InBand),
ParamName::Fresh(_) => {
(Ident::with_dummy_span(kw::UnderscoreLifetime), hir::LifetimeParamKind::Elided)
}
ParamName::Error => {
(Ident::with_dummy_span(kw::UnderscoreLifetime), hir::LifetimeParamKind::Error)
}
};

// Add a definition for the in-band lifetime def.
self.resolver.definitions().create_def_with_parent(
parent_index,
node_id,
DefPathData::LifetimeNs(str_name),
DefPathData::LifetimeNs(name),
ExpnId::root(),
span,
);
Expand Down Expand Up @@ -1560,7 +1564,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
self.context.resolver.definitions().create_def_with_parent(
self.parent,
def_node_id,
DefPathData::LifetimeNs(name.ident().name),
DefPathData::LifetimeNs(name.ident()),
ExpnId::root(),
lifetime.span,
);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/debuginfo/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub fn item_namespace(cx: &CodegenCx<'ll, '_>, def_id: DefId) -> &'ll DIScope {

let namespace_name = match def_key.disambiguated_data.data {
DefPathData::CrateRoot => cx.tcx.crate_name(def_id.krate),
data => data.as_symbol(),
data => data.as_ident().name,
};
let namespace_name = namespace_name.as_str();

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_ssa/debuginfo/type_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ pub fn push_debuginfo_type_name<'tcx>(
output.push_str(&tcx.crate_name(def_id.krate).as_str());
for path_element in tcx.def_path(def_id).data {
output.push_str("::");
output.push_str(&path_element.data.as_symbol().as_str());
output.push_str(&path_element.data.to_string());
}
} else {
output.push_str(&tcx.item_name(def_id).as_str());
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_utils/symbol_names/legacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ impl Printer<'tcx> for SymbolPrinter<'tcx> {
self.path.finalize_pending_component();
}

self.write_str(&disambiguated_data.data.as_symbol().as_str())?;
self.write_str(&disambiguated_data.data.to_string())?;
Ok(self)
}
fn path_generic_args(
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_infer/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
disambiguated_data: &DisambiguatedDefPathData,
) -> Result<Self::Path, Self::Error> {
let mut path = print_prefix(self)?;
path.push(disambiguated_data.data.as_symbol().to_string());
path.push(disambiguated_data.data.to_string());
Ok(path)
}
fn path_generic_args(
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ impl<'a, 'tcx> LateContext<'a, 'tcx> {
_ => {}
}

path.push(disambiguated_data.data.as_symbol());
path.push(disambiguated_data.data.as_ident().name);
Ok(path)
}

Expand Down
15 changes: 14 additions & 1 deletion src/librustc_metadata/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc_ast::ast;
use rustc_ast::attr;
use rustc_ast::expand::allocator::{global_allocator_spans, AllocatorKind};
use rustc_data_structures::svh::Svh;
use rustc_data_structures::sync::Lrc;
use rustc_data_structures::sync::{Lrc, Once};
use rustc_errors::struct_span_err;
use rustc_expand::base::SyntaxExtension;
use rustc_hir::def_id::{CrateNum, LOCAL_CRATE};
Expand All @@ -29,6 +29,19 @@ use proc_macro::bridge::client::ProcMacro;
use std::path::Path;
use std::{cmp, fs};

crate type SourceMapImportInfo = Once<Vec<ImportedSourceFile>>;

/// Holds information about a rustc_span::SourceFile imported from another crate.
/// See `imported_source_files()` for more information.
crate struct ImportedSourceFile {
/// This SourceFile's byte-offset within the source_map of its original crate
pub original_start_pos: rustc_span::BytePos,
/// The end of this SourceFile within the source_map of its original crate
pub original_end_pos: rustc_span::BytePos,
/// The imported SourceFile's representation within the local source_map
pub translated_source_file: Lrc<rustc_span::SourceFile>,
}

#[derive(Clone)]
pub struct CStore {
metas: IndexVec<CrateNum, Option<Lrc<CrateMetadata>>>,
Expand Down
Loading

0 comments on commit 5e98189

Please sign in to comment.