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

Lint double negation !count != 0 #5794

Closed
Tracked by #79
idubrov opened this issue Jul 13, 2020 · 15 comments · Fixed by #12248
Closed
Tracked by #79

Lint double negation !count != 0 #5794

idubrov opened this issue Jul 13, 2020 · 15 comments · Fixed by #12248
Assignees
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy

Comments

@idubrov
Copy link

idubrov commented Jul 13, 2020

What it does

I think, clippy should lint the following code:

if !count != 0 {
}

My brain melts trying to read this expression. It's not that difficult to read, just something immediately throwing me off.

Drawbacks

None.

Example

if !count != 0 {
}

Could be written as:

if count == 0 {
}
@idubrov idubrov added the A-lint Area: New lints label Jul 13, 2020
@ghost
Copy link

ghost commented Jul 14, 2020

This should probably be an enhancement to nonminimal_bool

@flip1995 flip1995 added good-first-issue These issues are a good way to get started with Clippy C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages labels Jul 14, 2020
@sim031999
Copy link

Hello, I want to work on this ! But I'm new to open source and would need help getting started

@flip1995
Copy link
Member

Hey @sim031999, welcome to Clippy! We have documentation in the doc directory of this repository on how Clippy works. For this issue the adding_lints documentation (especially the first three sections, Setup -> Testing) explains best how to get started with hacking on Clippy.

https://github.com/rust-lang/rust-clippy/blob/master/doc/common_tools_writing_lints.md This document explains some reoccurring patterns you'll find in Clippy with code examples.

If you have any questions, feel free to ask here, in discord, or open a WIP PR.

@sim031999
Copy link

Hi @flip1995, I have figured out that the following file ( after line 115) needs changes: https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/booleans.rs#L115. But the quine_mc_cluskey technique which is used in it, is not there for !=, can you suggest how do I proceed further?

@flip1995
Copy link
Member

You can't use the quine_mc_clusky check for this. Just implement a check based on the AST/HIR structure after this line:

NonminimalBoolVisitor { cx }.visit_body(body)

Maybe not in the check_fn, but in the check_expr method.

@ghost
Copy link

ghost commented Jul 17, 2020

I think I was mistaken about extending nonminimal_bool because I mistook !count != 0 for !(count != 0). I don't think any code in nonminimal_bool will help with this.

Maybe a better fit would be to add this to bit_mask module since there is a lot of bit comparison logic in there.

@flip1995
Copy link
Member

I think the issue meant !(count != 0), which I get from the example.

Replacing !count != 0 with count == 0 would change the semantics, whereas replacing !(count != 0) with count == 0 wouldn't.

@idubrov can you clarify here?

@idubrov
Copy link
Author

idubrov commented Jul 17, 2020

Right, in my case it was an integer variable, so I made an assumption that expression was parsed as !(count != 0) (and this definitely was implied meaning in our code). However, now I'm totally confused:

fn main() {
    let count = 0;
    // Original expression
    if !count != 0 {
        eprintln!("hello!");
    }
    // This is what I thought it means to compiler
    if !(count != 0) {
        eprintln!("hello!");
    }
    // This compiles? Why???
    if (!count) != 0 {
        eprintln!("hello!");
    }
    // This does not compile (makes sens; count is not a `bool`)
    if (!count) && true {
        eprintln!("hello!");
    }
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=a141aa1438ec32ef629a9a68638b8739

@flip1995
Copy link
Member

!count returns the bitwise inverse of count (Ones' complement).

I just saw that !(x != y) already gets linted by nonminimal_bool. Playground

I think there is already an issue open for suspicious uses of the Not/! operations in integer comparisons, but I can't find it.

@idubrov
Copy link
Author

idubrov commented Jul 17, 2020

Ah, got it! The fact that ! works for bitwise operators was a surprise (though, know I remember I knew that before...).

@flip1995
Copy link
Member

flip1995 commented Jul 17, 2020

Anyway, this is a new lint. The behavior of the lint would be: Lint when there is !var inside a comparison (!=/==) of integral types, but not if it is on a literal (i.e. count != !0 is fine). The bad_mask module should be good for this lint.

@flip1995 flip1995 removed the C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages label Jul 17, 2020
@idubrov
Copy link
Author

idubrov commented Jul 17, 2020

Yeah; now that I understand how it works, it actually turned out to be a logic error in our case!

Branch was intended to do count == 0, but effectively, it was doing if count != 0xffff_ffff_ffff_ffff_usize, which didn't make sense.

@DevAccentor
Copy link
Contributor

@rustbot claim

@MorrisLaw
Copy link

@rustbot claim

@GuillaumeGomez
Copy link
Member

Opened #12248.

@bors bors closed this as completed in 75f57cf Feb 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
7 participants