Fix type/value namespace clashes + test for that

src/librustdoc/visit_ast.rs

@@ -74,50 +74,46 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
         // well (_e.g._, `Copy`), these are wrongly bundled in there too, so we need to fix that by
         // moving them back to their correct locations.
         krate.exported_macros.iter().for_each(|def| {
-            /// A return value of `None` signifies a fallback to the default behavior (locating
+            let visit_macro = || self.visit_local_macro(def, None);
-            /// the macro at the root of the crate).
+            // The `def` of a macro in `exported_macros` should correspond to either:
-            fn containing_mod_of_macro<'module, 'hir>(
+            //  - a `#[macro-export] macro_rules!` macro,
-                def: &'_ rustc_hir::MacroDef<'_>,
+            //  - a built-in `derive` (or attribute) macro such as the ones in `::core`,
-                tcx: TyCtxt<'_>,
+            //  - a `pub macro`.
-                top_level_module: &'module mut Module<'hir>,
+            // Only the last two need to be fixed, thus:
-            ) -> Option<&'module mut Module<'hir>> {
+            if def.ast.macro_rules {
-                // The `def` of a macro in `exported_macros` should correspond to either:
+                top_level_module.macros.push(visit_macro());
-                //  - a `#[macro-export] macro_rules!` macro,
+                return;
-                //  - a built-in `derive` (or attribute) macro such as the ones in `::core`,
-                //  - a `pub macro`.
-                // Only the last two need to be fixed, thus:
-                if def.ast.macro_rules {
-                    return None;
-                }
-                /* Because of #77828 we cannot do the simpler:
-                let macro_parent_module = tcx.def_path(tcx.parent_module(def.hir_id).to_def_id());
-                // and instead have to do: */
-                let macro_parent_module = tcx.def_path({
-                    use rustc_middle::ty::DefIdTree;
-                    tcx.parent(tcx.hir().local_def_id(def.hir_id).to_def_id())?
-                });
-                // HACK: rustdoc has no way to lookup `doctree::Module`s by their HirId. Instead,
-                // lookup the module by its name, by looking at each path segment one at a time.
-                // WARNING: this will probably break in the presence of re-exports or shadowing.
-                let mut cur_mod = top_level_module;
-                for path_segment in macro_parent_module.data {
-                    let path_segment = path_segment.to_string();
-                    cur_mod = cur_mod.mods.iter_mut().find(|module| {
-                        matches!(
-                            module.name, Some(symbol)
-                            if symbol.with(|mod_name| mod_name == path_segment)
-                        )
-                    })?;
-                }
-                Some(cur_mod)
             }
-
+            let tcx = self.cx.tcx;
-            if let Some(module) = containing_mod_of_macro(def, self.cx.tcx, &mut top_level_module) {
+            /* Because of #77828 we cannot do the simpler:
-                &mut module.macros
+            let macro_parent_module = tcx.def_path(tcx.parent_module(def.hir_id).to_def_id());
-            } else {
+            // and instead have to do: */
-                &mut top_level_module.macros
+            let macro_parent_module = tcx.def_path({
+                use rustc_middle::ty::DefIdTree;
+                tcx.parent(tcx.hir().local_def_id(def.hir_id).to_def_id()).unwrap()
+            });
+            // HACK: rustdoc has no way to lookup `doctree::Module`s by their HirId. Instead,
+            // lookup the module by its name, by looking at each path segment one at a time.
+            let mut cur_mod = &mut top_level_module;
+            for path_segment in macro_parent_module.data {
+                let path_segment_ty_ns = match path_segment.data {
+                    rustc_hir::definitions::DefPathData::TypeNs(symbol) => symbol,
+                    _ => {
+                        // If the path segment is not from the type namespace
+                        // (_e.g._, it can be from a value namespace in the case of `f::` in:
+                        // `fn f() { pub macro m() {} }`
+                        // then the item is not accessible, and should thus act as if it didn't
+                        // exist (unless "associated macros" (inside an `impl`) were a thing…).
+                        return;
+                    }
+                };
+                cur_mod = cur_mod
+                    .mods
+                    .iter_mut()
+                    .find(|module| module.name == Some(path_segment_ty_ns))
+                    .unwrap();
             }
-            .push(self.visit_local_macro(def, None));
+            cur_mod.macros.push(visit_macro());
         });
 
         self.cx.renderinfo.get_mut().exact_paths = self.exact_paths;

src/test/rustdoc/auxiliary/macro_pub_in_module.rs

@@ -0,0 +1,13 @@
+// edition:2018
+
+#![feature(decl_macro)]
+#![crate_name = "external_crate"]
+
+pub mod some_module {
+    /* == Make sure the logic is not affected by a re-export == */
+    mod private {
+        pub macro external_macro() {}
+    }
+    // @has external_crate/some_module/macro.external_macro.html
+    pub use private::external_macro;
+}

src/test/rustdoc/macro_pub_in_module.rs

@@ -1,11 +1,18 @@
+// aux-build:macro_pub_in_module.rs
+// edition:2018
+// build-aux-docs
+// @has external_crate/some_module/macro.external_macro.html
+
 //! See issue #74355
 #![feature(decl_macro, no_core, rustc_attrs)]
 #![crate_name = "krate"]
 #![no_core]
 
+extern crate external_crate;
+
 pub mod inner {
-    // @has krate/inner/macro.my_macro.html
+    // @has krate/inner/macro.raw_const.html
-    pub macro my_macro() {}
+    pub macro raw_const() {}
 
     // @has krate/inner/macro.test.html
     #[rustc_builtin_macro]
@@ -14,4 +21,29 @@ pub mod inner {
     // @has krate/inner/macro.Clone.html
     #[rustc_builtin_macro]
     pub macro Clone($item:item) {}
+
+    // Make sure the logic is not affected by a re-export.
+    mod private {
+        pub macro m() {}
+    }
+    // @has krate/inner/macro.renamed.html
+    pub use private::m as renamed;
+
+    // @has krate/inner/macro.external_macro.html
+    pub use ::external_crate::some_module::external_macro;
+}
+
+// Namespaces: Make sure the logic does not mix up a function name with a module name…
+fn both_fn_and_mod() {
+    pub macro m() {}
+}
+pub mod both_fn_and_mod {
+    // @!has krate/both_fn_and_mod/macro.m.html
+}
+
+const __: () = {
+    pub macro m() {}
+};
+pub mod __ {
+    // @!has krate/__/macro.m.html
 }