dead code (+ unused assignment, etc) warnings in macros do more harm than good

[pnkfelix]

(imported from improperly closed bug #17427)

Consider the following code:

fn g(x: u8) -> u8 {
    1 - x
}

fn main() {
    let mut x = 1u8;
    macro_rules! m {
        () => {{ x = g(x);
                 if x == 0 { x = 1; }
        }}
    }

    m!();
    g(1/x);
    m!();
}

playpen

The unused_assignments lint fires from the expansion of the second occurrence of m!(). But if you follow the advice of the lint and remove the assignment, you discover that the assignment was in fact significant, because when you remove the assignment, the side effect from the first occurrence of m!() is lost, and so the call to g divides by zero.

There are a number of different ways to handle this.

• This simplest would be to disable such lints for code with spans that are inside macro definitions, as was essentially the original suggestion of Allow dead assignments in macros by default. #17427
• Another option would be to revise our strategy for such lints, and to warn about an unused assignment (or other dead code) for a given span only if ALL such occurrences trigger the warning. I.e. as soon as one expansion proves that that code in the macro is useful, then you do not get any warning about it.

[xaviershay]

I would like this. http://doc.rust-lang.org/bitflags/bitflags/macro.bitflags!.html is really annoying without it.

[xaviershay]

In the mean time I'm using the following workaround:

impl Sides {
    // Execute all the code paths to shut up warnings.
    // FIX: https://github.com/rust-lang/rust/issues/24580
    #[allow(dead_code)]
    fn _dead_code(&mut self) {
        self.is_all();
        self.is_empty();
        self.bits();
        self.intersects(*self);
        self.remove(SIDE_RIGHT);
        self.toggle(SIDE_RIGHT);
        Sides::from_bits(0b00000000);
        Sides::from_bits_truncate(0b00000000);
    }
}

It's not generic - you'll need to customize it for your struct - but at least it's something.

[brson]

Still repros.

[brson]

@pnkfelix Are you still passionate about this?

[SirVer]

I would still want this if possible. I have this macro:

macro_rules! assert_packet {
    ($var:ident, $( $x:expr ),+ ) => {
        {
            let mut index = 0;
            $(
                match $x.into() {
                    PacketMatchers::Str(a) => assert_eq!(a, $var[index]),
                    PacketMatchers::Regex(a) => {
                        let reg = regex::Regex::new(a).unwrap();
                        if !reg.is_match(&$var[index]) {
                            panic!("'{}' does not match regex: '{}'.", $var[index], a);
                        }
                    }
                }
                index += 1;
            )*
        }
    };
}

This is for testing variable lengths packets (lists of strings) in a custom protocol. Usage is as follows:

assert_packet!(packet, "LOGIN", "testuser", Regex(r"[A-Z]+"));

I am rather pleased that macro and assert code are easy to read and understand, but for the last instance of the macro expansion I always get warning: value assigned to index is never read, which is displeasing.

Maybe there is a way to avoid it, but I did not find it yet - I added a semantically non-sensical assert!(index <= 1000000); to shut this up for now.

[mlwilkerson]

+1 for me. I'm trying to figure out a workaround for these warnings as well.