Auto merge of #77862 - danielhenrymantilla:rustdoc/fix-macros_2_0-paths, r=jyn514,petrochenkov

Rustdoc: Fix macros 2.0 and built-in derives being shown at the wrong path

Fixes #74355

  - ~~waiting on author + draft PR since my code ought to be cleaned up _w.r.t._ the way I avoid the `.unwrap()`s:~~

      - ~~dummy items may avoid the first `?`,~~

      - ~~but within the module traversal some tests did fail (hence the second `?`), meaning the crate did not possess the exact path of the containing module (`extern` / `impl` blocks maybe? I'll look into that).~~

r? `@jyn514`

compiler/rustc_privacy/src/lib.rs

@@ -832,10 +832,15 @@ fn visit_mod(&mut self, m: &'tcx hir::Mod<'tcx>, _sp: Span, id: hir::HirId) {
     }
 
     fn visit_macro_def(&mut self, md: &'tcx hir::MacroDef<'tcx>) {
+        // Non-opaque macros cannot make other items more accessible than they already are.
         if attr::find_transparency(&self.tcx.sess, &md.attrs, md.ast.macro_rules).0
             != Transparency::Opaque
         {
-            self.update(md.hir_id, Some(AccessLevel::Public));
+            // `#[macro_export]`-ed `macro_rules!` are `Public` since they
+            // ignore their containing path to always appear at the crate root.
+            if md.ast.macro_rules {
+                self.update(md.hir_id, Some(AccessLevel::Public));
+            }
             return;
         }
 

library/std/src/prelude/v1.rs

@@ -41,17 +41,17 @@
 pub use core::prelude::v1::{
     asm, assert, cfg, column, compile_error, concat, concat_idents, env, file, format_args,
     format_args_nl, global_asm, include, include_bytes, include_str, line, llvm_asm, log_syntax,
-    module_path, option_env, stringify, trace_macros,
+    module_path, option_env, stringify, trace_macros, Clone, Copy, Debug, Default, Eq, Hash, Ord,
+    PartialEq, PartialOrd,
 };
 
-// FIXME: Attribute and derive macros are not documented because for them rustdoc generates
+// FIXME: Attribute and internal derive macros are not documented because for them rustdoc generates
 // dead links which fail link checker testing.
 #[stable(feature = "builtin_macro_prelude", since = "1.38.0")]
 #[allow(deprecated)]
 #[doc(hidden)]
 pub use core::prelude::v1::{
-    bench, global_allocator, test, test_case, Clone, Copy, Debug, Default, Eq, Hash, Ord,
-    PartialEq, PartialOrd, RustcDecodable, RustcEncodable,
+    bench, global_allocator, test, test_case, RustcDecodable, RustcEncodable,
 };
 
 #[unstable(

src/librustdoc/clean/inline.rs

@@ -169,7 +169,17 @@
         if !s.is_empty() { Some(s) } else { None }
     });
     let fqn = if let clean::TypeKind::Macro = kind {
-        vec![crate_name, relative.last().expect("relative was empty")]
+        // Check to see if it is a macro 2.0 or built-in macro
+        if matches!(
+            cx.enter_resolver(|r| r.cstore().load_macro_untracked(did, cx.sess())),
+            LoadedMacro::MacroDef(def, _)
+                if matches!(&def.kind, ast::ItemKind::MacroDef(ast_def)
+                    if !ast_def.macro_rules)
+        ) {
+            once(crate_name).chain(relative).collect()
+        } else {
+            vec![crate_name, relative.last().expect("relative was empty")]
+        }
     } else {
         once(crate_name).chain(relative).collect()
     };

src/librustdoc/visit_ast.rs

@@ -61,20 +61,60 @@ fn store_path(&mut self, did: DefId) {
     }
 
     crate fn visit(mut self, krate: &'tcx hir::Crate<'_>) -> Module<'tcx> {
