Skip to content

Consider deprecating integer_arithmetic in favor of arithmetic_side_effects #10200

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
c410-f3r opened this issue Jan 13, 2023 · 3 comments
Closed

Comments

@c410-f3r
Copy link
Contributor

c410-f3r commented Jan 13, 2023

Even though both lints can live separated, perhaps it might be worth considering deprecating integer_arithmetic in favor of arithmetic_side_effects.

  1. arithmetic_side_effects can detect arithmetic operations of any type, not only integers, which prevents more runtime overflows or panics.
  2. integer_arithmetic fires on trivial operations that in fact shouldn't be linted. On the other hand, arithmetic_side_effects is smarter.
    2.1. Adding or Subtracting 0¹: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=8c37520d26e4e59a85b419efc32c02c8
    2.2. Multiplying 1 or 0¹: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=5e95f95870e7cb6d6c3df178b98c2bd2
    2.3. CTFE: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=5144fb63c938b3796e65f91007eae9b0
    2.4. References: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=46c03f3b0851c376113f8ea161e92d08
    2.5. You can take a look at other spotted cases in https://github.com/rust-lang/rust-clippy/pulls?q=is%3Apr+author%3Ac410-f3r+is%3Aclosed.
¹: Used to reinforce the arguments. It its known that no_effect and similar exist.
@c410-f3r
Copy link
Contributor Author

c410-f3r commented Feb 7, 2023

Closing due to the lack of feedback.

@xFrednet
Copy link
Member

xFrednet commented May 5, 2023

Closing due to the lack of feedback.

Sorry for not providing feedback earlier. We sadly get too many issues, to review them all. If you want a discussion or feedback, I recommend pinging @rust-lang/clippy, asking on Zulip or even better in this case, adding the I-nominated later to add the issue as a topic in the next meeting. :)

@c410-f3r
Copy link
Contributor Author

c410-f3r commented May 9, 2023

Hey @xFrednet, I really appreciate your feedback and tips. Thank you very much!

bors added a commit that referenced this issue May 18, 2023
Rename `integer_arithmetic`

The lack of official feedback in #10200 made me give up on pursuing the matter but after yet another use-case that is not handled by `integer_arithmetic` (#10615), I think it is worth trying again.

---

changelog: Move/Deprecation: Rename `integer_arithmetic` to `arithmetic_side_effects`
[#10674](#10674)
<!-- changelog_checked -->
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

No branches or pull requests

2 participants