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

[arithmetic_side_effects] Division by potentially-zero can panic even with Wrapping #11393

Closed
guilliamxavier opened this issue Aug 24, 2023 · 1 comment · Fixed by #11395
Closed
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing I-false-negative Issue: The lint should have been triggered on code, but wasn't

Comments

@guilliamxavier
Copy link

guilliamxavier commented Aug 24, 2023

Summary

Division (/ and %, i.e. div() and rem()) of any core::num::Wrapping by Wrapping(0) (of the same type) panics, but the arithmetic_side_effects lint currently doesn't flag them (as opposed to the same operations with primitive integers).

(Note: kind of the opposite of #11392)

Lint Name

arithmetic_side_effects

Reproducer

I tried this code:

#![warn(clippy::arithmetic_side_effects)]

use std::num::Wrapping;

fn example_div(x: Wrapping<i32>, maybe_zero: Wrapping<i32>) -> Wrapping<i32> {
    x / maybe_zero
}

fn example_rem(x: Wrapping<i32>, maybe_zero: Wrapping<i32>) -> Wrapping<i32> {
    x % maybe_zero
}

fn main() {
    use std::panic;

    let [x, maybe_zero] = [1, 0].map(Wrapping);

    println!("{x} / {maybe_zero} is {:?}", panic::catch_unwind(|| example_div(x, maybe_zero)));
    println!("{x} % {maybe_zero} is {:?}", panic::catch_unwind(|| example_rem(x, maybe_zero)));
}

(note: compiles but panics with "attempt to divide by zero" and "attempt to calculate the remainder with a divisor of zero")

I expected to see this happen:

warning: arithmetic operation that can potentially result in unexpected side-effects
 --> src/main.rs:6:5
  |
6 |     x / maybe_zero
  |     ^^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#arithmetic_side_effects
note: the lint level is defined here
 --> src/main.rs:1:9
  |
1 | #![warn(clippy::arithmetic_side_effects)]
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: arithmetic operation that can potentially result in unexpected side-effects
  --> src/main.rs:10:5
   |
10 |     x % maybe_zero
   |     ^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#arithmetic_side_effects

Instead, this happened:

No warning

Version

Clippy 0.1.73 (2023-08-23 249595b) on the Rust Playground,
which I guess corresponds to rustc 1.74.0-nightly (2023-08-23 249595b7523fc07a99c1)
@guilliamxavier guilliamxavier added C-bug Category: Clippy is not doing the correct thing I-false-negative Issue: The lint should have been triggered on code, but wasn't labels Aug 24, 2023
@c410-f3r
Copy link
Contributor

@rustbot claim

bors added a commit that referenced this issue Aug 25, 2023
[`arithmetic_side_effects`] Fix #11393

Fix #11393

```
changelog: [`arithmetic_side_effects`]: Detect division by zero for `Wrapping` and `Saturating`
```
@bors bors closed this as completed in 2faa43c Aug 25, 2023
c410-f3r added a commit to c410-f3r/rust-clippy that referenced this issue Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-negative Issue: The lint should have been triggered on code, but wasn't
Projects
None yet
2 participants