rustdoc: Early doc link resolution fixes and refactorings

This commit is contained in:
Vadim Petrochenkov 2022-04-05 23:46:44 +03:00
parent c2afaba465
commit 69d6c3b2e6
10 changed files with 180 additions and 113 deletions

View File

@ -1169,14 +1169,18 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
}
}
fn get_associated_item_def_ids(self, tcx: TyCtxt<'tcx>, id: DefIndex) -> &'tcx [DefId] {
if let Some(children) = self.root.tables.children.get(self, id) {
tcx.arena.alloc_from_iter(
children.decode((self, tcx.sess)).map(|child_index| self.local_def_id(child_index)),
)
} else {
&[]
}
fn get_associated_item_def_ids(
self,
id: DefIndex,
sess: &'a Session,
) -> impl Iterator<Item = DefId> + 'a {
self.root
.tables
.children
.get(self, id)
.unwrap_or_else(Lazy::empty)
.decode((self, sess))
.map(move |child_index| self.local_def_id(child_index))
}
fn get_associated_item(self, id: DefIndex) -> ty::AssocItem {

View File

@ -160,7 +160,9 @@ provide! { <'tcx> tcx, def_id, other, cdata,
let _ = cdata;
tcx.calculate_dtor(def_id, |_,_| Ok(()))
}
associated_item_def_ids => { cdata.get_associated_item_def_ids(tcx, def_id.index) }
associated_item_def_ids => {
tcx.arena.alloc_from_iter(cdata.get_associated_item_def_ids(def_id.index, tcx.sess))
}
associated_item => { cdata.get_associated_item(def_id.index) }
inherent_impls => { cdata.get_inherent_implementations_for_type(tcx, def_id.index) }
is_foreign_item => { cdata.is_foreign_item(def_id.index) }

View File

@ -1248,7 +1248,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
};
let binding = (res, vis, span, expansion).to_name_binding(self.r.arenas);
self.r.set_binding_parent_module(binding, parent_scope.module);
self.r.all_macros.insert(ident.name, res);
self.r.all_macro_rules.insert(ident.name, res);
if is_macro_export {
let module = self.r.graph_root;
self.r.define(module, ident, MacroNS, (res, vis, span, expansion, IsMacroExport));

View File

@ -1003,7 +1003,8 @@ pub struct Resolver<'a> {
registered_attrs: FxHashSet<Ident>,
registered_tools: RegisteredTools,
macro_use_prelude: FxHashMap<Symbol, &'a NameBinding<'a>>,
all_macros: FxHashMap<Symbol, Res>,
/// FIXME: The only user of this is a doc link resolution hack for rustdoc.
all_macro_rules: FxHashMap<Symbol, Res>,
macro_map: FxHashMap<DefId, Lrc<SyntaxExtension>>,
dummy_ext_bang: Lrc<SyntaxExtension>,
dummy_ext_derive: Lrc<SyntaxExtension>,
@ -1385,7 +1386,7 @@ impl<'a> Resolver<'a> {
registered_attrs,
registered_tools,
macro_use_prelude: FxHashMap::default(),
all_macros: FxHashMap::default(),
all_macro_rules: Default::default(),
macro_map: FxHashMap::default(),
dummy_ext_bang: Lrc::new(SyntaxExtension::dummy_bang(session.edition())),
dummy_ext_derive: Lrc::new(SyntaxExtension::dummy_derive(session.edition())),
@ -3311,8 +3312,8 @@ impl<'a> Resolver<'a> {
}
// For rustdoc.
pub fn all_macros(&self) -> &FxHashMap<Symbol, Res> {
&self.all_macros
pub fn take_all_macro_rules(&mut self) -> FxHashMap<Symbol, Res> {
mem::take(&mut self.all_macro_rules)
}
/// For rustdoc.

View File

@ -1,3 +1,4 @@
use rustc_ast::NodeId;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::sync::{self, Lrc};
use rustc_errors::emitter::{Emitter, EmitterWriter};
@ -17,7 +18,7 @@ use rustc_session::lint;
use rustc_session::DiagnosticOutput;
use rustc_session::Session;
use rustc_span::symbol::sym;
use rustc_span::{source_map, Span};
use rustc_span::{source_map, Span, Symbol};
use std::cell::RefCell;
use std::lazy::SyncLazy;
@ -38,6 +39,7 @@ crate struct ResolverCaches {
crate traits_in_scope: DefIdMap<Vec<TraitCandidate>>,
crate all_traits: Option<Vec<DefId>>,
crate all_trait_impls: Option<Vec<DefId>>,
crate all_macro_rules: FxHashMap<Symbol, Res<NodeId>>,
}
crate struct DocContext<'tcx> {

View File

@ -10,6 +10,7 @@
#![feature(control_flow_enum)]
#![feature(box_syntax)]
#![feature(drain_filter)]
#![feature(let_chains)]
#![feature(let_else)]
#![feature(nll)]
#![feature(test)]

View File

@ -2,13 +2,11 @@
//!
//! [RFC 1946]: https://github.com/rust-lang/rfcs/blob/master/text/1946-intra-rustdoc-links.md
use pulldown_cmark::LinkType;
use rustc_data_structures::{fx::FxHashMap, intern::Interned, stable_set::FxHashSet};
use rustc_errors::{Applicability, Diagnostic};
use rustc_hir::def::{
DefKind,
Namespace::{self, *},
PerNS,
};
use rustc_hir::def::Namespace::*;
use rustc_hir::def::{DefKind, Namespace, PerNS};
use rustc_hir::def_id::{DefId, CRATE_DEF_ID};
use rustc_hir::Mutability;
use rustc_middle::ty::{DefIdTree, Ty, TyCtxt};
@ -19,10 +17,7 @@ use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_span::{BytePos, DUMMY_SP};
use smallvec::{smallvec, SmallVec};
use pulldown_cmark::LinkType;
use std::borrow::Cow;
use std::convert::{TryFrom, TryInto};
use std::fmt::Write;
use std::mem;
use std::ops::Range;
@ -487,25 +482,13 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
item_id: ItemId,
module_id: DefId,
) -> Result<Res, ResolutionFailure<'a>> {
self.cx.enter_resolver(|resolver| {
// NOTE: this needs 2 separate lookups because `resolve_rustdoc_path` doesn't take
// lexical scope into account (it ignores all macros not defined at the mod-level)
debug!("resolving {} as a macro in the module {:?}", path_str, module_id);
if let Some(res) = resolver.resolve_rustdoc_path(path_str, MacroNS, module_id) {
// don't resolve builtins like `#[derive]`
if let Ok(res) = res.try_into() {
return Ok(res);
}
}
if let Some(&res) = resolver.all_macros().get(&Symbol::intern(path_str)) {
return Ok(res.try_into().unwrap());
}
Err(ResolutionFailure::NotResolved {
self.resolve_path(path_str, MacroNS, item_id, module_id).ok_or_else(|| {
ResolutionFailure::NotResolved {
item_id,
module_id,
partial_res: None,
unresolved: path_str.into(),
})
}
})
}
@ -539,6 +522,21 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
})
}
/// HACK: Try to search the macro name in the list of all `macro_rules` items in the crate.
/// Used when nothing else works, may often give an incorrect result.
fn resolve_macro_rules(&self, path_str: &str, ns: Namespace) -> Option<Res> {
if ns != MacroNS {
return None;
}
self.cx
.resolver_caches
.all_macro_rules
.get(&Symbol::intern(path_str))
.copied()
.and_then(|res| res.try_into().ok())
}
/// Convenience wrapper around `resolve_rustdoc_path`.
///
/// This also handles resolving `true` and `false` as booleans.
@ -560,7 +558,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
.cx
.enter_resolver(|resolver| resolver.resolve_rustdoc_path(path_str, ns, module_id))
.and_then(|res| res.try_into().ok())
.or_else(|| resolve_primitive(path_str, ns));
.or_else(|| resolve_primitive(path_str, ns))
.or_else(|| self.resolve_macro_rules(path_str, ns));
debug!("{} resolved to {:?} in namespace {:?}", path_str, result, ns);
result
}

