Skip to content

Commit

Permalink
Rollup merge of rust-lang#83849 - jyn514:intra-doc-cleanup, r=bugadani
Browse files Browse the repository at this point in the history 
rustdoc: Cleanup handling of associated items for intra-doc links

Helps with rust-lang#83761 (right now the uses of the resolver are all intermingled with uses of the tyctxt). Best reviewed one commit at a time.

r? ``@bugadani`` maybe? Feel free to reassign :)
  • Loading branch information
@Dylan-DPC
Dylan-DPC authored Apr 5, 2021
2 parents 2bba1be + 3611a64 commit 317c514
Showing 1 changed file with 102 additions and 135 deletions.
237 changes: 102 additions & 135 deletions src/librustdoc/passes/collect_intra_doc_links.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,55 +368,28 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
}

/// Given a primitive type, try to resolve an associated item.
///
/// HACK(jynelson): `item_str` is passed in instead of derived from `item_name` so the
/// lifetimes on `&'path` will work.
fn resolve_primitive_associated_item(
&self,
prim_ty: PrimitiveType,
ns: Namespace,
module_id: DefId,
item_name: Symbol,
item_str: &'path str,
) -> Result<(Res, Option<String>), ErrorKind<'path>> {
) -> Option<(Res, String, Option<(DefKind, DefId)>)> {
let tcx = self.cx.tcx;

prim_ty
.impls(tcx)
.into_iter()
.find_map(|&impl_| {
tcx.associated_items(impl_)
.find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, impl_)
.map(|item| {
let kind = item.kind;
self.kind_side_channel.set(Some((kind.as_def_kind(), item.def_id)));
match kind {
ty::AssocKind::Fn => "method",
ty::AssocKind::Const => "associatedconstant",
ty::AssocKind::Type => "associatedtype",
}
})
.map(|out| {
(
Res::Primitive(prim_ty),
Some(format!("{}#{}.{}", prim_ty.as_str(), out, item_str)),
)
})
})
.ok_or_else(|| {
debug!(
"returning primitive error for {}::{} in {} namespace",
prim_ty.as_str(),
item_name,
ns.descr()
);
ResolutionFailure::NotResolved {
module_id,
partial_res: Some(Res::Primitive(prim_ty)),
unresolved: item_str.into(),
}
.into()
})
prim_ty.impls(tcx).into_iter().find_map(|&impl_| {
tcx.associated_items(impl_)
.find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, impl_)
.map(|item| {
let kind = item.kind;
let out = match kind {
ty::AssocKind::Fn => "method",
ty::AssocKind::Const => "associatedconstant",
ty::AssocKind::Type => "associatedtype",
};
let fragment = format!("{}#{}.{}", prim_ty.as_str(), out, item_name);
(Res::Primitive(prim_ty), fragment, Some((kind.as_def_kind(), item.def_id)))
})
})
}

/// Resolves a string as a macro.
Expand Down Expand Up @@ -490,8 +463,6 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
module_id: DefId,
extra_fragment: &Option<String>,
) -> Result<(Res, Option<String>), ErrorKind<'path>> {
let tcx = self.cx.tcx;

if let Some(res) = self.resolve_path(path_str, ns, module_id) {
match res {
// FIXME(#76467): make this fallthrough to lookup the associated
Expand Down Expand Up @@ -534,29 +505,58 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
}
})?;

// FIXME: are these both necessary?
let ty_res = if let Some(ty_res) = resolve_primitive(&path_root, TypeNS)
// FIXME(#83862): this arbitrarily gives precedence to primitives over modules to support
// links to primitives when `#[doc(primitive)]` is present. It should give an ambiguity
// error instead and special case *only* modules with `#[doc(primitive)]`, not all
// primitives.
resolve_primitive(&path_root, TypeNS)
.or_else(|| self.resolve_path(&path_root, TypeNS, module_id))
{
ty_res
} else {
// FIXME: this is duplicated on the end of this function.
return if ns == Namespace::ValueNS {
self.variant_field(path_str, module_id)
} else {
Err(ResolutionFailure::NotResolved {
module_id,
partial_res: None,
unresolved: path_root.into(),
.and_then(|ty_res| {
let (res, fragment, side_channel) =
self.resolve_associated_item(ty_res, item_name, ns, module_id)?;
let result = if extra_fragment.is_some() {
let diag_res = side_channel.map_or(res, |(k, r)| Res::Def(k, r));
Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(diag_res)))
} else {
// HACK(jynelson): `clean` expects the type, not the associated item
// but the disambiguator logic expects the associated item.
// Store the kind in a side channel so that only the disambiguator logic looks at it.
if let Some((kind, id)) = side_channel {
self.kind_side_channel.set(Some((kind, id)));
}
Ok((res, Some(fragment)))
};
Some(result)
})
.unwrap_or_else(|| {
if ns == Namespace::ValueNS {
self.variant_field(path_str, module_id)
} else {
Err(ResolutionFailure::NotResolved {
module_id,
partial_res: None,
unresolved: path_root.into(),
}
.into())
}
.into())
};
};
})
}

/// Returns:
/// - None if no associated item was found
/// - Some((_, _, Some(_))) if an item was found and should go through a side channel
/// - Some((_, _, None)) otherwise
fn resolve_associated_item(
&mut self,
root_res: Res,
item_name: Symbol,
ns: Namespace,
module_id: DefId,
) -> Option<(Res, String, Option<(DefKind, DefId)>)> {
let tcx = self.cx.tcx;

let res = match ty_res {
Res::Primitive(prim) => Some(
self.resolve_primitive_associated_item(prim, ns, module_id, item_name, item_str),
),
match root_res {
Res::Primitive(prim) => self.resolve_primitive_associated_item(prim, ns, item_name),
Res::Def(
DefKind::Struct
| DefKind::Union
Expand Down Expand Up @@ -599,59 +599,42 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
ty::AssocKind::Const => "associatedconstant",
ty::AssocKind::Type => "associatedtype",
};
Some(if extra_fragment.is_some() {
Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(ty_res)))
} else {
// HACK(jynelson): `clean` expects the type, not the associated item
// but the disambiguator logic expects the associated item.
// Store the kind in a side channel so that only the disambiguator logic looks at it.
self.kind_side_channel.set(Some((kind.as_def_kind(), id)));
Ok((ty_res, Some(format!("{}.{}", out, item_str))))
})
} else if ns == Namespace::ValueNS {
debug!("looking for variants or fields named {} for {:?}", item_name, did);
// FIXME(jynelson): why is this different from
// `variant_field`?
match tcx.type_of(did).kind() {
ty::Adt(def, _) => {
let field = if def.is_enum() {
def.all_fields().find(|item| item.ident.name == item_name)
} else {
def.non_enum_variant()
.fields
.iter()
.find(|item| item.ident.name == item_name)
};
field.map(|item| {
if extra_fragment.is_some() {
let res = Res::Def(
if def.is_enum() {
DefKind::Variant
} else {
DefKind::Field
},
item.did,
);
Err(ErrorKind::AnchorFailure(
AnchorFailure::RustdocAnchorConflict(res),
))
} else {
Ok((
ty_res,
Some(format!(
"{}.{}",
if def.is_enum() { "variant" } else { "structfield" },
item.ident
)),
))
}
})
}
_ => None,
}
} else {
None
// HACK(jynelson): `clean` expects the type, not the associated item
// but the disambiguator logic expects the associated item.
// Store the kind in a side channel so that only the disambiguator logic looks at it.
return Some((
root_res,
format!("{}.{}", out, item_name),
Some((kind.as_def_kind(), id)),
));
}

if ns != Namespace::ValueNS {
return None;
}
debug!("looking for variants or fields named {} for {:?}", item_name, did);
// FIXME: this doesn't really belong in `associated_item` (maybe `variant_field` is better?)
// NOTE: it's different from variant_field because it resolves fields and variants,
// not variant fields (2 path segments, not 3).
let def = match tcx.type_of(did).kind() {
ty::Adt(def, _) => def,
_ => return None,
};
let field = if def.is_enum() {
def.all_fields().find(|item| item.ident.name == item_name)
} else {
def.non_enum_variant().fields.iter().find(|item| item.ident.name == item_name)
}?;
let kind = if def.is_enum() { DefKind::Variant } else { DefKind::Field };
Some((
root_res,
format!(
"{}.{}",
if def.is_enum() { "variant" } else { "structfield" },
field.ident
),
Some((kind, field.did)),
))
}
Res::Def(DefKind::Trait, did) => tcx
.associated_items(did)
Expand All @@ -669,27 +652,11 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
}
};

if extra_fragment.is_some() {
Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(ty_res)))
} else {
let res = Res::Def(item.kind.as_def_kind(), item.def_id);
Ok((res, Some(format!("{}.{}", kind, item_str))))
}
let res = Res::Def(item.kind.as_def_kind(), item.def_id);
(res, format!("{}.{}", kind, item_name), None)
}),
_ => None,
};
res.unwrap_or_else(|| {
if ns == Namespace::ValueNS {
self.variant_field(path_str, module_id)
} else {
Err(ResolutionFailure::NotResolved {
module_id,
partial_res: Some(ty_res),
unresolved: item_str.into(),
}
.into())
}
})
}
}

/// Used for reporting better errors.
Expand Down

0 comments on commit 317c514

Please sign in to comment.