From 9cfe1a5b4b9464ea0bfb271201bb803305cc94d4 Mon Sep 17 00:00:00 2001
From: Joshua Nelson <jyn514@gmail.com>
Date: Mon, 10 Aug 2020 00:09:22 -0400
Subject: [PATCH 1/2] Remove fake IDs in rustdoc
They're confusing, cause crashes, and aren't necessary.
---
src/librustdoc/clean/auto_trait.rs | 2 +-
src/librustdoc/clean/blanket_impl.rs | 2 +-
src/librustdoc/clean/types.rs | 28 +---------
src/librustdoc/core.rs | 54 ++-----------------
.../passes/collect_intra_doc_links.rs | 5 +-
5 files changed, 8 insertions(+), 83 deletions(-)
diff --git a/src/librustdoc/clean/auto_trait.rs b/src/librustdoc/clean/auto_trait.rs
index 9c44d27447db..e4758665c543 100644
--- a/src/librustdoc/clean/auto_trait.rs
+++ b/src/librustdoc/clean/auto_trait.rs
@@ -122,7 +122,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> {
name: None,
attrs: Default::default(),
visibility: Inherited,
- def_id: self.cx.next_def_id(param_env_def_id.krate),
+ def_id: param_env_def_id,
stability: None,
deprecation: None,
inner: ImplItem(Impl {
diff --git a/src/librustdoc/clean/blanket_impl.rs b/src/librustdoc/clean/blanket_impl.rs
index 3d2785541bee..cca9532f9629 100644
--- a/src/librustdoc/clean/blanket_impl.rs
+++ b/src/librustdoc/clean/blanket_impl.rs
@@ -96,7 +96,7 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> {
name: None,
attrs: Default::default(),
visibility: Inherited,
- def_id: self.cx.next_def_id(impl_def_id.krate),
+ def_id: impl_def_id,
stability: None,
deprecation: None,
inner: ImplItem(Impl {
diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
index 50eca75d7cab..6e96669f78af 100644
--- a/src/librustdoc/clean/types.rs
+++ b/src/librustdoc/clean/types.rs
@@ -1,6 +1,5 @@
use std::cell::RefCell;
use std::default::Default;
-use std::fmt;
use std::hash::{Hash, Hasher};
use std::iter::FromIterator;
use std::num::NonZeroU32;
@@ -70,7 +69,7 @@ pub struct ExternalCrate {
/// Anything with a source location and set of attributes and, optionally, a
/// name. That is, anything that can be documented. This doesn't correspond
/// directly to the AST's concept of an item; it's a strict superset.
-#[derive(Clone)]
+#[derive(Clone, Debug)]
pub struct Item {
/// Stringified span
pub source: Span,
@@ -84,24 +83,6 @@ pub struct Item {
pub deprecation: Option<Deprecation>,
}
-impl fmt::Debug for Item {
- fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
- let fake = self.is_fake();
- let def_id: &dyn fmt::Debug = if fake { &"**FAKE**" } else { &self.def_id };
-
- fmt.debug_struct("Item")
- .field("source", &self.source)
- .field("name", &self.name)
- .field("attrs", &self.attrs)
- .field("inner", &self.inner)
- .field("visibility", &self.visibility)
- .field("def_id", def_id)
- .field("stability", &self.stability)
- .field("deprecation", &self.deprecation)
- .finish()
- }
-}
-
impl Item {
/// Finds the `doc` attribute as a NameValue and returns the corresponding
/// value found.
@@ -230,13 +211,6 @@ impl Item {
_ => false,
}
}
-
- /// See comments on next_def_id
- pub fn is_fake(&self) -> bool {
- MAX_DEF_ID.with(|m| {
- m.borrow().get(&self.def_id.krate).map(|id| self.def_id >= *id).unwrap_or(false)
- })
- }
}
#[derive(Clone, Debug)]
diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs
index cbd0ca0de641..01ac69d6dc87 100644
--- a/src/librustdoc/core.rs
+++ b/src/librustdoc/core.rs
@@ -6,7 +6,7 @@ use rustc_errors::emitter::{Emitter, EmitterWriter};
use rustc_errors::json::JsonEmitter;
use rustc_feature::UnstableFeatures;
use rustc_hir::def::{Namespace::TypeNS, Res};
-use rustc_hir::def_id::{CrateNum, DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE};
+use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE};
use rustc_hir::HirId;
use rustc_hir::{
intravisit::{self, NestedVisitorMap, Visitor},
@@ -14,7 +14,6 @@ use rustc_hir::{
};
use rustc_interface::interface;
use rustc_middle::hir::map::Map;
-use rustc_middle::middle::cstore::CrateStore;
use rustc_middle::middle::privacy::AccessLevels;
use rustc_middle::ty::{Ty, TyCtxt};
use rustc_resolve as resolve;
@@ -31,7 +30,7 @@ use std::mem;
use std::rc::Rc;
use crate::clean;
-use crate::clean::{AttributesExt, MAX_DEF_ID};
+use crate::clean::AttributesExt;
use crate::config::RenderInfo;
use crate::config::{Options as RustdocOptions, RenderOptions};
use crate::passes::{self, Condition::*, ConditionalPass};
@@ -61,8 +60,6 @@ pub struct DocContext<'tcx> {
pub ct_substs: RefCell<FxHashMap<DefId, clean::Constant>>,
/// Table synthetic type parameter for `impl Trait` in argument position -> bounds
pub impl_trait_bounds: RefCell<FxHashMap<ImplTraitParam, Vec<clean::GenericBound>>>,
- pub fake_def_ids: RefCell<FxHashMap<CrateNum, DefId>>,
- pub all_fake_def_ids: RefCell<FxHashSet<DefId>>,
/// Auto-trait or blanket impls processed so far, as `(self_ty, trait_def_id)`.
// FIXME(eddyb) make this a `ty::TraitRef<'tcx>` set.
pub generated_synthetics: RefCell<FxHashSet<(Ty<'tcx>, DefId)>>,
@@ -107,50 +104,9 @@ impl<'tcx> DocContext<'tcx> {
r
}
- // This is an ugly hack, but it's the simplest way to handle synthetic impls without greatly
- // refactoring either librustdoc or librustc_middle. In particular, allowing new DefIds to be
- // registered after the AST is constructed would require storing the defid mapping in a
- // RefCell, decreasing the performance for normal compilation for very little gain.
- //
- // Instead, we construct 'fake' def ids, which start immediately after the last DefId.
- // In the Debug impl for clean::Item, we explicitly check for fake
- // def ids, as we'll end up with a panic if we use the DefId Debug impl for fake DefIds
- pub fn next_def_id(&self, crate_num: CrateNum) -> DefId {
- let start_def_id = {
- let next_id = if crate_num == LOCAL_CRATE {
- self.tcx.hir().definitions().def_path_table().next_id()
- } else {
- self.enter_resolver(|r| r.cstore().def_path_table(crate_num).next_id())
- };
-
- DefId { krate: crate_num, index: next_id }
- };
-
- let mut fake_ids = self.fake_def_ids.borrow_mut();
-
- let def_id = *fake_ids.entry(crate_num).or_insert(start_def_id);
- fake_ids.insert(
- crate_num,
- DefId { krate: crate_num, index: DefIndex::from(def_id.index.index() + 1) },
- );
-
- MAX_DEF_ID.with(|m| {
- m.borrow_mut().entry(def_id.krate).or_insert(start_def_id);
- });
-
- self.all_fake_def_ids.borrow_mut().insert(def_id);
-
- def_id
- }
-
- /// Like the function of the same name on the HIR map, but skips calling it on fake DefIds.
- /// (This avoids a slice-index-out-of-bounds panic.)
+ /// Like the function of the same name on the HIR map, but more concisely.
pub fn as_local_hir_id(&self, def_id: DefId) -> Option<HirId> {
- if self.all_fake_def_ids.borrow().contains(&def_id) {
- None
- } else {
- def_id.as_local().map(|def_id| self.tcx.hir().as_local_hir_id(def_id))
- }
+ def_id.as_local().map(|def_id| self.tcx.hir().as_local_hir_id(def_id))
}
pub fn stability(&self, id: HirId) -> Option<attr::Stability> {
@@ -486,8 +442,6 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
lt_substs: Default::default(),
ct_substs: Default::default(),
impl_trait_bounds: Default::default(),
- fake_def_ids: Default::default(),
- all_fake_def_ids: Default::default(),
generated_synthetics: Default::default(),
auto_traits: tcx
.all_traits(LOCAL_CRATE)
diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
index f84486347afc..2cc23d3b8e0b 100644
--- a/src/librustdoc/passes/collect_intra_doc_links.rs
+++ b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -460,10 +460,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
fn fold_item(&mut self, mut item: Item) -> Option<Item> {
use rustc_middle::ty::DefIdTree;
- let parent_node = if item.is_fake() {
- // FIXME: is this correct?
- None
- } else {
+ let parent_node = {
let mut current = item.def_id;
// The immediate parent might not always be a module.
// Find the first parent which is.
From 758829e927e58d674d9ec27f3525c401b97f9926 Mon Sep 17 00:00:00 2001
From: Joshua Nelson <jyn514@gmail.com>
Date: Mon, 10 Aug 2020 01:04:22 -0400
Subject: [PATCH 2/2] Don't ignore impls that have already been seen
This code path was never taken before because all the ids were fake. So
the ids would never have been seen before.
---
src/librustdoc/passes/collect_trait_impls.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs
index 3000afde0c25..d1b3acd24da4 100644
--- a/src/librustdoc/passes/collect_trait_impls.rs
+++ b/src/librustdoc/passes/collect_trait_impls.rs
@@ -76,7 +76,7 @@ pub fn collect_trait_impls(krate: Crate, cx: &DocContext<'_>) -> Crate {
let impls = get_auto_trait_and_blanket_impls(cx, self_ty, def_id);
let mut renderinfo = cx.renderinfo.borrow_mut();
- new_items.extend(impls.filter(|i| renderinfo.inlined.insert(i.def_id)));
+ new_items.extend(impls.inspect(|i| { renderinfo.inlined.insert(i.def_id); }));
}
}
}