Implement a lint to warn about unused macro rules

[est31]

This implements a new lint to warn about unused macro rules (arms/matchers), similar to the unused_macros lint added by #41907 that warns about entire macros.

macro_rules! unused_empty {
    (hello) => { println!("Hello, world!") };
    () => { println!("empty") }; //~ ERROR: 2nd rule of macro `unused_empty` is never used
}

fn main() {
    unused_empty!(hello);
}

Builds upon #96149 and #96156.

Fixes #73576

[rust-highfive]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[rust-highfive]

r? @fee1-dead

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

[rust-highfive]

⚠️ Warning ⚠️

• These commits modify compiler targets. (See the Target Tier Policy.)

[est31]

#96149 is merged now. r? @fee1-dead

[est31]

One rule is used to give a better compile error when an impossible condition is used: https://fxrev.dev/c/fuchsia/+/679455/1/src/lib/mundane/src/boringssl/wrapper.rs , so it being unused is expected.

I'm wondering about making the lint not fire in this instance. On one hand, it feels a bit niche/surprising behavior to me, but on the other hand, there are few "parse time" lints that could have possible interactions with compile_error!, so it's expected to be niche.

Furthermore, the very point of compile_error! is to be not expanded during a successful compilation, as in, it exists to be "unused". A direct invocation of compile_error! is easy to detect, but it could be hidden through indirection, which is quite hard to unwrangle so I think not a good idea. I think that supporting direct invocations first would be a good start.

but I would hesitate to remove the first one since if code in the future were to use it, it would result in a change in behavior rather than a compile failure (since the u8 case would fall through to the $self: ty case).

Yeah I guess in that case it's better to add an #[allow(...)]. For the different combinations of arguments use case, it probably depends on your personal taste. I lean towards removing stuff as it is still preserved in git history, but shrug.

Thanks for writing all of this up, @ComputerDruid !

---

As any possible changes to the lint to make it more permissive (like the ones I discussed above) won't make it into the 1.62 release, which is the first release to feature the lint, it might be a good idea to turn the lint to allow by default until they are implemented. The second advantage is that it makes the introduction easier, as in, you can avoid allowing the unknown_lints lint in more cases (depending on the Rust compiler versions you as a developer care about). I wonder what others think.

[fee1-dead]

I wonder if this is more suitable for clippy. If this kind of lint has a high chance of linting something that it is not supposed to, then it might just be too annoying to be in rustc.

[est31]

That's an interesting suggestion. Rust lints have a way higher standard than clippy lints, they must be almost always right and also not be opinionated. For unused code, it's already a thing that rust warns about it so from an opinionatedness point of view, the lint should be fine (although there was that case in the experience report where the arguments were in weird orders, where it's a taste based question). For being almost always right, I skimmed @ComputerDruid 's PR and almost all cases seemed like the lint improved the situation, or it was a clear case for the ? kleene operator.

[est31]

Another data point: syn's changes after the PR was merged.

[est31]

@rustbot labels: -relnotes

[est31]

More data:

• hermit-os/kernel@0c19418 added an allow
• ComunidadAylas/PackSquash@fb5012a removed the rule
• Alexhuszagh/rust-lexical@5c2732c added an allow (next to an allow for unused_macros)

So most of the time people are adding allows instead of removing rules. In addition to the list found through github's global commit search, I've also encountered the lint in glium. I've removed the rule there, because it never matched, not even on Rust 1.0 (although glium predates rust 1.0). It also didn't compile any more due to refactors, so I figured it was code added through development that is now not neccessary any more.

Generally I didn't expect to find so much allows compared to the small number of removals. I wonder if the lint is useful to people. On the other hand, one of the people who made the commits above added a ❤️ reaction to this PR even though they added an allow.

[est31]

Crossbeam had the same compile_error issue: crossbeam-rs/crossbeam#827

Also a PR that removed unused rules instead of adding an allow.

[fee1-dead]

If these rules all unconditionally expand to compile_error, could we do a "dummy expand" to detect such cases? Although it might be too expensive and unnecessarily complex.

[est31]

@fee1-dead a dummy expand would add another entire mode to expansion which is quite an undertaking... and it's not fully known how the code block would be expanded as well, as the macro rule might take some input and the input might change whether compile_error is invoked or not.

    (@parse $($tail:tt)*) => {
        invoke_mac_if_foo!(compile_error, $($tail:tt)*)
    };

And invoke_mac_if_foo invokes the given macro if the tail passed to it matches "foo", or something, otherwise it doesn't invoke it.

Even if you "just" wanted to parse the output, you'd be in trouble because of the potential $ syntax that is not normal Rust code. Also you don't know in which context you are.

The simplest solution, the one I'd suggest, is to just scan all tokens in the rule, then look for a literal named compile_error followed by a bang. This doesn't catch a lot of stuff, doesn't work with reexports of compile_error for example. It also "matches" in the case of the compile_error macro being shadowed, which it probably shouldn't. But it's better than nothing.

Due to the highly dynamic, highly untyped nature of Rust's macros I doubt you can get something better than that.

I'd say that 99% of cases just invoke compile_error unconditionally and at the top level of the block.