Auto merge of #95706 - petrochenkov:doclink4, r=GuillaumeGomez

rustdoc: Early doc link resolution fixes and refactorings

A subset of #94857 that shouldn't cause perf regressions, but should fix some issues like https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/ICE.20in.20collect_intra_doc_links.2Ers #95290 and improve performance in cases like #95694.

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -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 {

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

@@ -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) }

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -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));

compiler/rustc_resolve/src/lib.rs

@@ -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.

src/librustdoc/core.rs

@@ -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> {

src/librustdoc/lib.rs

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

src/librustdoc/passes/collect_intra_doc_links.rs

@@ -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
     }