Rust: Warn about unused macro arms.

[iago-lito]

rustc now warns about unused macros. With the following code:

macro_rules! used {
    () => {};
}

macro_rules! unused { // <- This should be cleaned up.
    () => {};
}

fn main() {
    used!();
} 

we get the warning:

warning: unused macro definition
 --> src/main.rs:5:1
  |
5 | / macro_rules! unused {
6 | |     () => {};
7 | | }
  | |_^
  |
  = note: `#[warn(unused_macros)]` on by default

This is really helpful while cleaning up a large meta-programming mess.

However, it does not warn about unused macro arms, so the following:

macro_rules! used_macro {
    (used_arm) => {};
    (unused_arm) => {}; // <-- This should be cleaned up.
}

fn main() {
    used_macro!(used_arm);
} 

yields no warning yet.

Following #34938, I open this issue for discussing this feature. Is this something desirable to have rustc warn about unused macro arms?

[lcnr]

I personally think that we should not warn for unused macro arms by default, as I tend to fairly often write the following:

macro_rules! used_macro {
    ($($e:expr),*) => {};
    ($($e:expr),*,) => {}; // <-- This is unused.
}

fn main() {
    used_macro!(7, 13);
} 

To not have to worry about this in case I ever change a macro invocation to

use_macro!(
    7,
    13,
);

[iago-lito]

@lcnr This is convenient indeed :) But I don't think it should be blocking, for several reasons:

• First, ($($e:expr),*$(,)?) already works fine in this situation:

macro_rules! used_macro {
    ($($e:expr),*$(,)?) => {}; // Only one used arm.
}

fn main() {
    used_macro!(7, 13); // Without the comma.
    used_macro!(
        7,
        13, // With the comma.
        );
}

• Or, if this alternate solution is still not acceptable for some reason:

  • Then this problem is still only a matter of style, so it could maybe be special-cased and/or handled by rustfmt?

  • In any case, like we have the option to opt-out from this warning on a local basis for unused variables (let _unused_var = ();) with a _ prefix, we might consider an option to opt-out from this warning for unused macro arms, although I'm afraid it would need new syntax, like

  _(unused_arm) => { ... } // or
  (unused_arm) _=> { ... } // (eew.) or
  (unused_arm) => _{ ... } // (ew.)
  

  • Alternately, we could consider the warning to be a way to discourage such syntax extensions with redundant syntactic forms (like a macro working both with and without the comma).

In any case, I think that this is a good point, but also that it does not hinder the need for a warning about unused macro arms.

[est31]

One concern I have is that it's not possible to cfg-gate single macro arms, which would be needed in situations I've outlined in this comment: https://github.com/rust-lang/rust/issues/34938#issuecomment-302940520

[petrochenkov]

@est31
You could put an allow on the whole macro.

#[allow(unused_macro_arms)]
macro foo {
    used_arm => {},
    conditionally_used_arm => {}
}

Not perfect, but still a pretty good approximation.
Macros with platform-dependent use of arms are not common.

[est31]

Good point. Concern resolved! Would love to see this implemented. Begone, unused stuff!

[iago-lito]

@petrochenkov Is this finer-grained alternate allow line straightforward to feature, or would it need new syntax?

macro foo {
    used_arm => {},
    #[allow(unused_macro_arm)]
    conditionally_used_arm => {}
}

[petrochenkov]

@iago-lito
It will need some work, right now the whole macro body is treated as a single token stream.
The individual arms will need to be kept as nodes in AST and assigned NodeIds, then you'll be able to attach lint attributes like allow to them.
Also, the attributes on macro arms are indeed a new syntax.

[est31]

You'd likely require ids to implement the lint in the first place, so at least that cost has to be paid already. At least that's how I did it in #41907 (with a set of ids of unused macros), idk how much it has changed since then.

[petrochenkov]

For uniquely identifying the arm having only the macro's NodeId + the arm's index should be enough, then this pair can be used to track which arms are used and which are not.

For applying allows to macro arms you need real NodeIds for them because that's what the lint level infra works with.