Skip to content
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

rustc: remove unnecessary extern_prelude logic from ty::item_path. #56655

Closed
wants to merge 1 commit into from
Closed

rustc: remove unnecessary extern_prelude logic from ty::item_path. #56655

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
107 changes: 45 additions & 62 deletions src/librustc/ty/item_path.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
});
let mut buffer = LocalPathBuffer::new(mode);
debug!("item_path_str: buffer={:?} def_id={:?}", buffer, def_id);
self.push_item_path(&mut buffer, def_id, false);
self.push_item_path(&mut buffer, def_id);
buffer.into_string()
}

Expand All @@ -91,19 +91,14 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
pub fn absolute_item_path_str(self, def_id: DefId) -> String {
let mut buffer = LocalPathBuffer::new(RootMode::Absolute);
debug!("absolute_item_path_str: buffer={:?} def_id={:?}", buffer, def_id);
self.push_item_path(&mut buffer, def_id, false);
self.push_item_path(&mut buffer, def_id);
buffer.into_string()
}

/// Returns the "path" to a particular crate. This can proceed in
/// various ways, depending on the `root_mode` of the `buffer`.
/// (See `RootMode` enum for more details.)
///
/// `pushed_prelude_crate` argument should be `true` when the buffer
/// has had a prelude crate pushed to it. If this is the case, then
/// we do not want to prepend `crate::` (as that would not be a valid
/// path).
pub fn push_krate_path<T>(self, buffer: &mut T, cnum: CrateNum, pushed_prelude_crate: bool)
pub fn push_krate_path<T>(self, buffer: &mut T, cnum: CrateNum)
where T: ItemPathBuffer + Debug
{
debug!(
Expand All @@ -125,28 +120,27 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
//
// Returns `None` for the local crate.
if cnum != LOCAL_CRATE {
let opt_extern_crate = self.extern_crate(cnum.as_def_id());
if let Some(ExternCrate {
src: ExternCrateSource::Extern(def_id),
direct: true,
..
}) = *opt_extern_crate
{
debug!("push_krate_path: def_id={:?}", def_id);
self.push_item_path(buffer, def_id, pushed_prelude_crate);
} else {
let name = self.crate_name(cnum).as_str();
debug!("push_krate_path: name={:?}", name);
buffer.push(&name);
}
} else if self.sess.rust_2018() && !pushed_prelude_crate {
SHOULD_PREFIX_WITH_CRATE.with(|flag| {
// We only add the `crate::` keyword where appropriate. In particular,
// when we've not previously pushed a prelude crate to this path.
if flag.get() {
buffer.push(&keywords::Crate.name().as_str())
match *self.extern_crate(cnum.as_def_id()) {
Some(ExternCrate {
src: ExternCrateSource::Extern(def_id),
direct: true,
span,
..
}) if !span.is_dummy() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a comment would be good here -- it's pretty non-obvious what's going on.

debug!("push_krate_path: def_id={:?}", def_id);
self.push_item_path(buffer, def_id);
}
})
_ => {
let name = self.crate_name(cnum).as_str();
debug!("push_krate_path: name={:?}", name);
buffer.push(&name);
}
}
} else if self.sess.rust_2018() {
// We add the `crate::` keyword on Rust 2018, only when desired.
if SHOULD_PREFIX_WITH_CRATE.with(|flag| flag.get()) {
buffer.push(&keywords::Crate.name().as_str())
}
}
}
RootMode::Absolute => {
Expand All @@ -166,7 +160,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
self,
buffer: &mut T,
external_def_id: DefId,
pushed_prelude_crate: bool,
) -> bool
where T: ItemPathBuffer + Debug
{
Expand All @@ -189,10 +182,15 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
Some(ExternCrate {
src: ExternCrateSource::Extern(def_id),
direct: true,
span,
..
}) => {
debug!("try_push_visible_item_path: def_id={:?}", def_id);
self.push_item_path(buffer, def_id, pushed_prelude_crate);
if !span.is_dummy() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly, comment here -- or maybe we can pull this into some kind of common helper function with the code above?

Copy link
Member Author

Choose a reason for hiding this comment

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

So in a different branch I was able to deduplicate this by moving the local mode logic from push_krate_path to try_push_visible_item_path and relying on the fact that try_push_visible_item_path can override the behavior of push_item_path. But maybe we can avoid the dummy span check here by some other means.

self.push_item_path(buffer, def_id);
} else {
buffer.push(&self.crate_name(cur_def.krate).as_str());
}
cur_path.iter().rev().for_each(|segment| buffer.push(&segment));
return true;
}
Expand Down Expand Up @@ -290,16 +288,16 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
}
}

