Skip to content

Bad suggestion: nonminimal_bool suggests replacing !cfg!(windows) with true. #1885

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

Closed
kennytm opened this issue Jul 12, 2017 · 6 comments · Fixed by #2029
Closed

Bad suggestion: nonminimal_bool suggests replacing !cfg!(windows) with true. #1885

kennytm opened this issue Jul 12, 2017 · 6 comments · Fixed by #2029

Comments

@kennytm
Copy link
Member

kennytm commented Jul 12, 2017

Test case:

#![warn(nonminimal_bool)]
fn main() {
    let _ = !cfg!(windows);
}

Clippy will warn:

warning: this boolean expression can be simplified
 --> src/main.rs:3:13
  |
3 |     let _ = !cfg!(windows);
  |             ^^^^^^^^^^^^^^ help: try `true`
  |
note: lint level defined here
 --> src/main.rs:1:9
  |
1 | #![warn(nonminimal_bool)]
  |         ^^^^^^^^^^^^^^^
  = help: for further information visit https://github.com/Manishearth/rust-clippy/wiki#nonminimal_bool

The suggestion (try `true`) is wrong because the code will lose portability. Instead Clippy should suggest cfg!(not(windows)) or just don't emit the warning.


$ cargo clippy --version
0.0.143
@shssoichiro
Copy link
Contributor

This is likely related to this issue: https://github.com/Manishearth/rust-clippy/issues/1877

Basically clippy is losing its mind on macros right now.

@kennytm
Copy link
Member Author

kennytm commented Jul 12, 2017

Okay let's see if this will be fixed after rust-lang/rust#43179 is shipped.

@oli-obk
Copy link
Contributor

oli-obk commented Jul 16, 2017

Nope not fixed by it... Gotta figure out what else changed :/

@oli-obk
Copy link
Contributor

oli-obk commented Jul 16, 2017

It's not just clippy. Even rustc goes crazy:

while cfg!(unix) {} // better use a `loop` here, because "infinite loop" and "reasons" xD

EDIT: nevermind, it always did that :/

@mcarton
Copy link
Member

mcarton commented Jul 16, 2017

Maybe we should suggest a fix in rustc then

@oli-obk
Copy link
Contributor

oli-obk commented Jul 16, 2017

rust-lang/rust#43268 done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants