Auto merge of #88019 - inquisitivecrystal:macro-def, r=cjgillot

Treat macros as HIR items

Macros have historically been treated differently from other items at the HIR level. This PR makes them just like normal items. There are a few special cases left over, which I've attempted to lay out below. By normalizing the treatment of macro items, this PR simplifies a fair bit of code and fixes some bugs at the same time. For more information, see #87406.

r? `@cjgillot`

## Backwards incompatibility

This is backwards incompatible in one small way. Due to a mistake, it was previously possible to apply stability attributes to an exported macro, even without enabling the `staged_api` feature. This never should have worked. After this PR, it will error, as it should. We could make it a warning instead, but that would require a special case for a feature that shouldn't ever have worked and is likely used by no or very few crates, so I'm not thrilled about the idea.

## Notes for reviewers
### Commit seperation

I'd recommend reviewing this PR commit by commit. The commit chunking wasn't perfect, but it's better than looking at the combined diff, which is quite overwhelming. The compiler and standard library build after each commit, although tests do not necessarily pass and tools do not necessarily build till the end of the series.

### Special cases
There are a few special cases that remain after this change. Here are the notable ones I remember:

1. Visibility works a bit differently for `macro_rules!` macros than other items, since they aren't generally marked with `pub` but instead with `#[macro_export]`.
2. Since `#[macro_export]` macros always have paths at the top level of the crate, some additional handling needs to be done on the reexport to top level.
### Performance impact

I don't know for sure, but theses changes may slightly hurt performance. They create more work for the compiler in a few places. For instance, some operations that were previously run only for exported macros are now run for all macros. A perf run is probably advisable. For all I know we'll see performance improvements instead. :)

## Issues resolved

This resolves #87406 (the tracking issue for this change). It also fixes several bugs:

Fixes #59306.
Fixes #73754.
Fixes #87257.

Author: bors

Cargo.lock

@@ -4146,6 +4146,7 @@ dependencies = [
 name = "rustc_privacy"
 version = "0.0.0"
 dependencies = [
+ "rustc_ast",
  "rustc_attr",
  "rustc_data_structures",
  "rustc_errors",

compiler/rustc_ast_lowering/src/item.rs

@@ -170,7 +170,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 self.lower_item_id_use_tree(use_tree, i.id, &mut vec);
                 vec
             }
-            ItemKind::MacroDef(..) => SmallVec::new(),
             ItemKind::Fn(..) | ItemKind::Impl(box ImplKind { of_trait: None, .. }) => {
                 smallvec![i.id]
             }
@@ -212,28 +211,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
     pub fn lower_item(&mut self, i: &Item) -> Option<hir::Item<'hir>> {
         let mut ident = i.ident;
         let mut vis = self.lower_visibility(&i.vis, None);
-
-        if let ItemKind::MacroDef(MacroDef { ref body, macro_rules }) = i.kind {
-            if !macro_rules || self.sess.contains_name(&i.attrs, sym::macro_export) {
-                let hir_id = self.lower_node_id(i.id);
-                self.lower_attrs(hir_id, &i.attrs);
-                let body = P(self.lower_mac_args(body));
-                self.insert_macro_def(hir::MacroDef {
-                    ident,
-                    vis,
-                    def_id: hir_id.expect_owner(),
-                    span: i.span,
-                    ast: MacroDef { body, macro_rules },
-                });
-            } else {
-                for a in i.attrs.iter() {
-                    let a = self.lower_attr(a);
-                    self.non_exported_macro_attrs.push(a);
-                }
-            }
-            return None;
-        }
-
         let hir_id = self.lower_node_id(i.id);
         let attrs = self.lower_attrs(hir_id, &i.attrs);
         let kind = self.lower_item_kind(i.span, i.id, hir_id, &mut ident, attrs, &mut vis, &i.kind);
@@ -465,7 +442,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 self.lower_generics(generics, ImplTraitContext::disallowed()),
                 self.lower_param_bounds(bounds, ImplTraitContext::disallowed()),
             ),
-            ItemKind::MacroDef(..) | ItemKind::MacCall(..) => {
+            ItemKind::MacroDef(MacroDef { ref body, macro_rules }) => {
+                let body = P(self.lower_mac_args(body));
+
+                hir::ItemKind::Macro(ast::MacroDef { body, macro_rules })
+            }
+            ItemKind::MacCall(..) => {
                 panic!("`TyMac` should have been expanded by now")
             }
         }

compiler/rustc_ast_lowering/src/lib.rs

@@ -103,7 +103,6 @@ struct LoweringContext<'a, 'hir: 'a> {
     /// The items being lowered are collected here.
     owners: IndexVec<LocalDefId, Option<hir::OwnerNode<'hir>>>,
     bodies: BTreeMap<hir::BodyId, hir::Body<'hir>>,
-    non_exported_macro_attrs: Vec<ast::Attribute>,
 
     trait_impls: BTreeMap<DefId, Vec<LocalDefId>>,
 
@@ -330,7 +329,6 @@ pub fn lower_crate<'a, 'hir>(
         trait_impls: BTreeMap::new(),
         modules: BTreeMap::new(),
         attrs: BTreeMap::default(),
-        non_exported_macro_attrs: Vec::new(),
         catch_scopes: Vec::new(),
         loop_scopes: Vec::new(),
         is_in_loop_condition: false,
@@ -551,7 +549,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         }
 
         let krate = hir::Crate {
-            non_exported_macro_attrs: self.arena.alloc_from_iter(self.non_exported_macro_attrs),
             owners: self.owners,
             bodies: self.bodies,
             body_ids,
@@ -600,13 +597,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         id
     }
 
-    fn insert_macro_def(&mut self, item: hir::MacroDef<'hir>) {
-        let def_id = item.def_id;
-        let item = self.arena.alloc(item);
-        self.owners.ensure_contains_elem(def_id, || None);
-        self.owners[def_id] = Some(hir::OwnerNode::MacroDef(item));
-    }
-
     fn allocate_hir_id_counter(&mut self, owner: NodeId) -> hir::HirId {
         // Set up the counter if needed.
         self.item_local_id_counters.entry(owner).or_insert(0);

compiler/rustc_ast_pretty/src/pprust/state.rs

@@ -578,6 +578,33 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
         }
     }
 
+    fn print_mac_def(
+        &mut self,
+        macro_def: &ast::MacroDef,
+        ident: &Ident,
+        sp: &Span,
+        print_visibility: impl FnOnce(&mut Self),
+    ) {
+        let (kw, has_bang) = if macro_def.macro_rules {
+            ("macro_rules", true)
+        } else {
+            print_visibility(self);
+            ("macro", false)
+        };
+        self.print_mac_common(
+            Some(MacHeader::Keyword(kw)),
+            has_bang,
+            Some(*ident),
+            macro_def.body.delim(),
+            &macro_def.body.inner_tokens(),
+            true,
+            *sp,
+        );
+        if macro_def.body.need_semicolon() {
+            self.word(";");
+        }
+    }
+
     fn print_path(&mut self, path: &ast::Path, colons_before_params: bool, depth: usize) {
         self.maybe_print_comment(path.span.lo());
 
@@ -1305,24 +1332,9 @@ impl<'a> State<'a> {
                 }
             }
             ast::ItemKind::MacroDef(ref macro_def) => {
-                let (kw, has_bang) = if macro_def.macro_rules {
-                    ("macro_rules", true)
-                } else {
-                    self.print_visibility(&item.vis);
-                    ("macro", false)
-                };
-                self.print_mac_common(
-                    Some(MacHeader::Keyword(kw)),
-                    has_bang,
-                    Some(item.ident),
-                    macro_def.body.delim(),
-                    &macro_def.body.inner_tokens(),
-                    true,
-                    item.span,
-                );
-                if macro_def.body.need_semicolon() {
-                    self.word(";");
-                }
+                self.print_mac_def(macro_def, &item.ident, &item.span, |state| {
+                    state.print_visibility(&item.vis)
+                });
             }
         }
         self.ann.post(self, AnnNode::Item(item))

compiler/rustc_hir/src/arena.rs