pub fn push_item_path<T>(self, buffer: &mut T, def_id: DefId, pushed_prelude_crate: bool)
pub fn push_item_path<T>(self, buffer: &mut T, def_id: DefId)
where T: ItemPathBuffer + Debug
{
debug!(
"push_item_path: buffer={:?} def_id={:?} pushed_prelude_crate={:?}",
buffer, def_id, pushed_prelude_crate
"push_item_path: buffer={:?} def_id={:?}",
buffer, def_id
);
match *buffer.root_mode() {
RootMode::Local if !def_id.is_local() =>
if self.try_push_visible_item_path(buffer, def_id, pushed_prelude_crate) { return },
if self.try_push_visible_item_path(buffer, def_id) { return },
_ => {}
}

Expand All @@ -308,11 +306,11 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
match key.disambiguated_data.data {
DefPathData::CrateRoot => {
assert!(key.parent.is_none());
self.push_krate_path(buffer, def_id.krate, pushed_prelude_crate);
self.push_krate_path(buffer, def_id.krate);
}

DefPathData::Impl => {
self.push_impl_path(buffer, def_id, pushed_prelude_crate);
self.push_impl_path(buffer, def_id);
}

// Unclear if there is any value in distinguishing these.
Expand All @@ -335,36 +333,22 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
data @ DefPathData::ClosureExpr |
data @ DefPathData::ImplTrait |
data @ DefPathData::GlobalMetaData(..) => {
let parent_did = self.parent_def_id(def_id).unwrap();

// Keep track of whether we are one recursion away from the `CrateRoot` and
// pushing the name of a prelude crate. If we are, we'll want to know this when
// printing the `CrateRoot` so we don't prepend a `crate::` to paths.
let mut is_prelude_crate = false;
if let DefPathData::CrateRoot = self.def_key(parent_did).disambiguated_data.data {
if self.extern_prelude.contains_key(&data.as_interned_str().as_symbol()) {
is_prelude_crate = true;
}
}

self.push_item_path(
buffer, parent_did, pushed_prelude_crate || is_prelude_crate
);
let parent_def_id = self.parent_def_id(def_id).unwrap();
self.push_item_path(buffer, parent_def_id);
buffer.push(&data.as_interned_str().as_symbol().as_str());
},

DefPathData::StructCtor => { // present `X` instead of `X::{{constructor}}`
let parent_def_id = self.parent_def_id(def_id).unwrap();
self.push_item_path(buffer, parent_def_id, pushed_prelude_crate);
self.push_item_path(buffer, parent_def_id);
}
}
}

fn push_impl_path<T>(
self,
buffer: &mut T,
impl_def_id: DefId,
pushed_prelude_crate: bool,
buffer: &mut T,
impl_def_id: DefId,
)
where T: ItemPathBuffer + Debug
{
Expand All @@ -380,7 +364,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
};

if !use_types {
return self.push_impl_path_fallback(buffer, impl_def_id, pushed_prelude_crate);
return self.push_impl_path_fallback(buffer, impl_def_id);
}

// Decide whether to print the parent path for the impl.
Expand All @@ -404,7 +388,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
// If the impl is not co-located with either self-type or
// trait-type, then fallback to a format that identifies
// the module more clearly.
self.push_item_path(buffer, parent_def_id, pushed_prelude_crate);
self.push_item_path(buffer, parent_def_id);
if let Some(trait_ref) = impl_trait_ref {
buffer.push(&format!("<impl {} for {}>", trait_ref, self_ty));
} else {
Expand All @@ -428,13 +412,13 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
match self_ty.sty {
ty::Adt(adt_def, substs) => {
if substs.types().next().is_none() { // ignore regions
self.push_item_path(buffer, adt_def.did, pushed_prelude_crate);
self.push_item_path(buffer, adt_def.did);
} else {
buffer.push(&format!("<{}>", self_ty));
}
}

ty::Foreign(did) => self.push_item_path(buffer, did, pushed_prelude_crate),
ty::Foreign(did) => self.push_item_path(buffer, did),

ty::Bool |
ty::Char |
Expand All @@ -455,15 +439,14 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
self,
buffer: &mut T,
impl_def_id: DefId,
pushed_prelude_crate: bool,
)
where T: ItemPathBuffer + Debug
{
// If no type info is available, fall back to
// pretty printing some span information. This should
// only occur very early in the compiler pipeline.
let parent_def_id = self.parent_def_id(impl_def_id).unwrap();
self.push_item_path(buffer, parent_def_id, pushed_prelude_crate);
self.push_item_path(buffer, parent_def_id);
let node_id = self.hir().as_local_node_id(impl_def_id).unwrap();
let item = self.hir().expect_item(node_id);
let span_str = self.sess.source_map().span_to_string(item.span);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_utils/symbol_names.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ fn get_symbol_hash<'a, 'tcx>(
fn def_symbol_name<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> ty::SymbolName {
let mut buffer = SymbolPathBuffer::new();
item_path::with_forced_absolute_paths(|| {
tcx.push_item_path(&mut buffer, def_id, false);
tcx.push_item_path(&mut buffer, def_id);
});
buffer.into_interned()
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4023,7 +4023,7 @@ where F: Fn(DefId) -> Def {

let mut apb = AbsolutePathBuffer { names: vec![] };

tcx.push_item_path(&mut apb, def_id, false);
tcx.push_item_path(&mut apb, def_id);

hir::Path {
span: DUMMY_SP,
Expand Down