Unused lint warnings for macros

[est31]

If you declare a variable, and not use it, you get an unused code warning.

The same should be true for macros (that don't have the #[macro_export] attribute). So if you have a private macro like:

fn main() {
    macro_rules! do_something {
        ($x:expr) => {$x + 20}
    }
}

It should give you a warning. Same goes if you use a macro, but not all of its match arms:

fn main() {
    macro_rules! do_something {
        ($x:expr) => {$x + 20};
        ($i:ident, $x:expr) => {$i = $x + 20};
    }
    println!("done: {}", do_something!(2));
}

Note that this is no dupe or not even against #24580. That issue is about unused code inside macro expansions, but my issue is about macro use.

By description, this falls under the definition of the dead-code lint.

[mcarton]

That's hard. Macro expansion happens before Clippy is run (so if you write do_something!(2) clippy actually only sees 2 + 20). We can know that something happened because of a macro but I don't know whether we can even get macro definitions and I'm sure we have no way to know what branch of the macro the expansion that comes from.

[mcarton]

(oups, I somehow though this was reported on clippy, but it still is hard I guess)

[est31]

So PR #41907 has implemented the first case: a lint for entirely not used macros. The second case (unused macro arms) is still unimplemented. I however am not sure whether it should be added, for the following reason:

When doing the PR mentioned above, I found some macros that got the unused warning but which were still used, just by cfg-out'd code. E.g. take this case:

macro_rules! do_something {
    ($x:expr) => {$x + 20}
}
#[cfg(windows)]
fn foo() {
    do_something!();
}

Here, the correct response was to add the cfg clause to the do_something clause as well:

#[cfg(windows)]
macro_rules! do_something {
    ($x:expr) => {$x + 20}
}
#[cfg(windows)]
fn foo() {
    do_something!();
}

Now, what about the following case:

macro_rules! do_something {
    ($x:expr) => {$x + 20};
    ($i:ident, $x:expr) => {$i = $x + 20};
}
fn main() {
    println!("done: {}", do_something!(2));
}
#[cfg(windows)]
fn foo() {
    let mut i = 4;
    println!("done: {}", do_something!(i, 2));
}

Here, we'll get a warning on non windows platforms that the second macro case is unused. Easy, to solve, right? you just add a cfg to the match arm:

macro_rules! do_something {
    ($x:expr) => {$x + 20};
    #[cfg(windows)]
    ($i:ident, $x:expr) => {$i = $x + 20};
}

That unfortunately doesn't work, the parser doesn't expect such cfg arms here.

So if such a lint were present, the only way to act here would be to do two macros, or to turn the lint off for the macro, both of which are not really satisfying answers.

Therefore I think that for now implementing the lint for macro arms as well is not really a thing we should do.

[est31]

PR #42334 has expanded the lint to macros 2.0. Closing.

[iago-lito]

Rustc now warns about unused macro, but not about unused macro arms. Would this be considered an improvement to #42334 or does it need be considered separately?

[est31]

@iago-lito see my comment about the impossibility to attach cfg to macro arms. The basic issue is that macro arms aren't assigned their own id's, at least back when I made the comment. It might be different now.

[est31]

I'd file a new issue for unused macro arms. It is possible to implement it but you'd need changes in the language so that you can cfg out branches.

[iago-lito]

@est31 Okay, thanks, I'll open that at least :) Do you think this new issue rather belongs to this repo or to the RFCs repo?

[est31]

I'd file one here. In the best case, it attracts more feedback here than in the RFCs repo, in the worst case, you are told to move the issue.