Auto merge of rust-lang#96521 - petrochenkov:docrules, r=notriddle,GuillaumeGomez

rustdoc: Resolve doc links referring to `macro_rules` items

cc rust-lang#81633

UPD: the fallback to considering *all* `macro_rules` in the crate for unresolved names is not removed in this PR, it will be removed separately and will be run through crater.

Author: bors

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -1267,13 +1267,15 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
                 self.insert_unused_macro(ident, def_id, item.id);
             }
             self.r.visibilities.insert(def_id, vis);
-            self.r.arenas.alloc_macro_rules_scope(MacroRulesScope::Binding(
+            let scope = self.r.arenas.alloc_macro_rules_scope(MacroRulesScope::Binding(
                 self.r.arenas.alloc_macro_rules_binding(MacroRulesBinding {
                     parent_macro_rules_scope: parent_scope.macro_rules,
                     binding,
                     ident,
                 }),
-            ))
+            ));
+            self.r.macro_rules_scopes.insert(def_id, scope);
+            scope
         } else {
             let module = parent_scope.module;
             let vis = match item.kind {

compiler/rustc_resolve/src/lib.rs

@@ -143,9 +143,9 @@ enum ScopeSet<'a> {
 /// but not for late resolution yet.
 #[derive(Clone, Copy, Debug)]
 pub struct ParentScope<'a> {
-    module: Module<'a>,
+    pub module: Module<'a>,
     expansion: LocalExpnId,
-    macro_rules: MacroRulesScopeRef<'a>,
+    pub macro_rules: MacroRulesScopeRef<'a>,
     derives: &'a [ast::Path],
 }
 
@@ -990,6 +990,8 @@ pub struct Resolver<'a> {
     /// `macro_rules` scopes *produced* by expanding the macro invocations,
     /// include all the `macro_rules` items and other invocations generated by them.
     output_macro_rules_scopes: FxHashMap<LocalExpnId, MacroRulesScopeRef<'a>>,
+    /// `macro_rules` scopes produced by `macro_rules` item definitions.
+    macro_rules_scopes: FxHashMap<LocalDefId, MacroRulesScopeRef<'a>>,
     /// Helper attributes that are in scope for the given expansion.
     helper_attrs: FxHashMap<LocalExpnId, Vec<Ident>>,
     /// Ready or in-progress results of resolving paths inside the `#[derive(...)]` attribute
@@ -1361,6 +1363,7 @@ impl<'a> Resolver<'a> {
             non_macro_attr: Lrc::new(SyntaxExtension::non_macro_attr(session.edition())),
             invocation_parent_scopes: Default::default(),
             output_macro_rules_scopes: Default::default(),
+            macro_rules_scopes: Default::default(),
             helper_attrs: Default::default(),
             derive_data: Default::default(),
             local_macro_def_scopes: FxHashMap::default(),
@@ -1874,25 +1877,25 @@ impl<'a> Resolver<'a> {
         &mut self,
         path_str: &str,
         ns: Namespace,
-        mut module_id: DefId,
+        mut parent_scope: ParentScope<'a>,
     ) -> Option<Res> {
         let mut segments =
             Vec::from_iter(path_str.split("::").map(Ident::from_str).map(Segment::from_ident));
         if let Some(segment) = segments.first_mut() {
             if segment.ident.name == kw::Crate {
                 // FIXME: `resolve_path` always resolves `crate` to the current crate root, but
-                // rustdoc wants it to resolve to the `module_id`'s crate root. This trick of
+                // rustdoc wants it to resolve to the `parent_scope`'s crate root. This trick of
                 // replacing `crate` with `self` and changing the current module should achieve
                 // the same effect.
                 segment.ident.name = kw::SelfLower;
-                module_id = module_id.krate.as_def_id();
+                parent_scope.module =
+                    self.expect_module(parent_scope.module.def_id().krate.as_def_id());
             } else if segment.ident.name == kw::Empty {
                 segment.ident.name = kw::PathRoot;
             }
         }
 
-        let module = self.expect_module(module_id);
-        match self.maybe_resolve_path(&segments, Some(ns), &ParentScope::module(module, self)) {
+        match self.maybe_resolve_path(&segments, Some(ns), &parent_scope) {
             PathResult::Module(ModuleOrUniformRoot::Module(module)) => Some(module.res().unwrap()),
             PathResult::NonModule(path_res) if path_res.unresolved_segments() == 0 => {
                 Some(path_res.base_res())
@@ -1904,11 +1907,6 @@ impl<'a> Resolver<'a> {
         }
     }
 
-    // For rustdoc.
-    pub fn graph_root(&self) -> Module<'a> {
-        self.graph_root
-    }
-
     // For rustdoc.
     pub fn take_all_macro_rules(&mut self) -> FxHashMap<Symbol, Res> {
         mem::take(&mut self.all_macro_rules)
@@ -1924,6 +1922,11 @@ impl<'a> Resolver<'a> {
         }
     }
 
+    /// For rustdoc.
+    pub fn macro_rules_scope(&self, def_id: LocalDefId) -> MacroRulesScopeRef<'a> {
+        *self.macro_rules_scopes.get(&def_id).expect("not a `macro_rules` item")
+    }
+
     /// Retrieves the span of the given `DefId` if `DefId` is in the local crate.
     #[inline]
     pub fn opt_span(&self, def_id: DefId) -> Option<Span> {

library/core/src/macros/mod.rs

@@ -640,6 +640,8 @@ macro_rules! unreachable {
 ///
 /// Like `panic!`, this macro has a second form for displaying custom values.
 ///
+/// [`todo!`]: crate::todo
+///
 /// # Examples
 ///
 /// Say we have a trait `Foo`:

library/std/src/macros.rs

@@ -31,6 +31,8 @@ macro_rules! panic {
 /// [`eprint!`] instead to print error and progress messages.
 ///
 /// [flush]: crate::io::Write::flush
+/// [`println!`]: crate::println
+/// [`eprint!`]: crate::eprint
 ///
 /// # Panics
 ///
@@ -77,6 +79,7 @@ macro_rules! print {
 /// [`eprintln!`] instead to print error and progress messages.
 ///
 /// [`std::fmt`]: crate::fmt
+/// [`eprintln!`]: crate::eprintln
 ///
 /// # Panics
 ///
@@ -146,6 +149,7 @@ macro_rules! eprint {
 ///
 /// [`io::stderr`]: crate::io::stderr
 /// [`io::stdout`]: crate::io::stdout
+/// [`println!`]: crate::println
 ///
 /// # Panics
 ///

library/std/src/thread/local.rs

@@ -21,6 +21,8 @@ use crate::fmt;
 /// The [`with`] method yields a reference to the contained value which cannot be
 /// sent across threads or escape the given closure.
 ///
+/// [`thread_local!`]: crate::thread_local
+///
 /// # Initialization and Destruction
 ///
 /// Initialization is dynamically performed on the first call to [`with`]

library/std/src/thread/mod.rs

@@ -146,6 +146,7 @@
 //! [`Cell`]: crate::cell::Cell
 //! [`RefCell`]: crate::cell::RefCell
 //! [`with`]: LocalKey::with
+//! [`thread_local!`]: crate::thread_local
 
 #![stable(feature = "rust1", since = "1.0.0")]
 #![deny(unsafe_op_in_unsafe_fn)]

src/librustdoc/passes/collect_intra_doc_links.rs

@@ -12,6 +12,7 @@ use rustc_hir::def_id::{DefId, CRATE_DEF_ID};
 use rustc_hir::Mutability;
 use rustc_middle::ty::{DefIdTree, Ty, TyCtxt};
 use rustc_middle::{bug, span_bug, ty};
+use rustc_resolve::ParentScope;
 use rustc_session::lint::Lint;
 use rustc_span::hygiene::MacroKind;
 use rustc_span::symbol::{sym, Ident, Symbol};
@@ -564,7 +565,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
             .copied()
             .unwrap_or_else(|| {
                 self.cx.enter_resolver(|resolver| {
-                    resolver.resolve_rustdoc_path(path_str, ns, module_id)
+                    let parent_scope =
+                        ParentScope::module(resolver.expect_module(module_id), resolver);
+                    resolver.resolve_rustdoc_path(path_str, ns, parent_scope)
                 })
             })
             .and_then(|res| res.try_into().ok())

src/librustdoc/passes/collect_intra_doc_links/early.rs

@@ -1,20 +1,21 @@
 use crate::clean::Attributes;
 use crate::core::ResolverCaches;
 use crate::passes::collect_intra_doc_links::preprocessed_markdown_links;
-use crate::passes::collect_intra_doc_links::PreprocessedMarkdownLink;
+use crate::passes::collect_intra_doc_links::{Disambiguator, PreprocessedMarkdownLink};
 
 use rustc_ast::visit::{self, AssocCtxt, Visitor};
 use rustc_ast::{self as ast, ItemKind};
 use rustc_ast_lowering::ResolverAstLowering;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::def::Namespace::*;
 use rustc_hir::def::{DefKind, Namespace, Res};
-use rustc_hir::def_id::{DefId, DefIdMap, DefIdSet, LocalDefId, CRATE_DEF_ID};
+use rustc_hir::def_id::{DefId, DefIdMap, DefIdSet, CRATE_DEF_ID};
 use rustc_hir::TraitCandidate;
 use rustc_middle::ty::{DefIdTree, Visibility};
 use rustc_resolve::{ParentScope, Resolver};
 use rustc_session::config::Externs;
 use rustc_session::Session;
+use rustc_span::symbol::sym;
 use rustc_span::{Symbol, SyntaxContext};
 
 use std::collections::hash_map::Entry;
@@ -27,10 +28,12 @@ crate fn early_resolve_intra_doc_links(
     externs: Externs,
     document_private_items: bool,
 ) -> ResolverCaches {
+    let parent_scope =
+        ParentScope::module(resolver.expect_module(CRATE_DEF_ID.to_def_id()), resolver);
     let mut link_resolver = EarlyDocLinkResolver {
         resolver,
         sess,
-        current_mod: CRATE_DEF_ID,
+        parent_scope,
         visited_mods: Default::default(),
         markdown_links: Default::default(),
         doc_link_resolutions: Default::default(),
@@ -52,7 +55,7 @@ crate fn early_resolve_intra_doc_links(
     // 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) {
-        link_resolver.resolver.resolve_rustdoc_path(extern_name, TypeNS, CRATE_DEF_ID.to_def_id());
+        link_resolver.resolver.resolve_rustdoc_path(extern_name, TypeNS, parent_scope);
     }
 
     ResolverCaches {
@@ -72,7 +75,7 @@ fn doc_attrs<'a>(attrs: impl Iterator<Item = &'a ast::Attribute>) -> Attributes
 struct EarlyDocLinkResolver<'r, 'ra> {
     resolver: &'r mut Resolver<'ra>,
     sess: &'r Session,
-    current_mod: LocalDefId,
+    parent_scope: ParentScope<'ra>,
     visited_mods: DefIdSet,
     markdown_links: FxHashMap<String, Vec<PreprocessedMarkdownLink>>,
     doc_link_resolutions: FxHashMap<(Symbol, Namespace, DefId), Option<Res<ast::NodeId>>>,
@@ -82,7 +85,7 @@ struct EarlyDocLinkResolver<'r, 'ra> {
     document_private_items: bool,
 }
 
-impl EarlyDocLinkResolver<'_, '_> {
+impl<'ra> EarlyDocLinkResolver<'_, 'ra> {
     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.
@@ -205,34 +208,64 @@ impl EarlyDocLinkResolver<'_, '_> {
         if !attrs.iter().any(|attr| attr.may_have_doc_links()) {
             return;
         }
-        let module_id = self.current_mod.to_def_id();
-        self.resolve_doc_links(doc_attrs(attrs.iter()), module_id);
+        self.resolve_doc_links(doc_attrs(attrs.iter()), self.parent_scope);
     }
 
-    fn resolve_doc_links(&mut self, attrs: Attributes, module_id: DefId) {
+    fn resolve_and_cache(
+        &mut self,
+        path_str: &str,
+        ns: Namespace,
+        parent_scope: &ParentScope<'ra>,
+    ) -> bool {
+        // FIXME: This caching may be incorrect in case of multiple `macro_rules`
+        // items with the same name in the same module.
+        self.doc_link_resolutions
+            .entry((Symbol::intern(path_str), ns, parent_scope.module.def_id()))
+            .or_insert_with_key(|(path, ns, _)| {
+                self.resolver.resolve_rustdoc_path(path.as_str(), *ns, *parent_scope)
+            })
+            .is_some()
+    }
+
+    fn resolve_doc_links(&mut self, attrs: Attributes, parent_scope: ParentScope<'ra>) {
         let mut need_traits_in_scope = false;
         for (doc_module, doc) in attrs.prepare_to_doc_link_resolution() {
             assert_eq!(doc_module, None);
-            let links = self
-                .markdown_links
-                .entry(doc)
-                .or_insert_with_key(|doc| preprocessed_markdown_links(doc));
+            let mut tmp_links = mem::take(&mut self.markdown_links);
+            let links =
+                tmp_links.entry(doc).or_insert_with_key(|doc| preprocessed_markdown_links(doc));
             for PreprocessedMarkdownLink(pp_link, _) in links {
                 if let Ok(pinfo) = pp_link {
-                    // FIXME: Resolve the path in all namespaces and resolve its prefixes too.
-                    let ns = TypeNS;
-                    self.doc_link_resolutions
-                        .entry((Symbol::intern(&pinfo.path_str), ns, module_id))
-                        .or_insert_with_key(|(path, ns, module_id)| {
-                            self.resolver.resolve_rustdoc_path(path.as_str(), *ns, *module_id)
-                        });
-                    need_traits_in_scope = true;
+                    // The logic here is a conservative approximation for path resolution in
+                    // `resolve_with_disambiguator`.
+                    if let Some(ns) = pinfo.disambiguator.map(Disambiguator::ns) {
+                        if self.resolve_and_cache(&pinfo.path_str, ns, &parent_scope) {
+                            continue;
+                        }
+                    }
+
+                    // Resolve all namespaces due to no disambiguator or for diagnostics.
+                    let mut any_resolved = false;
+                    let mut need_assoc = false;
+                    for ns in [TypeNS, ValueNS, MacroNS] {
+                        if self.resolve_and_cache(&pinfo.path_str, ns, &parent_scope) {
+                            any_resolved = true;
+                        } else if ns != MacroNS {
+                            need_assoc = true;
+                        }
+                    }
+
+                    // FIXME: Resolve all prefixes for type-relative resolution or for diagnostics.
+                    if (need_assoc || !any_resolved) && pinfo.path_str.contains("::") {
+                        need_traits_in_scope = true;
+                    }
                 }
             }
+            self.markdown_links = tmp_links;
         }
 
         if need_traits_in_scope {
-            self.add_traits_in_scope(module_id);
+            self.add_traits_in_scope(parent_scope.module.def_id());
         }
     }
 
@@ -274,19 +307,33 @@ impl Visitor<'_> for EarlyDocLinkResolver<'_, '_> {
     fn visit_item(&mut self, item: &ast::Item) {
         self.resolve_doc_links_local(&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));
+            let module_def_id = self.resolver.local_def_id(item.id).to_def_id();
+            let module = self.resolver.expect_module(module_def_id);
+            let old_module = mem::replace(&mut self.parent_scope.module, module);
+            let old_macro_rules = self.parent_scope.macro_rules;
             self.resolve_doc_links_local(&item.attrs); // Inner attribute scope
-            self.process_module_children_or_reexports(self.current_mod.to_def_id());
+            self.process_module_children_or_reexports(module_def_id);
             visit::walk_item(self, item);
-            self.current_mod = old_mod;
+            if item
+                .attrs
+                .iter()
+                .all(|attr| !attr.has_name(sym::macro_use) && !attr.has_name(sym::macro_escape))
+            {
+                self.parent_scope.macro_rules = old_macro_rules;
+            }
+            self.parent_scope.module = old_module;
         } else {
-            match item.kind {
+            match &item.kind {
                 ItemKind::Trait(..) => {
                     self.all_traits.push(self.resolver.local_def_id(item.id).to_def_id());
                 }
                 ItemKind::Impl(box ast::Impl { of_trait: Some(..), .. }) => {
                     self.all_trait_impls.push(self.resolver.local_def_id(item.id).to_def_id());
                 }
+                ItemKind::MacroDef(macro_def) if macro_def.macro_rules => {
+                    self.parent_scope.macro_rules =
+                        self.resolver.macro_rules_scope(self.resolver.local_def_id(item.id));
+                }
                 _ => {}
             }
             visit::walk_item(self, item);
@@ -313,6 +360,12 @@ impl Visitor<'_> for EarlyDocLinkResolver<'_, '_> {
         visit::walk_field_def(self, field)
     }
 
+    fn visit_block(&mut self, block: &ast::Block) {
+        let old_macro_rules = self.parent_scope.macro_rules;
+        visit::walk_block(self, block);
+        self.parent_scope.macro_rules = old_macro_rules;
+    }
+
     // NOTE: if doc-comments are ever allowed on other nodes (e.g. function parameters),
     // then this will have to implement other visitor methods too.
 }

src/test/rustdoc-ui/intra-doc/macro-rules-error.rs

@@ -0,0 +1,27 @@
+// `macro_rules` scopes are respected during doc link resolution.
+
+// compile-flags: --document-private-items
+
+#![deny(rustdoc::broken_intra_doc_links)]
+
+mod no_escape {
+    macro_rules! before_but_limited_to_module {
+        () => {};
+    }
+}
+
+/// [before_but_limited_to_module] FIXME: This error should be reported
+// ERROR unresolved link to `before_but_limited_to_module`
+/// [after] FIXME: This error should be reported
+// ERROR unresolved link to `after`
+/// [str] FIXME: This error shouldn not be reported
+//~^ ERROR `str` is both a builtin type and a macro
+fn check() {}
+
+macro_rules! after {
+    () => {};
+}
+
+macro_rules! str {
+    () => {};
+}