Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
pnkfelix opened this issue Apr 18, 2015 · 6 comments
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. P-low Low priority T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@pnkfelix
Copy link
Member

(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.
@steveklabnik steveklabnik added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. labels Apr 20, 2015
@xaviershay
Copy link
Contributor

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

@xaviershay
Copy link
Contributor

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
Copy link
Contributor

brson commented Mar 9, 2017

Still repros.

@brson brson added I-wishlist P-low Low priority labels Mar 9, 2017
@brson
Copy link
Contributor

brson commented Mar 9, 2017

@pnkfelix Are you still passionate about this?

@SirVer
Copy link

SirVer commented Mar 31, 2017

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.

@Mark-Simulacrum Mark-Simulacrum added C-feature-request Category: A feature request, i.e: not implemented / a PR. and removed I-wishlist labels Jul 22, 2017
@mlwilkerson
Copy link

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

@Mark-Simulacrum Mark-Simulacrum added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Oct 26, 2020
@Dylan-DPC Dylan-DPC added C-bug Category: This is a bug. and removed C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Nov 26, 2023
@fmease fmease added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. and removed A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. labels Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. P-low Low priority T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants