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

False positive with match_on_same_arms and macros #1390

Closed
jugglerchris opened this issue Dec 18, 2016 · 5 comments · Fixed by #6843
Closed

False positive with match_on_same_arms and macros #1390

jugglerchris opened this issue Dec 18, 2016 · 5 comments · Fixed by #6843
Labels
C-an-interesting-project Category: Interesting projects, that usually are more involved design/code wise. C-bug Category: Clippy is not doing the correct thing E-hard Call for participation: This a hard problem and requires more experience or effort to work on I-false-positive Issue: The lint was triggered on code it shouldn't have T-middle Type: Probably requires verifiying types

Comments

@jugglerchris
Copy link
Contributor

jugglerchris commented Dec 18, 2016

Hi,

I have a trace macro which normally expands to nothing, but can be turned on by a Cargo feature. I get match_same_arms warning on a case like this:

macro_rules! trace {
    ($e:expr) => ()
}

pub enum Foo { A, B, C }
use Foo::*;

pub fn test(x: Foo) {
    match x {
        A => { println!("something"); },
        B => { trace!("Got B"); },
        _ => {},
    }
}

which gives the following warning:

warning: this `match` has identical arm bodies, #[warn(match_same_arms)] on by default
  --> src/lib.rs:15:14
   |
15 |         _ => {},
   |              ^^
   |
note: same as this
  --> src/lib.rs:14:14
   |
14 |         B => { trace!("Got B"); },
   |              ^^^^^^^^^^^^^^^^^^^^
note: `B` has the same arm body as the `_` wildcard, consider removing it`
  --> src/lib.rs:14:14
   |
14 |         B => { trace!("Got B"); },
   |              ^^^^^^^^^^^^^^^^^^^^
   = help: for further information visit https://github.com/Manishearth/rust-clippy/wiki#match_same_arms

In some sense they expand to the same, but they're not the same to me the programmer; when I turn on a feature the tracing becomes more useful.

Playground link: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=929ad50b7c6d82e2df6bf3a8cbd7a944

@oli-obk
Copy link
Contributor

oli-obk commented Dec 19, 2016

yea... we do have some issues with cfg... As a hack, you could expand to calling nop(), which is defined as #[inline(always)] fn nop() {}.

@oli-obk oli-obk added C-an-interesting-project Category: Interesting projects, that usually are more involved design/code wise. E-hard Call for participation: This a hard problem and requires more experience or effort to work on C-bug Category: Clippy is not doing the correct thing T-middle Type: Probably requires verifiying types labels Dec 19, 2016
@jugglerchris
Copy link
Contributor Author

Thanks - that seems like a perfectly good workaround to me.

@phansch

This comment has been minimized.

@phansch phansch closed this as completed Dec 18, 2020
@taiki-e

This comment has been minimized.

@phansch

This comment has been minimized.

@phansch phansch reopened this Dec 20, 2020
@phansch phansch added the I-false-positive Issue: The lint was triggered on code it shouldn't have label Dec 21, 2020
@bors bors closed this as completed in 7be3a32 Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-an-interesting-project Category: Interesting projects, that usually are more involved design/code wise. C-bug Category: Clippy is not doing the correct thing E-hard Call for participation: This a hard problem and requires more experience or effort to work on I-false-positive Issue: The lint was triggered on code it shouldn't have T-middle Type: Probably requires verifiying types
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants