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.

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

[WIP] Remove fake IDs in rustdoc #75355

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/librustdoc/clean/auto_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/blanket_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
28 changes: 1 addition & 27 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -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)]
Expand Down
54 changes: 4 additions & 50 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@ 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},
Path,
};
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;
Expand All @@ -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};
Expand Down Expand Up @@ -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)>>,
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 1 addition & 4 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/collect_trait_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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); }));
}
}
}
Expand Down