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

Spot wrong usage of bitwise and operator #1594

Closed
leonardo-m opened this issue Mar 3, 2017 · 8 comments · Fixed by #7133
Closed

Spot wrong usage of bitwise and operator #1594

leonardo-m opened this issue Mar 3, 2017 · 8 comments · Fixed by #7133
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy L-correctness Lint: Belongs in the correctness lint group T-middle Type: Probably requires verifiying types

Comments

@leonardo-m
Copy link

I'd like Clippy to warn in this expression suggesting that this is probably an error, and that the programmer perhaps meant to use "&&" instead:

fn foo2(x1: i32, x2: i32) {
    if (x1 < x2) & (x1 != 0) {}
}
@mcarton
Copy link
Member

mcarton commented Mar 3, 2017

/me disappointed
/me expected that to be a type-error in rustc

@mcarton mcarton added L-correctness Lint: Belongs in the correctness lint group good-first-issue These issues are a good way to get started with Clippy A-lint Area: New lints T-middle Type: Probably requires verifiying types labels Mar 3, 2017
@leonardo-m
Copy link
Author

In my opinion there's a small number of Clippy lints that should be Rustc errors (or warnings).

@mcarton
Copy link
Member

mcarton commented Mar 3, 2017

What I meant is I did not expect & to even exist on booleans.

@oli-obk
Copy link
Contributor

oli-obk commented Mar 4, 2017

Actually there are valid use cases for this, because the regular bool ops short circuit but in that case the lint can be disabled, since these cases are rare.

@clarfonthey
Copy link

clarfonthey commented Mar 4, 2017

Perhaps this could only apply to constant operations and functions that are known to have no side effects? That way those cases don't require disabling the lint.

Or better, we split into two lints, one that does just the ones that are known to have no side effects, and one that covers the ones that might have side effects. That way people can disable the ones for side effects while still having the benefit of checking cases like these.

@mcarton
Copy link
Member

mcarton commented Mar 4, 2017

@oli-obk I know but this is rare enough that I would have expected to have to bind the expressions (and of course add a comment) rather that be allowed to use & on booleans. Oh well, it's too late anyway.

@mcarton mcarton closed this as completed Mar 4, 2017
@mcarton
Copy link
Member

mcarton commented Mar 4, 2017

Wrong button (morning 😅).

@mcarton mcarton reopened this Mar 4, 2017
@leonardo-m
Copy link
Author

Wrong button (morning

It's a common mistake, so I've sent a "bug report" to GitHub about the design of those two buttons.

bors added a commit that referenced this issue May 17, 2021
Add `needless_bitwise_bool` lint

fixes #6827
fixes #1594

changelog: Add ``[`needless_bitwise_bool`]`` lint

Creates a new `bitwise_bool` lint to convert `x & y` to `x && y` when both `x` and `y` are booleans. I also had to adjust thh `needless_bool` lint slightly, and fix a couple failing dogfood tests. I made it a correctness lint as per flip1995's comment [here](#3385 (comment)), from a previous WIP attempt at this lint.
bors added a commit that referenced this issue May 17, 2021
Add `needless_bitwise_bool` lint

fixes #6827
fixes #1594

changelog: Add ``[`needless_bitwise_bool`]`` lint

Creates a new `bitwise_bool` lint to convert `x & y` to `x && y` when both `x` and `y` are booleans. I also had to adjust thh `needless_bool` lint slightly, and fix a couple failing dogfood tests. I made it a correctness lint as per flip1995's comment [here](#3385 (comment)), from a previous WIP attempt at this lint.
@bors bors closed this as completed in a3223af May 17, 2021
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 L-correctness Lint: Belongs in the correctness lint group T-middle Type: Probably requires verifiying types
Projects
None yet
5 participants