@@ -35,7 +35,6 @@ macro_rules! arena_types {
             [few] inline_asm: rustc_hir::InlineAsm<$tcx>,
             [few] llvm_inline_asm: rustc_hir::LlvmInlineAsm<$tcx>,
             [] local: rustc_hir::Local<$tcx>,
-            [few] macro_def: rustc_hir::MacroDef<$tcx>,
             [few] mod_: rustc_hir::Mod<$tcx>,
             [] param: rustc_hir::Param<$tcx>,
             [] pat: rustc_hir::Pat<$tcx>,

compiler/rustc_hir/src/hir.rs

@@ -670,9 +670,6 @@ pub struct ModuleItems {
 /// [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/hir.html
 #[derive(Debug)]
 pub struct Crate<'hir> {
-    // Attributes from non-exported macros, kept only for collecting the library feature list.
-    pub non_exported_macro_attrs: &'hir [Attribute],
-
     pub owners: IndexVec<LocalDefId, Option<OwnerNode<'hir>>>,
     pub bodies: BTreeMap<BodyId, Body<'hir>>,
     pub trait_impls: BTreeMap<DefId, Vec<LocalDefId>>,
@@ -743,7 +740,7 @@ impl Crate<'_> {
                 OwnerNode::ForeignItem(item) => visitor.visit_foreign_item(item),
                 OwnerNode::ImplItem(item) => visitor.visit_impl_item(item),
                 OwnerNode::TraitItem(item) => visitor.visit_trait_item(item),
-                OwnerNode::MacroDef(_) | OwnerNode::Crate(_) => {}
+                OwnerNode::Crate(_) => {}
             }
         }
     }
@@ -758,7 +755,7 @@ impl Crate<'_> {
             Some(OwnerNode::ForeignItem(item)) => visitor.visit_foreign_item(item),
             Some(OwnerNode::ImplItem(item)) => visitor.visit_impl_item(item),
             Some(OwnerNode::TraitItem(item)) => visitor.visit_trait_item(item),
-            Some(OwnerNode::MacroDef(_)) | Some(OwnerNode::Crate(_)) | None => {}
+            Some(OwnerNode::Crate(_)) | None => {}
         })
     }
 
@@ -768,32 +765,6 @@ impl Crate<'_> {
             _ => None,
         })
     }
-
-    pub fn exported_macros<'hir>(&'hir self) -> impl Iterator<Item = &'hir MacroDef<'hir>> + 'hir {
-        self.owners.iter().filter_map(|owner| match owner {
-            Some(OwnerNode::MacroDef(macro_def)) => Some(*macro_def),
-            _ => None,
-        })
-    }
-}
-
-/// A macro definition, in this crate or imported from another.
-///
-/// Not parsed directly, but created on macro import or `macro_rules!` expansion.
-#[derive(Debug)]
-pub struct MacroDef<'hir> {
-    pub ident: Ident,
-    pub vis: Visibility<'hir>,
-    pub def_id: LocalDefId,
-    pub span: Span,
-    pub ast: ast::MacroDef,
-}
-
-impl MacroDef<'_> {
-    #[inline]
-    pub fn hir_id(&self) -> HirId {
-        HirId::make_owner(self.def_id)
-    }
 }
 
 /// A block of statements `{ .. }`, which may have a label (in this case the
@@ -2602,7 +2573,7 @@ pub struct PolyTraitRef<'hir> {
 
 pub type Visibility<'hir> = Spanned<VisibilityKind<'hir>>;
 
-#[derive(Debug)]
+#[derive(Copy, Clone, Debug)]
 pub enum VisibilityKind<'hir> {
     Public,
     Crate(CrateSugar),
@@ -2791,6 +2762,8 @@ pub enum ItemKind<'hir> {
     Const(&'hir Ty<'hir>, BodyId),
     /// A function declaration.
     Fn(FnSig<'hir>, Generics<'hir>, BodyId),
+    /// A MBE macro definition (`macro_rules!` or `macro`).
+    Macro(ast::MacroDef),
     /// A module.
     Mod(Mod<'hir>),
     /// An external module, e.g. `extern { .. }`.
@@ -2856,6 +2829,7 @@ impl ItemKind<'_> {
             ItemKind::Static(..) => "static item",
             ItemKind::Const(..) => "constant item",
             ItemKind::Fn(..) => "function",
+            ItemKind::Macro(..) => "macro",
             ItemKind::Mod(..) => "module",
             ItemKind::ForeignMod { .. } => "extern block",
             ItemKind::GlobalAsm(..) => "global asm item",
@@ -2996,7 +2970,6 @@ pub enum OwnerNode<'hir> {
     ForeignItem(&'hir ForeignItem<'hir>),
     TraitItem(&'hir TraitItem<'hir>),
     ImplItem(&'hir ImplItem<'hir>),
-    MacroDef(&'hir MacroDef<'hir>),
     Crate(&'hir Mod<'hir>),
 }
 
@@ -3006,8 +2979,7 @@ impl<'hir> OwnerNode<'hir> {
             OwnerNode::Item(Item { ident, .. })
             | OwnerNode::ForeignItem(ForeignItem { ident, .. })
             | OwnerNode::ImplItem(ImplItem { ident, .. })
-            | OwnerNode::TraitItem(TraitItem { ident, .. })
-            | OwnerNode::MacroDef(MacroDef { ident, .. }) => Some(*ident),
+            | OwnerNode::TraitItem(TraitItem { ident, .. }) => Some(*ident),
             OwnerNode::Crate(..) => None,
         }
     }
@@ -3018,7 +2990,6 @@ impl<'hir> OwnerNode<'hir> {
             | OwnerNode::ForeignItem(ForeignItem { span, .. })
             | OwnerNode::ImplItem(ImplItem { span, .. })
             | OwnerNode::TraitItem(TraitItem { span, .. })
-            | OwnerNode::MacroDef(MacroDef { span, .. })
             | OwnerNode::Crate(Mod { inner: span, .. }) => *span,
         }
     }
@@ -3062,8 +3033,7 @@ impl<'hir> OwnerNode<'hir> {
             OwnerNode::Item(Item { def_id, .. })
             | OwnerNode::TraitItem(TraitItem { def_id, .. })
             | OwnerNode::ImplItem(ImplItem { def_id, .. })
-            | OwnerNode::ForeignItem(ForeignItem { def_id, .. })
-            | OwnerNode::MacroDef(MacroDef { def_id, .. }) => *def_id,
+            | OwnerNode::ForeignItem(ForeignItem { def_id, .. }) => *def_id,
             OwnerNode::Crate(..) => crate::CRATE_HIR_ID.owner,
         }
     }
@@ -3095,13 +3065,6 @@ impl<'hir> OwnerNode<'hir> {
             _ => panic!(),
         }
     }
-
-    pub fn expect_macro_def(self) -> &'hir MacroDef<'hir> {
-        match self {
-            OwnerNode::MacroDef(n) => n,
-            _ => panic!(),
-        }
-    }
 }
 
 impl<'hir> Into<OwnerNode<'hir>> for &'hir Item<'hir> {
@@ -3128,20 +3091,13 @@ impl<'hir> Into<OwnerNode<'hir>> for &'hir TraitItem<'hir> {
     }
 }
 
-impl<'hir> Into<OwnerNode<'hir>> for &'hir MacroDef<'hir> {
-    fn into(self) -> OwnerNode<'hir> {
-        OwnerNode::MacroDef(self)
-    }
-}
-
 impl<'hir> Into<Node<'hir>> for OwnerNode<'hir> {
     fn into(self) -> Node<'hir> {
         match self {
             OwnerNode::Item(n) => Node::Item(n),
             OwnerNode::ForeignItem(n) => Node::ForeignItem(n),
             OwnerNode::ImplItem(n) => Node::ImplItem(n),
             OwnerNode::TraitItem(n) => Node::TraitItem(n),
-            OwnerNode::MacroDef(n) => Node::MacroDef(n),
             OwnerNode::Crate(n) => Node::Crate(n),
         }
     }
