Fix unused attributes on macro_rules.

[ehuss]

The unused_attributes lint wasn't firing on attributes of macro_rules definitions. The consequence is that many attributes are silently ignored on macro_rules. The reason is that unused_attributes is a late-lint pass, and only has access to the HIR, which does not have macro_rules definitions.

My solution here is to change non_exported_macro_attrs to be macro_attrs (a list of all attributes used for macro_rules, instead of just those for macro_export), and then to check this list in the unused_attributes lint. There are a number of alternate approaches, but this seemed the most reliable and least invasive. I am open to completely different approaches, though.

One concern is that I don't fully understand the implications of extending non_exported_macro_attrs to include non-exported macros. That list was originally added in #62042 to handle stability attributes, so I suspect it was just an optimization since that was all that was needed. It was later extended to be included in SVH in #83901. #80641 also added a use to check for invalid attributes, which seems a little odd to me (it didn't validate non-exported macros, and seems highly specific).

Overall, there doesn't seem to be a clear story of when unused_attributes should be used versus an error like E0518. I considered alternatively using an "allow list" of built-in attributes that can be used on macro_rules (allow, warn, deny, forbid, cfg, cfg_attr, macro_export, deprecated, doc), but I feel like that could be a pain to maintain.

Some built-in attributes already present hard-errors when used with macro_rules. These are each hard-coded in various places:

• derive
• test and bench
• proc_macro and proc_macro_derive
• inline
• global_allocator

The primary motivation is that I sometimes see people use #[macro_use] in front of macro_rules, which indicates there is some confusion out there (evident that there was even a case of it in rustc).

[rust-highfive]

r? @petrochenkov

(rust-highfive has picked a reviewer for you, use r? to override)

[petrochenkov]

I am open to completely different approaches, though.

The proper solution is to keep all macros in HIR like any other items (#73754 (comment)), it would help with other issues as well.
That's a significantly larger work though.

[ehuss]

The proper solution is to keep all macros in HIR

I'd be willing to try that if you want. Is that something you'd be willing to review? Does it just boil down to adding macros to hir::ItemKind and then dealing with the consequences?

[petrochenkov]

@bors r+

[bors]

📌 Commit 5bbc240 has been approved by petrochenkov

[petrochenkov]

Is that something you'd be willing to review?

Yes, sure.

Does it just boil down to adding macros to hir::ItemKind and then dealing with the consequences?

Yes, with "dealing with the consequences" probably being the harder part.

[bors]

⌛ Testing commit 5bbc240 with merge fe72845...

[bors]

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing fe72845 to master...