View File

@ -1,4 +1,4 @@
use crate::clean;
use crate::clean::Attributes;
use crate::core::ResolverCaches;
use crate::html::markdown::markdown_links;
use crate::passes::collect_intra_doc_links::preprocess_link;
@ -24,7 +24,7 @@ crate fn early_resolve_intra_doc_links(
externs: Externs,
document_private_items: bool,
) -> ResolverCaches {
let mut loader = IntraLinkCrateLoader {
let mut link_resolver = EarlyDocLinkResolver {
resolver,
current_mod: CRATE_DEF_ID,
visited_mods: Default::default(),
@ -36,27 +36,28 @@ crate fn early_resolve_intra_doc_links(
// Overridden `visit_item` below doesn't apply to the crate root,
// so we have to visit its attributes and reexports separately.
loader.load_links_in_attrs(&krate.attrs);
loader.process_module_children_or_reexports(CRATE_DEF_ID.to_def_id());
visit::walk_crate(&mut loader, krate);
loader.add_foreign_traits_in_scope();
link_resolver.load_links_in_attrs(&krate.attrs);
link_resolver.process_module_children_or_reexports(CRATE_DEF_ID.to_def_id());
visit::walk_crate(&mut link_resolver, krate);
link_resolver.process_extern_impls();
// FIXME: somehow rustdoc is still missing crates even though we loaded all
// the known necessary crates. Load them all unconditionally until we find a way to fix this.
// DO NOT REMOVE THIS without first testing on the reproducer in
// https://github.com/jyn514/objr/commit/edcee7b8124abf0e4c63873e8422ff81beb11ebb
for (extern_name, _) in externs.iter().filter(|(_, entry)| entry.add_prelude) {
loader.resolver.resolve_rustdoc_path(extern_name, TypeNS, CRATE_DEF_ID.to_def_id());
link_resolver.resolver.resolve_rustdoc_path(extern_name, TypeNS, CRATE_DEF_ID.to_def_id());
}
ResolverCaches {
traits_in_scope: loader.traits_in_scope,
all_traits: Some(loader.all_traits),
all_trait_impls: Some(loader.all_trait_impls),
traits_in_scope: link_resolver.traits_in_scope,
all_traits: Some(link_resolver.all_traits),
all_trait_impls: Some(link_resolver.all_trait_impls),
all_macro_rules: link_resolver.resolver.take_all_macro_rules(),
}
}
struct IntraLinkCrateLoader<'r, 'ra> {
struct EarlyDocLinkResolver<'r, 'ra> {
resolver: &'r mut Resolver<'ra>,
current_mod: LocalDefId,
visited_mods: DefIdSet,
@ -66,7 +67,7 @@ struct IntraLinkCrateLoader<'r, 'ra> {
document_private_items: bool,
}
impl IntraLinkCrateLoader<'_, '_> {
impl EarlyDocLinkResolver<'_, '_> {
fn add_traits_in_scope(&mut self, def_id: DefId) {
// Calls to `traits_in_scope` are expensive, so try to avoid them if only possible.
// Keys in the `traits_in_scope` cache are always module IDs.
@ -101,64 +102,83 @@ impl IntraLinkCrateLoader<'_, '_> {
/// That pass filters impls using type-based information, but we don't yet have such
/// information here, so we just conservatively calculate traits in scope for *all* modules
/// having impls in them.
fn add_foreign_traits_in_scope(&mut self) {
for cnum in Vec::from_iter(self.resolver.cstore().crates_untracked()) {
let all_traits = Vec::from_iter(self.resolver.cstore().traits_in_crate_untracked(cnum));
let all_trait_impls =
Vec::from_iter(self.resolver.cstore().trait_impls_in_crate_untracked(cnum));
let all_inherent_impls =
Vec::from_iter(self.resolver.cstore().inherent_impls_in_crate_untracked(cnum));
let all_incoherent_impls =
Vec::from_iter(self.resolver.cstore().incoherent_impls_in_crate_untracked(cnum));
fn process_extern_impls(&mut self) {
// FIXME: Need to resolve doc links on all these impl and trait items below.
// Resolving links in already existing crates may trigger loading of new crates.
let mut start_cnum = 0;
loop {
let crates = Vec::from_iter(self.resolver.cstore().crates_untracked());
for &cnum in &crates[start_cnum..] {
let all_traits =
Vec::from_iter(self.resolver.cstore().traits_in_crate_untracked(cnum));
let all_trait_impls =
Vec::from_iter(self.resolver.cstore().trait_impls_in_crate_untracked(cnum));
let all_inherent_impls =
Vec::from_iter(self.resolver.cstore().inherent_impls_in_crate_untracked(cnum));
let all_incoherent_impls = Vec::from_iter(
self.resolver.cstore().incoherent_impls_in_crate_untracked(cnum),
);
// Querying traits in scope is expensive so we try to prune the impl and traits lists
// using privacy, private traits and impls from other crates are never documented in
// the current crate, and links in their doc comments are not resolved.
for &def_id in &all_traits {
if self.resolver.cstore().visibility_untracked(def_id) == Visibility::Public {
// Querying traits in scope is expensive so we try to prune the impl and traits lists
// using privacy, private traits and impls from other crates are never documented in
// the current crate, and links in their doc comments are not resolved.
for &def_id in &all_traits {
if self.resolver.cstore().visibility_untracked(def_id) == Visibility::Public {
self.add_traits_in_parent_scope(def_id);
}
}
for &(trait_def_id, impl_def_id, simplified_self_ty) in &all_trait_impls {
if self.resolver.cstore().visibility_untracked(trait_def_id)
== Visibility::Public
&& simplified_self_ty.and_then(|ty| ty.def()).map_or(true, |ty_def_id| {
self.resolver.cstore().visibility_untracked(ty_def_id)
== Visibility::Public
})
{
self.add_traits_in_parent_scope(impl_def_id);
}
}
for (ty_def_id, impl_def_id) in all_inherent_impls {
if self.resolver.cstore().visibility_untracked(ty_def_id) == Visibility::Public
{
self.add_traits_in_parent_scope(impl_def_id);
}
}
for def_id in all_incoherent_impls {
self.add_traits_in_parent_scope(def_id);
}
}
for &(trait_def_id, impl_def_id, simplified_self_ty) in &all_trait_impls {
if self.resolver.cstore().visibility_untracked(trait_def_id) == Visibility::Public
&& simplified_self_ty.and_then(|ty| ty.def()).map_or(true, |ty_def_id| {
self.resolver.cstore().visibility_untracked(ty_def_id) == Visibility::Public
})
{
self.add_traits_in_parent_scope(impl_def_id);
}
}
for (ty_def_id, impl_def_id) in all_inherent_impls {
if self.resolver.cstore().visibility_untracked(ty_def_id) == Visibility::Public {
self.add_traits_in_parent_scope(impl_def_id);
}
}
for def_id in all_incoherent_impls {
self.add_traits_in_parent_scope(def_id);
self.all_traits.extend(all_traits);
self.all_trait_impls
.extend(all_trait_impls.into_iter().map(|(_, def_id, _)| def_id));
}
self.all_traits.extend(all_traits);
self.all_trait_impls.extend(all_trait_impls.into_iter().map(|(_, def_id, _)| def_id));
if crates.len() > start_cnum {
start_cnum = crates.len();
} else {
break;
}
}
}
fn load_links_in_attrs(&mut self, attrs: &[ast::Attribute]) {
// FIXME: this needs to consider reexport inlining.
let attrs = clean::Attributes::from_ast(attrs, None);
for (parent_module, doc) in attrs.collapsed_doc_value_by_module_level() {
let module_id = parent_module.unwrap_or(self.current_mod.to_def_id());
self.add_traits_in_scope(module_id);
let module_id = self.current_mod.to_def_id();
let mut need_traits_in_scope = false;
for (doc_module, doc) in
Attributes::from_ast(attrs, None).collapsed_doc_value_by_module_level()
{
assert_eq!(doc_module, None);
for link in markdown_links(&doc.as_str()) {
let path_str = if let Some(Ok(x)) = preprocess_link(&link) {
x.path_str
} else {
continue;
};
self.resolver.resolve_rustdoc_path(&path_str, TypeNS, module_id);
if let Some(Ok(pinfo)) = preprocess_link(&link) {
self.resolver.resolve_rustdoc_path(&pinfo.path_str, TypeNS, module_id);
need_traits_in_scope = true;
}
}
}
if need_traits_in_scope {
self.add_traits_in_scope(module_id);
}
}
/// When reexports are inlined, they are replaced with item which they refer to, those items
@ -170,32 +190,41 @@ impl IntraLinkCrateLoader<'_, '_> {
}
for child in self.resolver.module_children_or_reexports(module_id) {
if child.vis == Visibility::Public || self.document_private_items {
if let Some(def_id) = child.res.opt_def_id() {
self.add_traits_in_parent_scope(def_id);
}
if let Res::Def(DefKind::Mod, module_id) = child.res {
self.process_module_children_or_reexports(module_id);
// This condition should give a superset of `denied` from `fn clean_use_statement`.
if child.vis == Visibility::Public
|| self.document_private_items
&& child.vis != Visibility::Restricted(module_id)
&& module_id.is_local()
{
if let Some(def_id) = child.res.opt_def_id() && !def_id.is_local() {
// FIXME: Need to resolve doc links on all these extern items
// reached through reexports.
let scope_id = match child.res {
Res::Def(DefKind::Variant, ..) => self.resolver.parent(def_id).unwrap(),
_ => def_id,
};
self.add_traits_in_parent_scope(scope_id); // Outer attribute scope
if let Res::Def(DefKind::Mod, ..) = child.res {
self.add_traits_in_scope(def_id); // Inner attribute scope
}
// Traits are processed in `add_extern_traits_in_scope`.
if let Res::Def(DefKind::Mod | DefKind::Enum, ..) = child.res {
self.process_module_children_or_reexports(def_id);
}
}
}
}
}
}
impl Visitor<'_> for IntraLinkCrateLoader<'_, '_> {
impl Visitor<'_> for EarlyDocLinkResolver<'_, '_> {
fn visit_item(&mut self, item: &ast::Item) {
self.load_links_in_attrs(&item.attrs); // Outer attribute scope
if let ItemKind::Mod(..) = item.kind {
let old_mod = mem::replace(&mut self.current_mod, self.resolver.local_def_id(item.id));
// A module written with a outline doc comments will resolve traits relative
// to the parent module. Make sure the parent module's traits-in-scope are
// loaded, even if the module itself has no doc comments.
self.add_traits_in_parent_scope(self.current_mod.to_def_id());
self.load_links_in_attrs(&item.attrs);
self.load_links_in_attrs(&item.attrs); // Inner attribute scope
self.process_module_children_or_reexports(self.current_mod.to_def_id());
visit::walk_item(self, item);
self.current_mod = old_mod;
} else {
match item.kind {
@ -207,7 +236,6 @@ impl Visitor<'_> for IntraLinkCrateLoader<'_, '_> {
}
_ => {}
}
self.load_links_in_attrs(&item.attrs);
visit::walk_item(self, item);
}
}

View File

@ -0,0 +1,19 @@
// Traits in scope are collected for doc links in both outer and inner module attributes.
// check-pass
// aux-build: assoc-mod-inner-outer-dep.rs
extern crate assoc_mod_inner_outer_dep;
pub use assoc_mod_inner_outer_dep::*;
#[derive(Clone)]
pub struct Struct;
pub mod outer1 {
/// [crate::Struct::clone]
pub mod inner {}
}
pub mod outer2 {
//! [crate::Struct::clone]
}

View File

@ -0,0 +1,11 @@
#[derive(Clone)]
pub struct Struct;
pub mod dep_outer1 {
/// [crate::Struct::clone]
pub mod inner {}
}
pub mod dep_outer2 {
//! [crate::Struct::clone]
}