rustdoc: Unify macro intra-doc link resolution with type and value resolution

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -1234,7 +1234,6 @@ 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);
             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

@@ -981,7 +981,6 @@ pub struct Resolver<'a> {
     registered_attrs: FxHashSet<Ident>,
     registered_tools: FxHashSet<Ident>,
     macro_use_prelude: FxHashMap<Symbol, &'a NameBinding<'a>>,
-    all_macros: FxHashMap<Symbol, Res>,
     macro_map: FxHashMap<DefId, Lrc<SyntaxExtension>>,
     dummy_ext_bang: Lrc<SyntaxExtension>,
     dummy_ext_derive: Lrc<SyntaxExtension>,
@@ -1370,7 +1369,6 @@ impl<'a> Resolver<'a> {
             registered_attrs,
             registered_tools,
             macro_use_prelude: FxHashMap::default(),
-            all_macros: FxHashMap::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())),
@@ -3383,11 +3381,6 @@ impl<'a> Resolver<'a> {
         self.graph_root
     }
 
-    // For rustdoc.
-    pub fn all_macros(&self) -> &FxHashMap<Symbol, Res> {
-        &self.all_macros
-    }
-
     /// 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> {

compiler/rustc_resolve/src/macros.rs

@@ -550,7 +550,7 @@ impl<'a> Resolver<'a> {
         Ok((ext, res))
     }
 
-    pub fn resolve_macro_path(
+    crate fn resolve_macro_path(
         &mut self,
         path: &ast::Path,
         kind: Option<MacroKind>,

src/librustdoc/passes/collect_intra_doc_links.rs

@@ -2,10 +2,8 @@
 //!
 //! [RFC 1946]: https://github.com/rust-lang/rfcs/blob/master/text/1946-intra-rustdoc-links.md
 
-use rustc_ast as ast;
 use rustc_data_structures::{fx::FxHashMap, stable_set::FxHashSet};
 use rustc_errors::{Applicability, DiagnosticBuilder};
-use rustc_expand::base::SyntaxExtensionKind;
 use rustc_hir as hir;
 use rustc_hir::def::{
     DefKind,
@@ -388,48 +386,6 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
         })
     }
 
-    /// Resolves a string as a macro.
-    ///
-    /// FIXME(jynelson): Can this be unified with `resolve()`?
-    fn resolve_macro(
-        &self,
-        path_str: &'a str,
-        module_id: DefId,
-    ) -> Result<Res, ResolutionFailure<'a>> {
-        let path = ast::Path::from_ident(Ident::from_str(path_str));
-        self.cx.enter_resolver(|resolver| {
-            // FIXME(jynelson): does this really need 3 separate lookups?
-            if let Ok((Some(ext), res)) = resolver.resolve_macro_path(
-                &path,
-                None,
-                &ParentScope::module(resolver.graph_root(), resolver),
-                false,
-                false,
-            ) {
-                if let SyntaxExtensionKind::LegacyBang { .. } = ext.kind {
-                    return Ok(res.try_into().unwrap());
-                }
-            }
-            if let Some(&res) = resolver.all_macros().get(&Symbol::intern(path_str)) {
-                return Ok(res.try_into().unwrap());
-            }
-            debug!("resolving {} as a macro in the module {:?}", path_str, module_id);
-            if let Ok((_, res)) =
-                resolver.resolve_str_path_error(DUMMY_SP, path_str, MacroNS, module_id)
-            {
-                // don't resolve builtins like `#[derive]`
-                if let Ok(res) = res.try_into() {
-                    return Ok(res);
-                }
-            }
-            Err(ResolutionFailure::NotResolved {
-                module_id,
-                partial_res: None,
-                unresolved: path_str.into(),
-            })
-        })
-    }
-
     /// Convenience wrapper around `resolve_str_path_error`.
     ///
     /// This also handles resolving `true` and `false` as booleans.
@@ -703,15 +659,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
         module_id: DefId,
         extra_fragment: &Option<String>,
     ) -> Option<Res> {
-        // resolve() can't be used for macro namespace
-        let result = match ns {
-            Namespace::MacroNS => self.resolve_macro(path_str, module_id).map_err(ErrorKind::from),
-            Namespace::TypeNS | Namespace::ValueNS => {
-                self.resolve(path_str, ns, module_id, extra_fragment).map(|(res, _)| res)
-            }
-        };
-
-        let res = match result {
+        let res = match self.resolve(path_str, ns, module_id, extra_fragment).map(|(res, _)| res) {
             Ok(res) => Some(res),
             Err(ErrorKind::Resolve(box kind)) => kind.full_res(),
             Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))) => Some(res),
@@ -1344,7 +1292,6 @@ impl LinkCollector<'_, '_> {
     }
 
     /// After parsing the disambiguator, resolve the main part of the link.
