From 67d8d18b230dda20758f3733ebce56fb09f0e093 Mon Sep 17 00:00:00 2001 From: Justus K Date: Sat, 8 May 2021 10:04:03 +0200 Subject: [PATCH] Minimize amount of fake `DefId`s used in rustdoc --- src/librustdoc/clean/mod.rs | 16 ++--- src/librustdoc/clean/types.rs | 8 +-- src/librustdoc/core.rs | 6 +- src/librustdoc/formats/cache.rs | 4 +- src/librustdoc/html/render/cache.rs | 8 +-- src/librustdoc/html/render/mod.rs | 8 +-- src/librustdoc/html/render/print_item.rs | 2 +- src/librustdoc/html/render/write_shared.rs | 2 - src/librustdoc/json/conversions.rs | 5 +- src/librustdoc/json/mod.rs | 15 ++--- .../passes/collect_intra_doc_links.rs | 63 +++++++------------ src/librustdoc/passes/collect_trait_impls.rs | 8 +-- 12 files changed, 55 insertions(+), 90 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index e1dde8eeaf84a..feeb03b1b6700 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -533,8 +533,7 @@ impl Clean for hir::Generics<'_> { match param.kind { GenericParamDefKind::Lifetime => unreachable!(), GenericParamDefKind::Type { did, ref bounds, .. } => { - cx.impl_trait_bounds - .insert(FakeDefId::new_real(did).into(), bounds.clone()); + cx.impl_trait_bounds.insert(did.into(), bounds.clone()); } GenericParamDefKind::Const { .. } => unreachable!(), } @@ -615,7 +614,7 @@ impl<'a, 'tcx> Clean for (&'a ty::Generics, ty::GenericPredicates<'tcx .collect::>(); // param index -> [(DefId of trait, associated type name, type)] - let mut impl_trait_proj = FxHashMap::)>>::default(); + let mut impl_trait_proj = FxHashMap::)>>::default(); let where_predicates = preds .predicates @@ -687,13 +686,7 @@ impl<'a, 'tcx> Clean for (&'a ty::Generics, ty::GenericPredicates<'tcx if let Some(proj) = impl_trait_proj.remove(&idx) { for (trait_did, name, rhs) in proj { let rhs = rhs.clean(cx); - simplify::merge_bounds( - cx, - &mut bounds, - trait_did.expect_real(), - name, - &rhs, - ); + simplify::merge_bounds(cx, &mut bounds, trait_did, name, &rhs); } } } else { @@ -1183,8 +1176,7 @@ fn clean_qpath(hir_ty: &hir::Ty<'_>, cx: &mut DocContext<'_>) -> Type { if let Some(new_ty) = cx.ty_substs.get(&did).cloned() { return new_ty; } - if let Some(bounds) = cx.impl_trait_bounds.remove(&FakeDefId::new_real(did).into()) - { + if let Some(bounds) = cx.impl_trait_bounds.remove(&did.into()) { return ImplTrait(bounds); } } diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index f0e0401824265..edd3d77eeb780 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -73,10 +73,6 @@ impl FakeDefId { Self::Fake(DefIndex::from(id), krate) } - crate fn new_real(id: DefId) -> Self { - Self::Real(id) - } - #[inline] crate fn is_local(self) -> bool { match self { @@ -470,7 +466,7 @@ impl Item { .filter_map(|ItemLink { link: s, link_text, did, ref fragment }| { match did { Some(did) => { - if let Some((mut href, ..)) = href(did.expect_real(), cx) { + if let Some((mut href, ..)) = href(did.clone(), cx) { if let Some(ref fragment) = *fragment { href.push('#'); href.push_str(fragment); @@ -972,7 +968,7 @@ crate struct ItemLink { /// This may not be the same as `link` if there was a disambiguator /// in an intra-doc link (e.g. \[`fn@f`\]) pub(crate) link_text: String, - pub(crate) did: Option, + pub(crate) did: Option, /// The url fragment to append to the link pub(crate) fragment: Option, } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index b43916f9f332c..2930c3c5fb7ec 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -576,12 +576,12 @@ impl<'tcx> Visitor<'tcx> for EmitIgnoredResolutionErrors<'tcx> { /// for `impl Trait` in argument position. #[derive(Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] crate enum ImplTraitParam { - DefId(FakeDefId), + DefId(DefId), ParamIndex(u32), } -impl From for ImplTraitParam { - fn from(did: FakeDefId) -> Self { +impl From for ImplTraitParam { + fn from(did: DefId) -> Self { ImplTraitParam::DefId(did) } } diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index 50496f320096c..5734a4a98e2b5 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -67,7 +67,7 @@ crate struct Cache { /// When rendering traits, it's often useful to be able to list all /// implementors of the trait, and this mapping is exactly, that: a mapping /// of trait ids to the list of known implementors of the trait - crate implementors: FxHashMap>, + crate implementors: FxHashMap>, /// Cache of where external crate documentation can be found. crate extern_locations: FxHashMap, @@ -299,7 +299,7 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { desc: item .doc_value() .map_or_else(String::new, |x| short_markdown_summary(&x.as_str())), - parent: parent.map(FakeDefId::new_real), + parent, parent_idx: None, search_type: get_index_search_type(&item, &self.empty_cache, self.tcx), aliases: item.attrs.get_doc_aliases(), diff --git a/src/librustdoc/html/render/cache.rs b/src/librustdoc/html/render/cache.rs index 57520a1a1fb46..3e056c4b67a70 100644 --- a/src/librustdoc/html/render/cache.rs +++ b/src/librustdoc/html/render/cache.rs @@ -7,7 +7,7 @@ use serde::ser::{Serialize, SerializeStruct, Serializer}; use crate::clean; use crate::clean::types::{ - FakeDefId, FnDecl, FnRetTy, GenericBound, Generics, GetDefId, Type, WherePredicate, + FnDecl, FnRetTy, GenericBound, Generics, GetDefId, Type, WherePredicate, }; use crate::formats::cache::Cache; use crate::formats::item_type::ItemType; @@ -82,7 +82,7 @@ crate fn build_index<'tcx>(krate: &clean::Crate, cache: &mut Cache, tcx: TyCtxt< defid_to_pathid.insert(defid, pathid); lastpathid += 1; - if let Some(&(ref fqp, short)) = paths.get(&defid.expect_real()) { + if let Some(&(ref fqp, short)) = paths.get(&defid) { crate_paths.push((short, fqp.last().unwrap().clone())); Some(pathid) } else { @@ -214,7 +214,7 @@ crate fn get_index_search_type<'tcx>( fn get_index_type(clean_type: &clean::Type, cache: &Cache) -> RenderType { RenderType { - ty: clean_type.def_id_full(cache).map(FakeDefId::new_real), + ty: clean_type.def_id_full(cache), idx: None, name: get_index_type_name(clean_type, true).map(|s| s.as_str().to_ascii_lowercase()), generics: get_generics(clean_type, cache), @@ -256,7 +256,7 @@ fn get_generics(clean_type: &clean::Type, cache: &Cache) -> Option> .filter_map(|t| { get_index_type_name(t, false).map(|name| Generic { name: name.as_str().to_ascii_lowercase(), - defid: t.def_id_full(cache).map(FakeDefId::new_real), + defid: t.def_id_full(cache), idx: None, }) }) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 5b54b32e4ddea..e4dbf14586629 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -87,7 +87,7 @@ crate struct IndexItem { crate name: String, crate path: String, crate desc: String, - crate parent: Option, + crate parent: Option, crate parent_idx: Option, crate search_type: Option, crate aliases: Box<[String]>, @@ -96,7 +96,7 @@ crate struct IndexItem { /// A type used for the search index. #[derive(Debug)] crate struct RenderType { - ty: Option, + ty: Option, idx: Option, name: Option, generics: Option>, @@ -128,7 +128,7 @@ impl Serialize for RenderType { #[derive(Debug)] crate struct Generic { name: String, - defid: Option, + defid: Option, idx: Option, } @@ -2137,7 +2137,7 @@ fn sidebar_trait(cx: &Context<'_>, buf: &mut Buffer, it: &clean::Item, t: &clean "", ); - if let Some(implementors) = cx.cache.implementors.get(&it.def_id) { + if let Some(implementors) = cx.cache.implementors.get(&it.def_id.expect_real()) { let cache = cx.cache(); let mut res = implementors .iter() diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 20e82cf2caf38..ff639cb292462 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -642,7 +642,7 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra // If there are methods directly on this trait object, render them here. render_assoc_items(w, cx, it, it.def_id.expect_real(), AssocItemRender::All); - if let Some(implementors) = cx.cache.implementors.get(&it.def_id) { + if let Some(implementors) = cx.cache.implementors.get(&it.def_id.expect_real()) { // The DefId is for the first Type found with that name. The bool is // if any Types with the same name but different DefId have been found. let mut implementor_dups: FxHashMap = FxHashMap::default(); diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs index d0518cb6862fe..c25a73f58eb60 100644 --- a/src/librustdoc/html/render/write_shared.rs +++ b/src/librustdoc/html/render/write_shared.rs @@ -465,8 +465,6 @@ pub(super) fn write_shared( // Update the list of all implementors for traits let dst = cx.dst.join("implementors"); for (&did, imps) in &cx.cache.implementors { - let did = did.expect_real(); - // Private modules can leak through to this phase of rustdoc, which // could contain implementations for otherwise private types. In some // rare cases we could find an implementation for an item which wasn't diff --git a/src/librustdoc/json/conversions.rs b/src/librustdoc/json/conversions.rs index d2349b42ae930..5ac43c7364622 100644 --- a/src/librustdoc/json/conversions.rs +++ b/src/librustdoc/json/conversions.rs @@ -14,9 +14,8 @@ use rustc_span::Pos; use rustdoc_json_types::*; -use crate::clean; use crate::clean::utils::print_const_expr; -use crate::clean::FakeDefId; +use crate::clean::{self, FakeDefId}; use crate::formats::item_type::ItemType; use crate::json::JsonRenderer; use std::collections::HashSet; @@ -31,7 +30,7 @@ impl JsonRenderer<'_> { .into_iter() .flatten() .filter_map(|clean::ItemLink { link, did, .. }| { - did.map(|did| (link.clone(), from_def_id(did))) + did.map(|did| (link.clone(), from_def_id(did.into()))) }) .collect(); let docs = item.attrs.collapsed_doc_value(); diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs index d56acad60c00f..f8bd971081395 100644 --- a/src/librustdoc/json/mod.rs +++ b/src/librustdoc/json/mod.rs @@ -12,13 +12,14 @@ use std::path::PathBuf; use std::rc::Rc; use rustc_data_structures::fx::FxHashMap; +use rustc_hir::def_id::DefId; use rustc_middle::ty::TyCtxt; use rustc_session::Session; use rustdoc_json_types as types; use crate::clean; -use crate::clean::{ExternalCrate, FakeDefId}; +use crate::clean::ExternalCrate; use crate::config::RenderOptions; use crate::error::Error; use crate::formats::cache::Cache; @@ -42,7 +43,7 @@ impl JsonRenderer<'tcx> { self.tcx.sess } - fn get_trait_implementors(&mut self, id: FakeDefId) -> Vec { + fn get_trait_implementors(&mut self, id: DefId) -> Vec { Rc::clone(&self.cache) .implementors .get(&id) @@ -59,10 +60,10 @@ impl JsonRenderer<'tcx> { .unwrap_or_default() } - fn get_impls(&mut self, id: FakeDefId) -> Vec { + fn get_impls(&mut self, id: DefId) -> Vec { Rc::clone(&self.cache) .impls - .get(&id.expect_real()) + .get(&id) .map(|impls| { impls .iter() @@ -163,11 +164,11 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { let id = item.def_id; if let Some(mut new_item) = self.convert_item(item) { if let types::ItemEnum::Trait(ref mut t) = new_item.inner { - t.implementors = self.get_trait_implementors(id) + t.implementors = self.get_trait_implementors(id.expect_real()) } else if let types::ItemEnum::Struct(ref mut s) = new_item.inner { - s.impls = self.get_impls(id) + s.impls = self.get_impls(id.expect_real()) } else if let types::ItemEnum::Enum(ref mut e) = new_item.inner { - e.impls = self.get_impls(id) + e.impls = self.get_impls(id.expect_real()) } let removed = self.index.borrow_mut().insert(from_def_id(id), new_item.clone()); diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 8838dc57d5a05..88207b67743cf 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -30,9 +30,7 @@ use std::convert::{TryFrom, TryInto}; use std::mem; use std::ops::Range; -use crate::clean::{ - self, utils::find_nearest_parent_module, Crate, FakeDefId, Item, ItemLink, PrimitiveType, -}; +use crate::clean::{self, utils::find_nearest_parent_module, Crate, Item, ItemLink, PrimitiveType}; use crate::core::DocContext; use crate::fold::DocFolder; use crate::html::markdown::{markdown_links, MarkdownLink}; @@ -248,7 +246,7 @@ enum AnchorFailure { #[derive(Clone, Debug, Hash, PartialEq, Eq)] struct ResolutionInfo { - module_id: FakeDefId, + module_id: DefId, dis: Option, path_str: String, extra_fragment: Option, @@ -274,7 +272,7 @@ struct LinkCollector<'a, 'tcx> { /// /// The last module will be used if the parent scope of the current item is /// unknown. - mod_ids: Vec, + mod_ids: Vec, /// This is used to store the kind of associated items, /// because `clean` and the disambiguator code expect them to be different. /// See the code for associated items on inherent impls for details. @@ -861,7 +859,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { let inner_docs = item.inner_docs(self.cx.tcx); if item.is_mod() && inner_docs { - self.mod_ids.push(item.def_id); + self.mod_ids.push(item.def_id.expect_real()); } // We want to resolve in the lexical scope of the documentation. @@ -888,7 +886,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { Some(if item.is_mod() { if !inner_docs { - self.mod_ids.push(item.def_id); + self.mod_ids.push(item.def_id.expect_real()); } let ret = self.fold_item_recur(item); @@ -1070,11 +1068,8 @@ impl LinkCollector<'_, '_> { // we've already pushed this node onto the resolution stack but // for outer comments we explicitly try and resolve against the // parent_node first. - let base_node = if item.is_mod() && inner_docs { - self.mod_ids.last().copied() - } else { - parent_node.map(|id| FakeDefId::new_real(id)) - }; + let base_node = + if item.is_mod() && inner_docs { self.mod_ids.last().copied() } else { parent_node }; let mut module_id = if let Some(id) = base_node { id @@ -1119,7 +1114,7 @@ impl LinkCollector<'_, '_> { resolved_self = format!("self::{}", &path_str["crate::".len()..]); path_str = &resolved_self; } - module_id = FakeDefId::new_real(DefId { krate, index: CRATE_DEF_INDEX }); + module_id = DefId { krate, index: CRATE_DEF_INDEX }; } let (mut res, mut fragment) = self.resolve_with_disambiguator_cached( @@ -1180,8 +1175,8 @@ impl LinkCollector<'_, '_> { report_diagnostic(self.cx.tcx, BROKEN_INTRA_DOC_LINKS, &msg, &diag_info, callback); }; - let verify = |kind: DefKind, id: FakeDefId| { - let (kind, id) = self.kind_side_channel.take().unwrap_or((kind, id.expect_real())); + let verify = |kind: DefKind, id: DefId| { + let (kind, id) = self.kind_side_channel.take().unwrap_or((kind, id)); debug!("intra-doc link to {} resolved to {:?} (id: {:?})", path_str, res, id); // Disallow e.g. linking to enums with `struct@` @@ -1345,7 +1340,7 @@ impl LinkCollector<'_, '_> { match disambiguator.map(Disambiguator::ns) { Some(expected_ns @ (ValueNS | TypeNS)) => { - match self.resolve(path_str, expected_ns, base_node.expect_real(), extra_fragment) { + match self.resolve(path_str, expected_ns, base_node, extra_fragment) { Ok(res) => Some(res), Err(ErrorKind::Resolve(box mut kind)) => { // We only looked in one namespace. Try to give a better error if possible. @@ -1354,12 +1349,9 @@ impl LinkCollector<'_, '_> { // FIXME: really it should be `resolution_failure` that does this, not `resolve_with_disambiguator` // See https://github.com/rust-lang/rust/pull/76955#discussion_r493953382 for a good approach for &new_ns in &[other_ns, MacroNS] { - if let Some(res) = self.check_full_res( - new_ns, - path_str, - base_node.expect_real(), - extra_fragment, - ) { + if let Some(res) = + self.check_full_res(new_ns, path_str, base_node, extra_fragment) + { kind = ResolutionFailure::WrongNamespace { res, expected_ns }; break; } @@ -1381,14 +1373,9 @@ impl LinkCollector<'_, '_> { // Try everything! let mut candidates = PerNS { macro_ns: self - .resolve_macro(path_str, base_node.expect_real()) + .resolve_macro(path_str, base_node) .map(|res| (res, extra_fragment.clone())), - type_ns: match self.resolve( - path_str, - TypeNS, - base_node.expect_real(), - extra_fragment, - ) { + type_ns: match self.resolve(path_str, TypeNS, base_node, extra_fragment) { Ok(res) => { debug!("got res in TypeNS: {:?}", res); Ok(res) @@ -1399,12 +1386,7 @@ impl LinkCollector<'_, '_> { } Err(ErrorKind::Resolve(box kind)) => Err(kind), }, - value_ns: match self.resolve( - path_str, - ValueNS, - base_node.expect_real(), - extra_fragment, - ) { + value_ns: match self.resolve(path_str, ValueNS, base_node, extra_fragment) { Ok(res) => Ok(res), Err(ErrorKind::AnchorFailure(msg)) => { anchor_failure(self.cx, diag, msg); @@ -1460,17 +1442,14 @@ impl LinkCollector<'_, '_> { } } Some(MacroNS) => { - match self.resolve_macro(path_str, base_node.expect_real()) { + match self.resolve_macro(path_str, base_node) { Ok(res) => Some((res, extra_fragment.clone())), Err(mut kind) => { // `resolve_macro` only looks in the macro namespace. Try to give a better error if possible. for &ns in &[TypeNS, ValueNS] { - if let Some(res) = self.check_full_res( - ns, - path_str, - base_node.expect_real(), - extra_fragment, - ) { + if let Some(res) = + self.check_full_res(ns, path_str, base_node, extra_fragment) + { kind = ResolutionFailure::WrongNamespace { res, expected_ns: MacroNS }; break; diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index 90b797da24915..9b7e10b2688bb 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -126,7 +126,7 @@ crate fn collect_trait_impls(krate: Crate, cx: &mut DocContext<'_>) -> Crate { // Since only the `DefId` portion of the `Type` instances is known to be same for both the // `Deref` target type and the impl for type positions, this map of types is keyed by // `DefId` and for convenience uses a special cleaner that accepts `DefId`s directly. - if cleaner.keep_impl_with_def_id(&FakeDefId::new_real(*type_did)) { + if cleaner.keep_impl_with_def_id(FakeDefId::Real(*type_did)) { add_deref_target(&type_did_to_deref_target, &mut cleaner, type_did); } } @@ -206,13 +206,13 @@ impl BadImplStripper { } else if let Some(prim) = ty.primitive_type() { self.prims.contains(&prim) } else if let Some(did) = ty.def_id() { - self.keep_impl_with_def_id(&did.into()) + self.keep_impl_with_def_id(did.into()) } else { false } } - fn keep_impl_with_def_id(&self, did: &FakeDefId) -> bool { - self.items.contains(did) + fn keep_impl_with_def_id(&self, did: FakeDefId) -> bool { + self.items.contains(&did) } }