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 in cmp_owned lint #4909

Closed
tesuji opened this issue Dec 17, 2019 · 3 comments
Closed

False positive in cmp_owned lint #4909

tesuji opened this issue Dec 17, 2019 · 3 comments
Labels
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

Comments

@tesuji
Copy link
Contributor

tesuji commented Dec 17, 2019

Consider this (crappy code)

macro_rules! foo {
    ($($i:ident: $ty:ty, $def:expr);+ $(;)*) => {
        pub fn bar(key: &str) -> bool {
            match key {
                $(
                    stringify!($i) => {
                        let res = <$ty>::default() == $def;
                        let _i: $ty = $def;
                        return res;
                    }
                )+
                _ => false,
            }
        }
    };
}

foo! {
    max_width: usize, 100;
    required_version: String, env!("HOME").to_string();
}

Clippy outputs this warnings:

warning: this creates an owned instance just for comparison
  --> src/lib.rs:20:31
   |
20 |     required_version: String, env!("HOME").to_string();
   |                               ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `env!("HOME")`
   |
   = note: `#[warn(clippy::cmp_owned)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cmp_owned

Which is wrong.


cargo clippy --version: clippy 0.0.212 (69f99e7 2019-12-14)

@ghost
Copy link

ghost commented Dec 19, 2019

This is incorrect because the instance is being assigned in this line let _i: $ty = $def;, right?

@tesuji
Copy link
Contributor Author

tesuji commented Dec 19, 2019

Yeah.

@flip1995 flip1995 added 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 labels Dec 21, 2019
@phansch phansch added the I-false-positive Issue: The lint was triggered on code it shouldn't have label Dec 21, 2020
@tesuji
Copy link
Contributor Author

tesuji commented Jul 15, 2024

The example code in OP was gotten from rustfmt create_config!. But since then, there are no real practical usage
of the same code pattern. Closing as not planned.

@tesuji tesuji closed this as completed Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Projects
None yet
Development

No branches or pull requests

3 participants