@@ -3167,7 +3123,6 @@ pub enum Node<'hir> {
     Arm(&'hir Arm<'hir>),
     Block(&'hir Block<'hir>),
     Local(&'hir Local<'hir>),
-    MacroDef(&'hir MacroDef<'hir>),
 
     /// `Ctor` refers to the constructor of an enum variant or struct. Only tuple or unit variants
     /// with synthesized constructors.
@@ -3204,7 +3159,6 @@ impl<'hir> Node<'hir> {
             | Node::ForeignItem(ForeignItem { ident, .. })
             | Node::Field(FieldDef { ident, .. })
             | Node::Variant(Variant { ident, .. })
-            | Node::MacroDef(MacroDef { ident, .. })
             | Node::Item(Item { ident, .. })
             | Node::PathSegment(PathSegment { ident, .. }) => Some(*ident),
             Node::Lifetime(lt) => Some(lt.name.ident()),
@@ -3265,8 +3219,7 @@ impl<'hir> Node<'hir> {
             Node::Item(Item { def_id, .. })
             | Node::TraitItem(TraitItem { def_id, .. })
             | Node::ImplItem(ImplItem { def_id, .. })
-            | Node::ForeignItem(ForeignItem { def_id, .. })
-            | Node::MacroDef(MacroDef { def_id, .. }) => Some(HirId::make_owner(*def_id)),
+            | Node::ForeignItem(ForeignItem { def_id, .. }) => Some(HirId::make_owner(*def_id)),
             Node::Field(FieldDef { hir_id, .. })
             | Node::AnonConst(AnonConst { hir_id, .. })
             | Node::Expr(Expr { hir_id, .. })
@@ -3326,7 +3279,6 @@ impl<'hir> Node<'hir> {
             Node::ForeignItem(i) => Some(OwnerNode::ForeignItem(i)),
             Node::TraitItem(i) => Some(OwnerNode::TraitItem(i)),
             Node::ImplItem(i) => Some(OwnerNode::ImplItem(i)),
-            Node::MacroDef(i) => Some(OwnerNode::MacroDef(i)),
             Node::Crate(i) => Some(OwnerNode::Crate(i)),
             _ => None,
         }

compiler/rustc_hir/src/intravisit.rs

@@ -466,9 +466,6 @@ pub trait Visitor<'v>: Sized {
         walk_assoc_type_binding(self, type_binding)
     }
     fn visit_attribute(&mut self, _id: HirId, _attr: &'v Attribute) {}
-    fn visit_macro_def(&mut self, macro_def: &'v MacroDef<'v>) {
-        walk_macro_def(self, macro_def)
-    }
     fn visit_vis(&mut self, vis: &'v Visibility<'v>) {
         walk_vis(self, vis)
     }
@@ -484,19 +481,13 @@ pub trait Visitor<'v>: Sized {
 pub fn walk_crate<'v, V: Visitor<'v>>(visitor: &mut V, krate: &'v Crate<'v>) {
     let top_mod = krate.module();
     visitor.visit_mod(top_mod, top_mod.inner, CRATE_HIR_ID);
-    walk_list!(visitor, visit_macro_def, krate.exported_macros());
     for (&id, attrs) in krate.attrs.iter() {
         for a in *attrs {
             visitor.visit_attribute(id, a)
         }
     }
 }
 
-pub fn walk_macro_def<'v, V: Visitor<'v>>(visitor: &mut V, macro_def: &'v MacroDef<'v>) {
-    visitor.visit_id(macro_def.hir_id());
-    visitor.visit_ident(macro_def.ident);
-}
-
 pub fn walk_mod<'v, V: Visitor<'v>>(visitor: &mut V, module: &'v Mod<'v>, mod_hir_id: HirId) {
     visitor.visit_id(mod_hir_id);
     for &item_id in module.item_ids {
@@ -586,6 +577,9 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item<'v>) {
             item.span,
             item.hir_id(),
         ),
+        ItemKind::Macro(_) => {
+            visitor.visit_id(item.hir_id());
+        }
         ItemKind::Mod(ref module) => {
             // `visit_mod()` takes care of visiting the `Item`'s `HirId`.
             visitor.visit_mod(module, item.span, item.hir_id())