Macro definitions should be regular HIR items

[cjgillot]

Macro definitions are currently handled separately from other item kinds in HIR, either as MacroDef nodes inside hir::Crate::exported_macros or as using the hir::Crate::non_exported_macro_attrs.

They should be transformed into additional variants in the hir::ItemKind enum, and iterated over like any other item.

Instructions:

• introduce two additional variants hir::ItemKind::ExportedMacro { ast: ast::MacroDef } and hir::ItemKind::NonExportedMacro;
• move the macro case in rustc_ast_lowering::item::LoweringContext::lower_item into lower_item_kind;
• remove hir::OwnerNode::{MacroDef, NonExportedMacro} variants.

I recomment waiting on #83723 and #87234 to be merged.

Original idea by @petrochenkov in #83723 (comment)
Should help with #73754

[jyn514]

See also #77862, #87257, #79396 (comment).

[inquisitivecrystal]

ExportedMacro (or indeed, exported_macros) is a fairly misleading name, since this includes non exported MBE 2.0 macros. I brought this up in #86968, but I haven't opened a follow-up discussion (partly because of a lack of bandwidth, and partly because it seemed like the relevant code needed to be rewritten at some point).

Incidentally, how difficult do you think this task will be?

[jyn514]

@inquisitivecrystal my understanding without looking into it much is that basically everything that touches exported_macros today will have to be rewritten. So at least E-medium, possibly E-hard. But @petrochenkov would have a better idea than me.

[inquisitivecrystal]

Alright. I'm currently looking at whether this might be within my ability to implement. I don't like saying I'll do something until I I've checked that I have some chance of figuring it out, but the E-mentor helps a bit with that.

[inquisitivecrystal]

I'm going to give this a go! There's no way of knowing for sure if I can do it until I actually try it out, and I can't do that properly until the PRs mentioned in the initial post are merged.

[cjgillot]

@inquisitivecrystal : #87234 contains a perf regression which may very well be caused by this bug. I think you can go ahead with the implementation, I will rebase it on top of your work.
923ae28c0d60720a76782803ff478e49b099746c introduces the hir::OwnerNode::NonExportedMacro variant I mention in the top post. This commit will point the right place to modify, but you should not need to cherry-pick it.
Don't hesitate to contact me on zulip if you need more precisions.

[inquisitivecrystal]

I'm on it!

[inquisitivecrystal]

I'm making progress on this.

I'm going step by step, and I'm on step two, which is migrating the way rustc_ast_lowering encodes macros to hir::Item. I've just gotten to the point where the standard library builds again. Getting this far has taken... a while. I'm not sure exactly, but it wouldn't surprise me if if you told me I'd spent ten hours on everything put together. (I actually started before my last post.) I'm not very experienced, which is probably why it's taking so long. For reference, there was one bug that took a few hours to track down on its own.

My next subgoal is making the tests pass again. I currently have nine failing tests. As far as I can tell, there are at least two distinct bugs that are causing tests to fail, and possibly more. After I fix those, I've got to move onto the future steps. Here's what the checklist looks like:

• Add macros to hir::ItemKind
• Change over to the new macro representation
• Remove macros fromOwnerNode
• Lift #[export_macro] macros to the top level
• Change rustdoc to the new system
• Add tests
• General code cleanup

I'll update that as I go. I may slow down a bit, as I'm not really sure I can keep up this intensity. :)

[jyn514]

I'll update that as I go. I may slow down a bit, as I'm not really sure I can keep up this intensity. :)

No worries, slow progress is better than no progress :) thank you for working on this!

[inquisitivecrystal]

I'm still working on this. Lifting the macros to top level is surprisingly complicated, but I'm making headway.

[inquisitivecrystal]

After a fair bit of work, the standard library now builds again, even with the macros being lifted to the top level. However, I do still need to make the tests pass again. I'm also definitely going to need to spend a while cleaning my code up. It's horribly messy.

[inquisitivecrystal]

I had to take a break for a few days, but I'm back at it. I'm hoping I'll be able to have a PR out for people to comment on soonish.

[inquisitivecrystal]

The PR is up! It's #88019.