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.

Sign up for GitHub

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

Jump to bottom

WIP: Lint using bitwise operators for boolean expressions #3385

Closed
wants to merge 1 commit into from
Closed

WIP: Lint using bitwise operators for boolean expressions #3385

wants to merge 1 commit into from

Conversation

fhartwig
Copy link
Contributor

Fixes #1594

Still needs work (tests, documentation), but was hoping to get some feedback. One thing I'm not sure about is the lint level/type that this should be. I made it style for now, but that doesn't seem quite right, since I think this would mostly happen by accident. But I'm not sold on correctness either, since this will work just fine most of the time (any time when short-circuiting isn't required for correctness). Any thoughts?

@phansch phansch added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 31, 2018
@flip1995
Copy link
Member

LGTM, as you already stated, the documentation and testing is missing.

I would make this lint a correctness lint. Using & or | only makes sense, if you want to avoid the short-circuiting. Most of the time short-circuiting does not matter (from a logical point of view) or it is even required (if x != 0 && y / x > 1 {...}). In the former case it would be a perf lint, in the later case a correctness lint. I would still make this a correctness lint. The rare cases where you really need to avoid the short-circuiting you should just disable this lint (which needs to be done regardless whether it generates a warning or an error)

@flip1995 flip1995 added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Nov 19, 2018
@flip1995
Copy link
Member

Ping from triage @fhartwig: It looks like this PR hasn't received any updates in a while, so I'm closing it per our new guidelines. Thank you for your contributions and please feel free to re-open in the future.

@flip1995 flip1995 closed this Dec 14, 2018
@flip1995 flip1995 added S-inactive-closed Status: Closed due to inactivity and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Dec 14, 2018
@flip1995 flip1995 mentioned this pull request Dec 17, 2018
Merged
@arya-k arya-k mentioned this pull request Apr 25, 2021
Merged
bors added a commit that referenced this pull request May 17, 2021
@bors
Auto merge of #7133 - arya-k:master, r=llogiq
f2eeed3 
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 pull request May 17, 2021
@bors
Auto merge of #7133 - arya-k:master, r=llogiq
efc09bd 
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 pull request May 17, 2021
@bors
Auto merge of #7133 - arya-k:master, r=llogiq
a3223af 
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.
@jonboh
Copy link
Contributor

jonboh commented Oct 10, 2023

@rustbot label -S-inactive-closed

@rustbot rustbot removed the S-inactive-closed Status: Closed due to inactivity label Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Spot wrong usage of bitwise and operator
5 participants
@fhartwig @flip1995 @jonboh @phansch @rustbot