Teach tools that macros are now HIR items

inquisitivecrystal · inquisitivecrystal · commit 1f7bce012d7d · 2021-08-28T00:24:39.000-07:00
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
@@ -91,7 +91,6 @@ impl Clean<Item> for doctree::Module<'_> {
         items.extend(self.foreigns.iter().map(|x| x.clean(cx)));
         items.extend(self.mods.iter().map(|x| x.clean(cx)));
         items.extend(self.items.iter().map(|x| x.clean(cx)).flatten());
-        items.extend(self.macros.iter().map(|x| x.clean(cx)));
 
         // determine if we should display the inner contents or
         // the outer `mod` item for the source code.
@@ -1861,6 +1860,10 @@ impl Clean<Vec<Item>> for (&hir::Item<'_>, Option<Symbol>) {
                 ItemKind::Fn(ref sig, ref generics, body_id) => {
                     clean_fn_or_proc_macro(item, sig, generics, body_id, &mut name, cx)
                 }
+                ItemKind::Macro(ref macro_def) => MacroItem(Macro {
+                    source: display_macro_source(cx, name, &macro_def, def_id, &item.vis),
+                    imported_from: None,
+                }),
                 ItemKind::Trait(is_auto, unsafety, ref generics, ref bounds, ref item_ids) => {
                     let items = item_ids
                         .iter()
@@ -2138,24 +2141,6 @@ impl Clean<Item> for (&hir::ForeignItem<'_>, Option<Symbol>) {
     }
 }
 
-impl Clean<Item> for (&hir::MacroDef<'_>, Option<Symbol>) {
-    fn clean(&self, cx: &mut DocContext<'_>) -> Item {
-        let (item, renamed) = self;
-        let name = renamed.unwrap_or(item.ident.name);
-        let def_id = item.def_id.to_def_id();
-
-        Item::from_hir_id_and_parts(
-            item.hir_id(),
-            Some(name),
-            MacroItem(Macro {
-                source: display_macro_source(cx, name, &item.ast, def_id, &item.vis),
-                imported_from: None,
-            }),
-            cx,
-        )
-    }
-}
-
 impl Clean<TypeBinding> for hir::TypeBinding<'_> {
     fn clean(&self, cx: &mut DocContext<'_>) -> TypeBinding {
         TypeBinding { name: self.ident.name, kind: self.kind.clean(cx) }
diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs
@@ -1171,10 +1171,21 @@ impl<'a, 'hir, 'tcx> intravisit::Visitor<'hir> for HirCollector<'a, 'hir, 'tcx>
     }
 
     fn visit_item(&mut self, item: &'hir hir::Item<'_>) {
-        let name = if let hir::ItemKind::Impl(impl_) = &item.kind {
-            rustc_hir_pretty::id_to_string(&self.map, impl_.self_ty.hir_id)
-        } else {
-            item.ident.to_string()
+        let name = match &item.kind {
+            hir::ItemKind::Macro(ref macro_def) => {
+                // FIXME(#88038): Non exported macros have historically not been tested,
+                // but we really ought to start testing them.
+                let def_id = item.def_id.to_def_id();
+                if macro_def.macro_rules && !self.tcx.has_attr(def_id, sym::macro_export) {
+                    intravisit::walk_item(self, item);
+                    return;
+                }
+                item.ident.to_string()
+            }
+            hir::ItemKind::Impl(impl_) => {
+                rustc_hir_pretty::id_to_string(&self.map, impl_.self_ty.hir_id)
+            }
+            _ => item.ident.to_string(),
         };
 
         self.visit_testable(name, item.hir_id(), item.span, |this| {
@@ -1216,15 +1227,6 @@ impl<'a, 'hir, 'tcx> intravisit::Visitor<'hir> for HirCollector<'a, 'hir, 'tcx>
             intravisit::walk_field_def(this, f);
         });
     }
-
-    fn visit_macro_def(&mut self, macro_def: &'hir hir::MacroDef<'_>) {
-        self.visit_testable(
-            macro_def.ident.to_string(),
-            macro_def.hir_id(),
-            macro_def.span,
-            |_| (),
-        );
-    }
 }
 
 #[cfg(test)]
diff --git a/src/librustdoc/doctree.rs b/src/librustdoc/doctree.rs
@@ -5,6 +5,7 @@ use rustc_span::{self, Span, Symbol};
 
 use rustc_hir as hir;
 
+#[derive(Debug)]
 crate struct Module<'hir> {
     crate name: Symbol,
     crate where_inner: Span,
@@ -13,20 +14,11 @@ crate struct Module<'hir> {
     // (item, renamed)
     crate items: Vec<(&'hir hir::Item<'hir>, Option<Symbol>)>,
     crate foreigns: Vec<(&'hir hir::ForeignItem<'hir>, Option<Symbol>)>,
-    crate macros: Vec<(&'hir hir::MacroDef<'hir>, Option<Symbol>)>,
 }
 
 impl Module<'hir> {
     crate fn new(name: Symbol, id: hir::HirId, where_inner: Span) -> Module<'hir> {