-        let mut module = self.visit_mod_contents(
+        let mut top_level_module = self.visit_mod_contents(
             krate.item.span,
             &Spanned { span: rustc_span::DUMMY_SP, node: hir::VisibilityKind::Public },
             hir::CRATE_HIR_ID,
             &krate.item.module,
             None,
         );
-        // Attach the crate's exported macros to the top-level module:
-        module.macros.extend(krate.exported_macros.iter().map(|def| (def, None)));
-        module.is_crate = true;
-
+        top_level_module.is_crate = true;
+        // Attach the crate's exported macros to the top-level module.
+        // In the case of macros 2.0 (`pub macro`), and for built-in `derive`s or attributes as
+        // 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.
+        'exported_macros: for def in krate.exported_macros {
+            // The `def` of a macro in `exported_macros` should correspond to either:
+            //  - a `#[macro_export] macro_rules!` macro,
+            //  - 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 {
+                top_level_module.macros.push((def, None));
+                continue 'exported_macros;
+            }
+            let tcx = self.cx.tcx;
+            // Note: this is not the same as `.parent_module()`. Indeed, the latter looks
+            // for the closest module _ancestor_, which is not necessarily a direct parent
+            // (since a direct parent isn't necessarily a module, c.f. #77828).
+            let macro_parent_def_id = {
+                use rustc_middle::ty::DefIdTree;
+                tcx.parent(tcx.hir().local_def_id(def.hir_id).to_def_id()).unwrap()
+            };
+            let macro_parent_path = tcx.def_path(macro_parent_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.
+            let mut cur_mod = &mut top_level_module;
+            for path_segment in macro_parent_path.data {
+                // Path segments may refer to a module (in which case they belong to the type
+                // namespace), which is _necessary_ for the macro to be accessible outside it
+                // (no "associated macros" as of yet). Else we bail with an outer `continue`.
+                let path_segment_ty_ns = match path_segment.data {
+                    rustc_hir::definitions::DefPathData::TypeNs(symbol) => symbol,
+                    _ => continue 'exported_macros,
+                };
+                // Descend into the child module that matches this path segment (if any).
+                match cur_mod.mods.iter_mut().find(|child| child.name == Some(path_segment_ty_ns)) {
+                    Some(child_mod) => cur_mod = &mut *child_mod,
+                    None => continue 'exported_macros,
+                }
+            }
+            let cur_mod_def_id = tcx.hir().local_def_id(cur_mod.id).to_def_id();
+            assert_eq!(cur_mod_def_id, macro_parent_def_id);
+            cur_mod.macros.push((def, None));
+        }
         self.cx.renderinfo.get_mut().exact_paths = self.exact_paths;
-
-        module
+        top_level_module
     }
 
     fn visit_mod_contents(

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() {}
+    }
+
+    pub use private::external_macro;
+}

src/test/rustdoc/macro_pub_in_module.rs

@@ -0,0 +1,82 @@
+// aux-build:macro_pub_in_module.rs
+// edition:2018
+// build-aux-docs
+
+//! See issue #74355
+#![feature(decl_macro, no_core, rustc_attrs)]
+#![crate_name = "krate"]
+#![no_core]
+
+ // @has external_crate/some_module/macro.external_macro.html
+  // @!has external_crate/macro.external_macro.html
+extern crate external_crate;
+
+pub mod inner {
+    // @has krate/inner/macro.raw_const.html
+    // @!has krate/macro.raw_const.html
+    pub macro raw_const() {}
+
+    // @has krate/inner/macro.test.html
+    // @!has krate/macro.test.html
+    #[rustc_builtin_macro]
+    pub macro test($item:item) {}
+
+    // @has krate/inner/macro.Clone.html
+    // @!has krate/macro.Clone.html
+    #[rustc_builtin_macro]
+    pub macro Clone($item:item) {}
+
+    // Make sure the logic is not affected by re-exports.
+    mod unrenamed {
+        // @!has krate/macro.unrenamed.html
+        #[rustc_macro_transparency = "semitransparent"]
+        pub macro unrenamed() {}
+    }
+    // @has krate/inner/macro.unrenamed.html
+    pub use unrenamed::unrenamed;
+
+    mod private {
+        // @!has krate/macro.m.html
+        pub macro m() {}
+    }
+    // @has krate/inner/macro.renamed.html
+    // @!has krate/macro.renamed.html
+    pub use private::m as renamed;
+
+    mod private2 {
+        // @!has krate/macro.m2.html
+        pub macro m2() {}
+    }
+    use private2 as renamed_mod;
+    // @has krate/inner/macro.m2.html
+    pub use renamed_mod::m2;
+
+    // @has krate/inner/macro.external_macro.html
+    // @!has krate/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() {
+    // @!has krate/macro.in_both_fn_and_mod.html
+    pub macro in_both_fn_and_mod() {}
+}
+pub mod both_fn_and_mod {
+    // @!has krate/both_fn_and_mod/macro.in_both_fn_and_mod.html
+}
+
+const __: () = {
+    // @!has krate/macro.in_both_const_and_mod.html
+    pub macro in_both_const_and_mod() {}
+};
+pub mod __ {
+    // @!has krate/__/macro.in_both_const_and_mod.html
+}
+
+enum Enum {
+    Crazy = {
+        // @!has krate/macro.this_is_getting_weird.html;
+        pub macro this_is_getting_weird() {}
+        42
+    },
+}