-    // FIXME(jynelson): wow this is just so much
     fn resolve_with_disambiguator(
         &mut self,
         key: &ResolutionInfo,
@@ -1356,16 +1303,16 @@ impl LinkCollector<'_, '_> {
         let extra_fragment = &key.extra_fragment;
 
         match disambiguator.map(Disambiguator::ns) {
-            Some(expected_ns @ (ValueNS | TypeNS)) => {
+            Some(expected_ns) => {
                 match self.resolve(path_str, expected_ns, base_node, extra_fragment) {
                     Ok(res) => Some(res),
                     Err(ErrorKind::Resolve(box mut kind)) => {
                         // We only looked in one namespace. Try to give a better error if possible.
                         if kind.full_res().is_none() {
-                            let other_ns = if expected_ns == ValueNS { TypeNS } else { ValueNS };
+                            let all_ns = [TypeNS, ValueNS, MacroNS];
                             // FIXME: really it should be `resolution_failure` that does this, not `resolve_with_disambiguator`
                             // See https://github.com/rust-lang/rust/pull/76955#discussion_r493953382 for a good approach
-                            for new_ns in [other_ns, MacroNS] {
+                            for new_ns in all_ns.into_iter().filter(|x| *x != expected_ns) {
                                 if let Some(res) =
                                     self.check_full_res(new_ns, path_str, base_node, extra_fragment)
                                 {
@@ -1388,44 +1335,25 @@ impl LinkCollector<'_, '_> {
             }
             None => {
                 // Try everything!
-                let mut candidates = PerNS {
-                    macro_ns: self
-                        .resolve_macro(path_str, base_node)
-                        .map(|res| (res, extra_fragment.clone())),
-                    type_ns: match self.resolve(path_str, TypeNS, base_node, extra_fragment) {
-                        Ok(res) => {
-                            debug!("got res in TypeNS: {:?}", res);
-                            Ok(res)
-                        }
+                let mut do_resolve =
+                    |ns| match self.resolve(path_str, ns, base_node, extra_fragment) {
+                        Ok(res) => Some(Ok(res)),
                         Err(ErrorKind::AnchorFailure(msg)) => {
-                            anchor_failure(self.cx, diag, msg);
-                            return None;
-                        }
-                        Err(ErrorKind::Resolve(box kind)) => Err(kind),
-                    },
-                    value_ns: match self.resolve(path_str, ValueNS, base_node, extra_fragment) {
-                        Ok(res) => Ok(res),
-                        Err(ErrorKind::AnchorFailure(msg)) => {
-                            anchor_failure(self.cx, diag, msg);
-                            return None;
+                            anchor_failure(self.cx, diag.clone(), msg);
+                            None
                         }
-                        Err(ErrorKind::Resolve(box kind)) => Err(kind),
-                    }
-                    .and_then(|(res, fragment)| {
+                        Err(ErrorKind::Resolve(box kind)) => Some(Err(kind)),
+                    };
+                let mut candidates = PerNS {
+                    macro_ns: do_resolve(MacroNS)?,
+                    type_ns: do_resolve(TypeNS)?,
+                    value_ns: do_resolve(ValueNS)?.and_then(|(res, fragment)| {
                         // Constructors are picked up in the type namespace.
                         match res {
                             Res::Def(DefKind::Ctor(..), _) => {
                                 Err(ResolutionFailure::WrongNamespace { res, expected_ns: TypeNS })
                             }
-                            _ => {
-                                match (fragment, extra_fragment.clone()) {
-                                    (Some(fragment), Some(_)) => {
-                                        // Shouldn't happen but who knows?
-                                        Ok((res, Some(fragment)))
-                                    }
-                                    (fragment, None) | (None, fragment) => Ok((res, fragment)),
-                                }
-                            }
+                            _ => Ok((res, fragment)),
                         }
                     }),
                 };
@@ -1458,25 +1386,6 @@ impl LinkCollector<'_, '_> {
                     None
                 }
             }
-            Some(MacroNS) => {
-                match self.resolve_macro(path_str, base_node) {
-                    Ok(res) => Some((res, extra_fragment.clone())),
-                    Err(mut kind) => {
-                        // `resolve_macro` only looks in the macro namespace. Try to give a better error if possible.
-                        for ns in [TypeNS, ValueNS] {
-                            if let Some(res) =
-                                self.check_full_res(ns, path_str, base_node, extra_fragment)
-                            {
-                                kind =
-                                    ResolutionFailure::WrongNamespace { res, expected_ns: MacroNS };
-                                break;
-                            }
-                        }
-                        resolution_failure(self, diag, path_str, disambiguator, smallvec![kind]);
-                        None
-                    }
-                }
-            }
         }
     }
 }

src/test/rustdoc-ui/intra-doc/private-macro.rs

@@ -0,0 +1,14 @@
+// check-pass
+#![deny(rustdoc::broken_intra_doc_links)]
+
+mod bar {
+    macro_rules! str {() => {}}
+}
+
+pub mod foo {
+    pub struct Baz {}
+    impl Baz {
+        /// [str]
+        pub fn foo(&self) {}
+    }
+}