-        Module {
-            name,
-            id,
-            where_inner,
-            mods: Vec::new(),
-            items: Vec::new(),
-            foreigns: Vec::new(),
-            macros: Vec::new(),
-        }
+        Module { name, id, where_inner, mods: Vec::new(), items: Vec::new(), foreigns: Vec::new() }
     }
 
     crate fn where_outer(&self, tcx: TyCtxt<'_>) -> Span {
diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs
@@ -9,7 +9,7 @@ use rustc_hir::Node;
 use rustc_middle::middle::privacy::AccessLevel;
 use rustc_middle::ty::TyCtxt;
 use rustc_span;
-use rustc_span::def_id::LOCAL_CRATE;
+use rustc_span::def_id::{CRATE_DEF_ID, LOCAL_CRATE};
 use rustc_span::source_map::Spanned;
 use rustc_span::symbol::{kw, sym, Symbol};
 
@@ -79,49 +79,23 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
             &krate.module(),
             self.cx.tcx.crate_name(LOCAL_CRATE),
         );
-        // 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(def.def_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 == path_segment_ty_ns) {
-                    Some(child_mod) => cur_mod = &mut *child_mod,
-                    None => continue 'exported_macros,
+
+        // `#[macro_export] macro_rules!` items are reexported at the top level of the
+        // crate, regardless of where they're defined. We want to document the
+        // top level rexport of the macro, not its original definition, since
+        // the rexport defines the path that a user will actually see. Accordingly,
+        // we add the rexport as an item here, and then skip over the original
+        // definition in `visit_item()` below.
+        for export in self.cx.tcx.module_exports(CRATE_DEF_ID).unwrap_or(&[]) {
+            if let Res::Def(DefKind::Macro(_), def_id) = export.res {
+                if let Some(local_def_id) = def_id.as_local() {
+                    if self.cx.tcx.has_attr(def_id, sym::macro_export) {
+                        let hir_id = self.cx.tcx.hir().local_def_id_to_hir_id(local_def_id);
+                        let item = self.cx.tcx.hir().expect_item(hir_id);
+                        top_level_module.items.push((item, None));
+                    }
                 }
             }
-            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.cache.exact_paths = self.exact_paths;
         top_level_module
@@ -238,10 +212,6 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
                 self.inlining = prev;
                 true
             }
-            Node::MacroDef(def) if !glob => {
-                om.macros.push((def, renamed));
-                true
-            }
             _ => false,
         };
         self.view_item_stack.remove(&res_hir_id);
@@ -257,7 +227,10 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
         debug!("visiting item {:?}", item);
         let name = renamed.unwrap_or(item.ident.name);
 
-        if item.vis.node.is_pub() {
+        let def_id = item.def_id.to_def_id();
+        let is_pub = item.vis.node.is_pub() || self.cx.tcx.has_attr(def_id, sym::macro_export);
+
+        if is_pub {
             self.store_path(item.def_id.to_def_id());
         }
 
@@ -269,7 +242,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
                 }
             }
             // If we're inlining, skip private items.
