integer_arithmetic and arithmetic_side_effects trigger on user-defined types

[tarcieri]

Summary

As of Rust 1.71, integer_arithmetic started warning for arithmetic operations on user-defined types. On Rust 1.70 and earlier it did not. As far as I can tell, on earlier versions of Rust the lint was restricted to arithmetic operations on core integer types.

This same problem applies to arithemtic_side_effects which is intended to replace integer_arithmetic.

User-defined types may implement core::ops traits in ways that always use checked and panic-free arithmetic internally. The checked crate is an example. Such an approach makes it possible to use traditional arithmetic operators (which are easier to read) while still performing checked arithmetic, and perhaps more importantly deliberately don't implement unchecked arithmetic, and thus prevent you from doing the wrong thing (much in the same way this lint is intended to do).

Warning for arithmetic operations on such types prevents them being from used as a strategy for mitigating this class of operations in a way that satisfies the lint.

Lint Name

integer_arithmetic

Reproducer

I tried this code:

#![warn(clippy::integer_arithmetic)]

use core::ops::Add;

pub struct MyNewtype(pub u64);

pub struct Error;

impl Add for MyNewtype {
    type Output = Result<Self, Error>;

    fn add(self, other: Self) -> Result<Self, Error> {
        self.0
            .checked_add(other.0)
            .map(Self)
            .ok_or(Error)
    }
}

pub fn example(a: MyNewtype, b: MyNewtype) -> Result<MyNewtype, Error> {
    a + b
}

I saw this happen:

warning: arithmetic operation that can potentially result in unexpected side-effects
  --> src/lib.rs:21:5
   |
21 |     a + b
   |     ^^^^^

I expected to see this happen:

Success

Version

rustc 1.71.0 (8ede3aae2 2023-07-12)
binary: rustc
commit-hash: 8ede3aae28fe6e4d52b38157d7bfe0d3bceef225
commit-date: 2023-07-12
host: aarch64-apple-darwin
release: 1.71.0
LLVM version: 16.0.5

Additional Labels

No response

[c410-f3r]

There needs to be some way to tell Clippy that a custom arithmetic implementation is safe. Perhaps something like #[has_significant_drop]? Maybe #[has_safe_arith]?

#![warn(clippy::arithmetic_side_effects)]

use core::ops::Add;

pub struct MyNewtype(pub u64);

pub struct Error;

#[clippy::has_safe_arith]
impl Add for MyNewtype {
    type Output = Result<Self, Error>;

    fn add(self, other: Self) -> Result<Self, Error> {
        self.0
            .checked_add(other.0)
            .map(Self)
            .ok_or(Error)
    }
}

pub fn example(a: MyNewtype, b: MyNewtype) -> Result<MyNewtype, Error> {
    a + b // OK
}

If the team is willing to accept such change, I can create a PR.

[tarcieri]

It might be nice if the attribute for "blessing" an arithmetic operation as safe ala clippy::has_safe_arith could potentially also apply to the newly added integer_division_remainder_used lint (#12451)

That lint is designed to find usages of division and remainder which may be problematic in a cryptographic context, but it's possible for user-defined types to have a "safe" Div impl which uses something constant-time internally and is therefore safe for the purposes of the lint.

Of course, if you don't want to collude the two either, that's fine, and such types can be annotated twice for the two different lints.

[taiki-e]

There is a configuration variable for this, but it would be useful if the implementer could control whether the warning is triggered or not, as suggested in #11220 (comment).

https://rust-lang.github.io/rust-clippy/master/index.html#/arithmetic_side_effects

• arithmetic-side-effects-allowed: Suppress checking of the passed type names in all types of operations.
  If a specific operation is desired, consider using arithmetic_side_effects_allowed_binary or arithmetic_side_effects_allowed_unary instead.