Skip to content

Commit

Permalink
Auto merge of #91195 - camelid:path-did, r=jyn514
Browse files Browse the repository at this point in the history
rustdoc: Remove `ResolvedPath.did`

`ResolvedPath.did` was not actually the same as `.path.def_id()`. Instead,
`.did` referred to the `DefId` of the page to be used as a hyperlink target.
For example, a link to `Struct::method()` would use `Struct`'s `DefId` as its
`.did` field. This behavior is confusing, easy to accidentally misuse, and can
instead be obtained on-demand when computing hyperlink targets. It's also likely
part of the reason `kind_side_channel` exists. I'm currently working on some
experimental refactorings in `collect_intra_doc_links` that I believe require --
or at least benefit from -- removing `.did`.

r? `@jyn514`
  • Loading branch information
bors committed Nov 25, 2021
2 parents 23a4366 + 617bbb2 commit 862962b
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 42 deletions.
4 changes: 2 additions & 2 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1411,12 +1411,12 @@ impl<'tcx> Clean<Type> for Ty<'tcx> {
};
inline::record_extern_fqn(cx, did, kind);
let path = external_path(cx, did, false, vec![], substs);
ResolvedPath { path, did }
ResolvedPath { path }
}
ty::Foreign(did) => {
inline::record_extern_fqn(cx, did, ItemType::ForeignType);
let path = external_path(cx, did, false, vec![], InternalSubsts::empty());
ResolvedPath { path, did }
ResolvedPath { path }
}
ty::Dynamic(obj, ref reg) => {
// HACK: pick the first `did` as the `did` of the trait object. Someone
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1419,7 +1419,7 @@ crate enum Type {
/// A named type, which could be a trait.
///
/// This is mostly Rustdoc's version of [`hir::Path`]. It has to be different because Rustdoc's [`PathSegment`] can contain cleaned generics.
ResolvedPath { path: Path, did: DefId },
ResolvedPath { path: Path },
/// A `dyn Trait` object: `dyn for<'a> Trait<'a> + Send + 'static`
DynTrait(Vec<PolyTrait>, Option<Lifetime>),
/// A type parameter.
Expand Down Expand Up @@ -1522,7 +1522,7 @@ impl Type {

fn inner_def_id(&self, cache: Option<&Cache>) -> Option<DefId> {
let t: PrimitiveType = match *self {
ResolvedPath { did, .. } => return Some(did),
ResolvedPath { ref path } => return Some(path.def_id()),
DynTrait(ref bounds, _) => return Some(bounds[0].trait_.def_id()),
Primitive(p) => return cache.and_then(|c| c.primitive_locations.get(&p).cloned()),
BorrowedRef { type_: box Generic(..), .. } => PrimitiveType::Reference,
Expand Down
21 changes: 7 additions & 14 deletions src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ crate fn build_deref_target_impls(cx: &mut DocContext<'_>, items: &[Item], ret:
for &did in prim.impls(tcx).iter().filter(|did| !did.is_local()) {
inline::build_impl(cx, None, did, None, ret);
}
} else if let ResolvedPath { did, .. } = *target {
} else if let ResolvedPath { path } = target {
let did = path.def_id();
if !did.is_local() {
inline::build_impls(cx, None, did, None, ret);
}
Expand Down Expand Up @@ -360,8 +361,8 @@ crate fn resolve_type(cx: &mut DocContext<'_>, path: Path) -> Type {
Res::SelfTy(..) if path.segments.len() == 1 => Generic(kw::SelfUpper),
Res::Def(DefKind::TyParam, _) if path.segments.len() == 1 => Generic(path.segments[0].name),
_ => {
let did = register_res(cx, path.res);
ResolvedPath { path, did }
let _ = register_res(cx, path.res);
ResolvedPath { path }
}
}
}
Expand Down Expand Up @@ -393,20 +394,12 @@ crate fn register_res(cx: &mut DocContext<'_>, res: Res) -> DefId {
debug!("register_res({:?})", res);

let (did, kind) = match res {
Res::Def(DefKind::AssocTy | DefKind::AssocFn | DefKind::AssocConst, i) => {
// associated items are documented, but on the page of their parent
(cx.tcx.parent(i).unwrap(), ItemType::Trait)
}
Res::Def(DefKind::Variant, i) => {
// variant items are documented, but on the page of their parent
(cx.tcx.parent(i).expect("cannot get parent def id"), ItemType::Enum)
}
// Each of these have their own page.
// These should be added to the cache using `record_extern_fqn`.
Res::Def(
kind
@
(Fn | TyAlias | Enum | Trait | Struct | Union | Mod | ForeignTy | Const | Static
| Macro(..) | TraitAlias),
(AssocTy | AssocFn | AssocConst | Variant | Fn | TyAlias | Enum | Trait | Struct
| Union | Mod | ForeignTy | Const | Static | Macro(..) | TraitAlias),
i,
) => (i, kind.into()),
// This is part of a trait definition; document the trait.
Expand Down
10 changes: 5 additions & 5 deletions src/librustdoc/formats/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,8 +401,8 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
clean::ImplItem(ref i) => {
self.cache.parent_is_trait_impl = i.trait_.is_some();
match i.for_ {
clean::ResolvedPath { did, .. } => {
self.cache.parent_stack.push(did);
clean::ResolvedPath { ref path } => {
self.cache.parent_stack.push(path.def_id());
true
}
clean::DynTrait(ref bounds, _)
Expand Down Expand Up @@ -436,9 +436,9 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
// Note: matching twice to restrict the lifetime of the `i` borrow.
let mut dids = FxHashSet::default();
match i.for_ {
clean::ResolvedPath { did, .. }
| clean::BorrowedRef { type_: box clean::ResolvedPath { did, .. }, .. } => {
dids.insert(did);
clean::ResolvedPath { ref path }
| clean::BorrowedRef { type_: box clean::ResolvedPath { ref path }, .. } => {
dids.insert(path.def_id());
}
clean::DynTrait(ref bounds, _)
| clean::BorrowedRef { type_: box clean::DynTrait(ref bounds, _), .. } => {
Expand Down
16 changes: 14 additions & 2 deletions src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ use rustc_attr::{ConstStability, StabilityLevel};
use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::DefId;
use rustc_middle::ty;
use rustc_middle::ty::DefIdTree;
use rustc_middle::ty::TyCtxt;
use rustc_span::def_id::CRATE_DEF_INDEX;
use rustc_target::spec::abi::Abi;
Expand Down Expand Up @@ -502,7 +504,16 @@ crate fn href_with_root_path(
cx: &Context<'_>,
root_path: Option<&str>,
) -> Result<(String, ItemType, Vec<String>), HrefError> {
let cache = &cx.cache();
let tcx = cx.tcx();
let def_kind = tcx.def_kind(did);
let did = match def_kind {
DefKind::AssocTy | DefKind::AssocFn | DefKind::AssocConst | DefKind::Variant => {
// documented on their parent's page
tcx.parent(did).unwrap()
}
_ => did,
};
let cache = cx.cache();
let relative_to = &cx.current;
fn to_module_fqp(shortty: ItemType, fqp: &[String]) -> &[String] {
if shortty == ItemType::Module { fqp } else { &fqp[..fqp.len() - 1] }
Expand Down Expand Up @@ -751,8 +762,9 @@ fn fmt_type<'cx>(

match *t {
clean::Generic(name) => write!(f, "{}", name),
clean::ResolvedPath { did, ref path } => {
clean::ResolvedPath { ref path } => {
// Paths like `T::Output` and `Self::Output` should be rendered with all segments.
let did = path.def_id();
resolved_path(f, did, path, path.is_assoc_ty(), use_absolute, cx)
}
clean::DynTrait(ref bounds, ref lt) => {
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/html/render/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ crate fn get_real_types<'tcx>(
let mut ty_generics = Vec::new();
for bound in bound.get_bounds().unwrap_or(&[]) {
if let Some(path) = bound.get_trait_path() {
let ty = Type::ResolvedPath { did: path.def_id(), path };
let ty = Type::ResolvedPath { path };
get_real_types(generics, &ty, tcx, recurse + 1, &mut ty_generics, cache);
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1227,8 +1227,8 @@ fn should_render_item(item: &clean::Item, deref_mut_: bool, tcx: TyCtxt<'_>) ->
| SelfTy::SelfExplicit(clean::BorrowedRef { mutability, .. }) => {
(mutability == Mutability::Mut, false, false)
}
SelfTy::SelfExplicit(clean::ResolvedPath { did, .. }) => {
(false, Some(did) == tcx.lang_items().owned_box(), false)
SelfTy::SelfExplicit(clean::ResolvedPath { path }) => {
(false, Some(path.def_id()) == tcx.lang_items().owned_box(), false)
}
SelfTy::SelfValue => (false, false, true),
_ => (false, false, false),
Expand Down Expand Up @@ -2520,7 +2520,7 @@ fn collect_paths_for_type(first_ty: clean::Type, cache: &Cache) -> Vec<String> {
}

match ty {
clean::Type::ResolvedPath { did, .. } => process_path(did),
clean::Type::ResolvedPath { path } => process_path(path.def_id()),
clean::Type::Tuple(tys) => {
work.extend(tys.into_iter());
}
Expand Down
9 changes: 5 additions & 4 deletions src/librustdoc/html/render/print_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -727,10 +727,11 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra
let mut implementor_dups: FxHashMap<Symbol, (DefId, bool)> = FxHashMap::default();
for implementor in implementors {
match implementor.inner_impl().for_ {
clean::ResolvedPath { ref path, did, .. }
| clean::BorrowedRef {
type_: box clean::ResolvedPath { ref path, did, .. }, ..
} if !path.is_assoc_ty() => {
clean::ResolvedPath { ref path }
| clean::BorrowedRef { type_: box clean::ResolvedPath { ref path }, .. }
if !path.is_assoc_ty() =>
{
let did = path.def_id();
let &mut (prev_did, ref mut has_duplicates) =
implementor_dups.entry(path.last()).or_insert((did, false));
if prev_did != did {
Expand Down
14 changes: 5 additions & 9 deletions src/librustdoc/json/conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,7 @@ impl FromWithTcx<clean::GenericBound> for GenericBound {
match bound {
TraitBound(clean::PolyTrait { trait_, generic_params }, modifier) => {
// FIXME: should `trait_` be a clean::Path equivalent in JSON?
let trait_ =
clean::ResolvedPath { did: trait_.def_id(), path: trait_ }.into_tcx(tcx);
let trait_ = clean::ResolvedPath { path: trait_ }.into_tcx(tcx);
GenericBound::TraitBound {
trait_,
generic_params: generic_params.into_iter().map(|x| x.into_tcx(tcx)).collect(),
Expand All @@ -391,9 +390,9 @@ impl FromWithTcx<clean::Type> for Type {
fn from_tcx(ty: clean::Type, tcx: TyCtxt<'_>) -> Self {
use clean::Type::*;
match ty {
ResolvedPath { path, did } => Type::ResolvedPath {
ResolvedPath { path } => Type::ResolvedPath {
name: path.whole_name(),
id: from_item_id(did.into()),
id: from_item_id(path.def_id().into()),
args: path.segments.last().map(|args| Box::new(args.clone().args.into_tcx(tcx))),
param_names: Vec::new(),
},
Expand Down Expand Up @@ -436,7 +435,7 @@ impl FromWithTcx<clean::Type> for Type {
},
QPath { name, self_type, trait_, .. } => {
// FIXME: should `trait_` be a clean::Path equivalent in JSON?
let trait_ = ResolvedPath { did: trait_.def_id(), path: trait_ }.into_tcx(tcx);
let trait_ = ResolvedPath { path: trait_ }.into_tcx(tcx);
Type::QualifiedPath {
name: name.to_string(),
self_type: Box::new((*self_type).into_tcx(tcx)),
Expand Down Expand Up @@ -502,10 +501,7 @@ impl FromWithTcx<clean::Impl> for Impl {
let provided_trait_methods = impl_.provided_trait_methods(tcx);
let clean::Impl { unsafety, generics, trait_, for_, items, polarity, kind } = impl_;
// FIXME: should `trait_` be a clean::Path equivalent in JSON?
let trait_ = trait_.map(|path| {
let did = path.def_id();
clean::ResolvedPath { path, did }.into_tcx(tcx)
});
let trait_ = trait_.map(|path| clean::ResolvedPath { path }.into_tcx(tcx));
// FIXME: use something like ImplKind in JSON?
let (synthetic, blanket_impl) = match kind {
clean::ImplKind::Normal => (false, None),
Expand Down

0 comments on commit 862962b

Please sign in to comment.