-            _ if self.inlining && !item.vis.node.is_pub() => {}
+            _ if self.inlining && !is_pub => {}
             hir::ItemKind::GlobalAsm(..) => {}
             hir::ItemKind::Use(_, hir::UseKind::ListStem) => {}
             hir::ItemKind::Use(ref path, kind) => {
@@ -285,7 +258,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
 
                 // If there was a private module in the current path then don't bother inlining
                 // anything as it will probably be stripped anyway.
-                if item.vis.node.is_pub() && self.inside_public_path {
+                if is_pub && self.inside_public_path {
                     let please_inline = attrs.iter().any(|item| match item.meta_item_list() {
                         Some(ref list) if item.has_name(sym::doc) => {
                             list.iter().any(|i| i.has_name(sym::inline))
@@ -307,6 +280,26 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
 
                 om.items.push((item, renamed))
             }
+            hir::ItemKind::Macro(ref macro_def) => {
+                // `#[macro_export] macro_rules!` items are handled seperately in `visit()`,
+                // above, since they need to be documented at the module top level. Accordingly,
+                // we only want to handle macros if one of three conditions holds:
+                //
+                // 1. This macro was defined by `macro`, and thus isn't covered by the case
+                //    above.
+                // 2. This macro isn't marked with `#[macro_export]`, and thus isn't covered
+                //    by the case above.
+                // 3. We're inlining, since a reexport where inlining has been requested
+                //    should be inlined even if it is also documented at the top level.
+
+                let def_id = item.def_id.to_def_id();
+                let is_macro_2_0 = !macro_def.macro_rules;
+                let nonexported = !self.cx.tcx.has_attr(def_id, sym::macro_export);
+
+                if is_macro_2_0 || nonexported || self.inlining {
+                    om.items.push((item, renamed));
+                }
+            }
             hir::ItemKind::Mod(ref m) => {
                 om.mods.push(self.visit_mod_contents(&item.vis, item.hir_id(), m, name));
             }
diff --git a/src/tools/clippy/clippy_lints/src/missing_doc.rs b/src/tools/clippy/clippy_lints/src/missing_doc.rs
@@ -122,8 +122,8 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {
             },
             hir::ItemKind::Const(..)
             | hir::ItemKind::Enum(..)
-            | hir::ItemKind::Mod(..)
             | hir::ItemKind::Macro(..)
+            | hir::ItemKind::Mod(..)
             | hir::ItemKind::Static(..)
             | hir::ItemKind::Struct(..)
             | hir::ItemKind::Trait(..)
diff --git a/src/tools/clippy/clippy_lints/src/missing_inline.rs b/src/tools/clippy/clippy_lints/src/missing_inline.rs
@@ -118,6 +118,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingInline {
             },
             hir::ItemKind::Const(..)
             | hir::ItemKind::Enum(..)
+            | hir::ItemKind::Macro(..)
             | hir::ItemKind::Mod(..)
             | hir::ItemKind::Static(..)
             | hir::ItemKind::Struct(..)
diff --git a/src/tools/clippy/clippy_lints/src/utils/inspector.rs b/src/tools/clippy/clippy_lints/src/utils/inspector.rs
@@ -381,6 +381,13 @@ fn print_item(cx: &LateContext<'_>, item: &hir::Item<'_>) {
             let item_ty = cx.tcx.type_of(did);
             println!("function of type {:#?}", item_ty);
         },
+        hir::ItemKind::Macro(ref macro_def) => {
+            if macro_def.macro_rules {
+                println!("macro introduced by `macro_rules!`");
+            } else {
+                println!("macro introduced by `macro`");
+            }
+        },
         hir::ItemKind::Mod(..) => println!("module"),
         hir::ItemKind::ForeignMod { abi, .. } => println!("foreign module with abi: {}", abi),
         hir::ItemKind::GlobalAsm(asm) => println!("global asm: {:?}", asm),
