Auto merge of #9592 - c410-f3r:arith, r=Jarcho

[arithmetic-side-effects] Detect overflowing associated constants of integers

Triggers the negation of maximum unsigned integers using associated constants. Rustc already handles `-128i8` but doesn't handle `-i8::MAX`.

At the same time, allows stuff like `-1234`.

changelog: FP: [arithmetic-side-effects] Detect overflowing associated constants of integers

clippy_lints/src/operators/arithmetic_side_effects.rs

@@ -1,5 +1,9 @@
 use super::ARITHMETIC_SIDE_EFFECTS;
-use clippy_utils::{consts::constant_simple, diagnostics::span_lint};
+use clippy_utils::{
+    consts::{constant, constant_simple},
+    diagnostics::span_lint,
+    peel_hir_expr_refs,
+};
 use rustc_ast as ast;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir as hir;
@@ -38,24 +42,6 @@ impl ArithmeticSideEffects {
         }
     }
 
-    /// Assuming that `expr` is a literal integer, checks operators (+=, -=, *, /) in a
-    /// non-constant environment that won't overflow.
-    fn has_valid_op(op: &Spanned<hir::BinOpKind>, expr: &hir::Expr<'_>) -> bool {
-        if let hir::ExprKind::Lit(ref lit) = expr.kind &&
-            let ast::LitKind::Int(value, _) = lit.node
-        {
-            match (&op.node, value) {
-                (hir::BinOpKind::Div | hir::BinOpKind::Rem, 0) => false,
-                (hir::BinOpKind::Add | hir::BinOpKind::Sub, 0)
-                    | (hir::BinOpKind::Div | hir::BinOpKind::Rem, _)
-                    | (hir::BinOpKind::Mul, 0 | 1) => true,
-                _ => false,
-            }
-        } else {
-            false
-        }
-    }
-
     /// Checks if the given `expr` has any of the inner `allowed` elements.
     fn is_allowed_ty(&self, ty: Ty<'_>) -> bool {
         self.allowed
@@ -74,16 +60,15 @@ impl ArithmeticSideEffects {
         self.expr_span = Some(expr.span);
     }
 
-    /// If `expr` does not match any variant of `LiteralIntegerTy`, returns `None`.
+    /// If `expr` is not a literal integer like `1`, returns `None`.
-    fn literal_integer<'expr, 'tcx>(expr: &'expr hir::Expr<'tcx>) -> Option<LiteralIntegerTy<'expr, 'tcx>> {
+    fn literal_integer(expr: &hir::Expr<'_>) -> Option<u128> {
-        if matches!(expr.kind, hir::ExprKind::Lit(_)) {
+        if let hir::ExprKind::Lit(ref lit) = expr.kind && let ast::LitKind::Int(n, _) = lit.node {
-            return Some(LiteralIntegerTy::Value(expr));
+            Some(n)
-        }
-        if let hir::ExprKind::AddrOf(.., inn) = expr.kind && let hir::ExprKind::Lit(_) = inn.kind {
-            return Some(LiteralIntegerTy::Ref(inn));
         }
+        else {
             None
         }
+    }
 
     /// Manages when the lint should be triggered. Operations in constant environments, hard coded
     /// types, custom allowed types and non-constant operations that won't overflow are ignored.
@@ -117,10 +102,20 @@ impl ArithmeticSideEffects {
             return;
         }
         let has_valid_op = if Self::is_integral(lhs_ty) && Self::is_integral(rhs_ty) {
-            match (Self::literal_integer(lhs), Self::literal_integer(rhs)) {
+            let (actual_lhs, lhs_ref_counter) = peel_hir_expr_refs(lhs);
-                (None, Some(lit_int_ty)) | (Some(lit_int_ty), None) => Self::has_valid_op(op, lit_int_ty.into()),
+            let (actual_rhs, rhs_ref_counter) = peel_hir_expr_refs(rhs);
-                (Some(LiteralIntegerTy::Value(_)), Some(LiteralIntegerTy::Value(_))) => true,
+            match (Self::literal_integer(actual_lhs), Self::literal_integer(actual_rhs)) {
-                (None, None) | (Some(_), Some(_)) => false,
+                (None, None) => false,
+                (None, Some(n)) | (Some(n), None) => match (&op.node, n) {
+                    (hir::BinOpKind::Div | hir::BinOpKind::Rem, 0) => false,
+                    (hir::BinOpKind::Add | hir::BinOpKind::Sub, 0)
+                    | (hir::BinOpKind::Div | hir::BinOpKind::Rem, _)
+                    | (hir::BinOpKind::Mul, 0 | 1) => true,
+                    _ => false,
+                },
+                (Some(_), Some(_)) => {
+                    matches!((lhs_ref_counter, rhs_ref_counter), (0, 0))
+                },
             }
         } else {
             false
@@ -129,21 +124,45 @@ impl ArithmeticSideEffects {
             self.issue_lint(cx, expr);
         }
     }
+
+    fn manage_unary_ops<'tcx>(
+        &mut self,
+        cx: &LateContext<'tcx>,
+        expr: &hir::Expr<'tcx>,
+        un_expr: &hir::Expr<'tcx>,
+        un_op: hir::UnOp,
+    ) {
+        let hir::UnOp::Neg = un_op else { return; };
+        if constant(cx, cx.typeck_results(), un_expr).is_some() {
+            return;
+        }
+        let ty = cx.typeck_results().expr_ty(expr).peel_refs();
+        if self.is_allowed_ty(ty) {
+            return;
+        }
+        let actual_un_expr = peel_hir_expr_refs(un_expr).0;
+        if Self::literal_integer(actual_un_expr).is_some() {
+            return;
+        }
+        self.issue_lint(cx, expr);
+    }
+
+    fn should_skip_expr(&mut self, expr: &hir::Expr<'_>) -> bool {
+        self.expr_span.is_some() || self.const_span.map_or(false, |sp| sp.contains(expr.span))
+    }
 }
 
 impl<'tcx> LateLintPass<'tcx> for ArithmeticSideEffects {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &hir::Expr<'tcx>) {
-        if self.expr_span.is_some() || self.const_span.map_or(false, |sp| sp.contains(expr.span)) {
+        if self.should_skip_expr(expr) {
             return;
         }
         match &expr.kind {
-            hir::ExprKind::Binary(op, lhs, rhs) | hir::ExprKind::AssignOp(op, lhs, rhs) => {
+            hir::ExprKind::AssignOp(op, lhs, rhs) | hir::ExprKind::Binary(op, lhs, rhs) => {
                 self.manage_bin_ops(cx, expr, op, lhs, rhs);
             },
-            hir::ExprKind::Unary(hir::UnOp::Neg, _) => {
+            hir::ExprKind::Unary(un_op, un_expr) => {
-                if constant_simple(cx, cx.typeck_results(), expr).is_none() {
+                self.manage_unary_ops(cx, expr, un_expr, *un_op);
-                    self.issue_lint(cx, expr);
-                }
             },
             _ => {},
         }
@@ -177,22 +196,3 @@ impl<'tcx> LateLintPass<'tcx> for ArithmeticSideEffects {
         }
     }
 }
-
-/// Tells if an expression is a integer declared by value or by reference.
-///
-/// If `LiteralIntegerTy::Ref`, then the contained value will be `hir::ExprKind::Lit` rather
-/// than `hirExprKind::Addr`.
-enum LiteralIntegerTy<'expr, 'tcx> {
-    /// For example, `&199`
-    Ref(&'expr hir::Expr<'tcx>),
-    /// For example, `1` or `i32::MAX`
-    Value(&'expr hir::Expr<'tcx>),
-}
-
-impl<'expr, 'tcx> From<LiteralIntegerTy<'expr, 'tcx>> for &'expr hir::Expr<'tcx> {
-    fn from(from: LiteralIntegerTy<'expr, 'tcx>) -> Self {
-        match from {
-            LiteralIntegerTy::Ref(elem) | LiteralIntegerTy::Value(elem) => elem,
-        }
-    }
-}

tests/ui-toml/arithmetic_side_effects_allowed/arithmetic_side_effects_allowed.rs

@@ -1,6 +1,6 @@
 #![warn(clippy::arithmetic_side_effects)]
 
-use core::ops::Add;
+use core::ops::{Add, Neg};
 
 #[derive(Clone, Copy)]
 struct Point {
@@ -16,9 +16,18 @@ impl Add for Point {
     }
 }
 
+impl Neg for Point {
+    type Output = Self;
+
+    fn neg(self) -> Self::Output {
+        todo!()
+    }
+}
+
 fn main() {
     let _ = Point { x: 1, y: 0 } + Point { x: 2, y: 3 };
 
     let point: Point = Point { x: 1, y: 0 };
     let _ = point + point;
+    let _ = -point;
 }

tests/ui/arithmetic_side_effects.rs

@@ -150,8 +150,12 @@ pub fn non_overflowing_ops_or_ops_already_handled_by_the_compiler_should_not_tri
     _n = 23 + 85;
 
     // Unary
-    _n = -1;
+    _n = -2147483647;
-    _n = -(-1);
+    _n = -i32::MAX;
+    _n = -i32::MIN;
+    _n = -&2147483647;
+    _n = -&i32::MAX;
+    _n = -&i32::MIN;
 }
 
 pub fn runtime_ops() {

tests/ui/arithmetic_side_effects.stderr

@@ -19,331 +19,331 @@ LL |     let _ = inferred_string + "";
    |             ^^^^^^^^^^^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:161:5
+  --> $DIR/arithmetic_side_effects.rs:165:5
    |
 LL |     _n += 1;
    |     ^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:162:5
+  --> $DIR/arithmetic_side_effects.rs:166:5
    |
 LL |     _n += &1;
    |     ^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:163:5
+  --> $DIR/arithmetic_side_effects.rs:167:5
    |
 LL |     _n -= 1;
    |     ^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:164:5
+  --> $DIR/arithmetic_side_effects.rs:168:5
    |
 LL |     _n -= &1;
    |     ^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:165:5
+  --> $DIR/arithmetic_side_effects.rs:169:5
    |
 LL |     _n /= 0;
    |     ^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:166:5
+  --> $DIR/arithmetic_side_effects.rs:170:5
    |
 LL |     _n /= &0;
    |     ^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:167:5
+  --> $DIR/arithmetic_side_effects.rs:171:5
    |
 LL |     _n %= 0;
    |     ^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:168:5
+  --> $DIR/arithmetic_side_effects.rs:172:5
    |
 LL |     _n %= &0;
    |     ^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:169:5
+  --> $DIR/arithmetic_side_effects.rs:173:5
    |
 LL |     _n *= 2;
    |     ^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:170:5
+  --> $DIR/arithmetic_side_effects.rs:174:5
    |
 LL |     _n *= &2;
    |     ^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:173:10
+  --> $DIR/arithmetic_side_effects.rs:177:10
    |
 LL |     _n = _n + 1;
    |          ^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:174:10
+  --> $DIR/arithmetic_side_effects.rs:178:10
    |
 LL |     _n = _n + &1;
    |          ^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:175:10
+  --> $DIR/arithmetic_side_effects.rs:179:10
    |
 LL |     _n = 1 + _n;
    |          ^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:176:10
+  --> $DIR/arithmetic_side_effects.rs:180:10
    |
 LL |     _n = &1 + _n;
    |          ^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:177:10
+  --> $DIR/arithmetic_side_effects.rs:181:10
    |
 LL |     _n = _n - 1;
    |          ^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:178:10
+  --> $DIR/arithmetic_side_effects.rs:182:10
    |
 LL |     _n = _n - &1;
    |          ^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:179:10
+  --> $DIR/arithmetic_side_effects.rs:183:10
    |
 LL |     _n = 1 - _n;
    |          ^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:180:10
+  --> $DIR/arithmetic_side_effects.rs:184:10
    |
 LL |     _n = &1 - _n;
    |          ^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:181:10
+  --> $DIR/arithmetic_side_effects.rs:185:10
    |
 LL |     _n = _n / 0;
    |          ^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:182:10
+  --> $DIR/arithmetic_side_effects.rs:186:10
    |
 LL |     _n = _n / &0;
    |          ^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:183:10
+  --> $DIR/arithmetic_side_effects.rs:187:10
    |
 LL |     _n = _n % 0;
    |          ^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:184:10
+  --> $DIR/arithmetic_side_effects.rs:188:10
    |
 LL |     _n = _n % &0;
    |          ^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:185:10
+  --> $DIR/arithmetic_side_effects.rs:189:10
    |
 LL |     _n = _n * 2;
    |          ^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:186:10
+  --> $DIR/arithmetic_side_effects.rs:190:10
    |
 LL |     _n = _n * &2;
    |          ^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:187:10
+  --> $DIR/arithmetic_side_effects.rs:191:10
    |
 LL |     _n = 2 * _n;
    |          ^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:188:10
+  --> $DIR/arithmetic_side_effects.rs:192:10
    |
 LL |     _n = &2 * _n;
    |          ^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:189:10
+  --> $DIR/arithmetic_side_effects.rs:193:10
    |
 LL |     _n = 23 + &85;
    |          ^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:190:10
+  --> $DIR/arithmetic_side_effects.rs:194:10
    |
 LL |     _n = &23 + 85;
    |          ^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:191:10
+  --> $DIR/arithmetic_side_effects.rs:195:10
    |
 LL |     _n = &23 + &85;
    |          ^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:194:13
+  --> $DIR/arithmetic_side_effects.rs:198:13
    |
 LL |     let _ = Custom + 0;
    |             ^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:195:13
+  --> $DIR/arithmetic_side_effects.rs:199:13
    |
 LL |     let _ = Custom + 1;
    |             ^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:196:13
+  --> $DIR/arithmetic_side_effects.rs:200:13
    |
 LL |     let _ = Custom + 2;
    |             ^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:197:13
+  --> $DIR/arithmetic_side_effects.rs:201:13
    |
 LL |     let _ = Custom + 0.0;
    |             ^^^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:198:13
+  --> $DIR/arithmetic_side_effects.rs:202:13
    |
 LL |     let _ = Custom + 1.0;
    |             ^^^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:199:13
+  --> $DIR/arithmetic_side_effects.rs:203:13
    |
 LL |     let _ = Custom + 2.0;
    |             ^^^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:200:13
+  --> $DIR/arithmetic_side_effects.rs:204:13
    |
 LL |     let _ = Custom - 0;
    |             ^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:201:13
+  --> $DIR/arithmetic_side_effects.rs:205:13
    |
 LL |     let _ = Custom - 1;
    |             ^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:202:13
+  --> $DIR/arithmetic_side_effects.rs:206:13
    |
 LL |     let _ = Custom - 2;
    |             ^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:203:13
+  --> $DIR/arithmetic_side_effects.rs:207:13
    |
 LL |     let _ = Custom - 0.0;
    |             ^^^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:204:13
+  --> $DIR/arithmetic_side_effects.rs:208:13
    |
 LL |     let _ = Custom - 1.0;
    |             ^^^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:205:13
+  --> $DIR/arithmetic_side_effects.rs:209:13
    |
 LL |     let _ = Custom - 2.0;
    |             ^^^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:206:13
+  --> $DIR/arithmetic_side_effects.rs:210:13
    |
 LL |     let _ = Custom / 0;
    |             ^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:207:13
+  --> $DIR/arithmetic_side_effects.rs:211:13
    |
 LL |     let _ = Custom / 1;
    |             ^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:208:13
+  --> $DIR/arithmetic_side_effects.rs:212:13
    |
 LL |     let _ = Custom / 2;
    |             ^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:209:13
+  --> $DIR/arithmetic_side_effects.rs:213:13
    |
 LL |     let _ = Custom / 0.0;
    |             ^^^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:210:13
+  --> $DIR/arithmetic_side_effects.rs:214:13
    |
 LL |     let _ = Custom / 1.0;
    |             ^^^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:211:13
+  --> $DIR/arithmetic_side_effects.rs:215:13
    |
 LL |     let _ = Custom / 2.0;
    |             ^^^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:212:13
+  --> $DIR/arithmetic_side_effects.rs:216:13
    |
 LL |     let _ = Custom * 0;
    |             ^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:213:13
+  --> $DIR/arithmetic_side_effects.rs:217:13
    |
 LL |     let _ = Custom * 1;
    |             ^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:214:13
+  --> $DIR/arithmetic_side_effects.rs:218:13
    |
 LL |     let _ = Custom * 2;
    |             ^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:215:13
+  --> $DIR/arithmetic_side_effects.rs:219:13
    |
 LL |     let _ = Custom * 0.0;
    |             ^^^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:216:13
+  --> $DIR/arithmetic_side_effects.rs:220:13
    |
 LL |     let _ = Custom * 1.0;
    |             ^^^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:217:13
+  --> $DIR/arithmetic_side_effects.rs:221:13
    |
 LL |     let _ = Custom * 2.0;
    |             ^^^^^^^^^^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:220:10
+  --> $DIR/arithmetic_side_effects.rs:224:10
    |
 LL |     _n = -_n;
    |          ^^^
 
 error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:221:10
+  --> $DIR/arithmetic_side_effects.rs:225:10
    |
 LL |     _n = -&_n;
    |          ^^^^