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

Limit the `[identity_op]` lint to integral operands. #8183

Merged
merged 3 commits into from
Dec 28, 2021

Conversation

alex-ozdemir
Copy link
Contributor

changelog: limit [`identity_op`] to integral operands

In the [`identity_op`] lint, if the operands are non-integers, then the lint is likely
wrong.

If operands are being applied to non-integers, then the lint is likely
wrong.
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @camsteffen (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 28, 2021
@camsteffen
Copy link
Contributor

Do you have an example false positive that this fixes?

@alex-ozdemir
Copy link
Contributor Author

Ah yes. good_lp overloads the shift operators to construct bounds in an ILP. See, for example this overload.

I encountered the false positive here.

@camsteffen
Copy link
Contributor

I see. Can you add a test for that?

Also I think you should use peel_refs().is_integral() since there can be + &0.

@alex-ozdemir
Copy link
Contributor Author

Good suggestions.

First commit avoids ignoring refs.

Second commit actually catches x << &0, by peeling refs around constants.

@camsteffen
Copy link
Contributor

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Dec 28, 2021

📌 Commit ee6d5c5 has been approved by camsteffen

@bors
Copy link
Contributor

bors commented Dec 28, 2021

⌛ Testing commit ee6d5c5 with merge 16ef044...

@bors
Copy link
Contributor

bors commented Dec 28, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: camsteffen
Pushing 16ef044 to master...

@bors bors merged commit 16ef044 into rust-lang:master Dec 28, 2021
@alex-ozdemir alex-ozdemir deleted the limit-ident branch December 29, 2021 04:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants