]> git.proxmox.com Git - rustc.git/blobdiff - src/tools/clippy/clippy_lints/src/operators/arithmetic_side_effects.rs
New upstream version 1.67.1+dfsg1
[rustc.git] / src / tools / clippy / clippy_lints / src / operators / arithmetic_side_effects.rs
index 83b69fbb3116466f3799a5f9b3d81bb3bb8ae000..20b82d81a2aeb64e57d11994ceb3755a96e4cde4 100644 (file)
@@ -1,10 +1,9 @@
-#![allow(
-    // False positive
-    clippy::match_same_arms
-)]
-
 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;
@@ -14,11 +13,12 @@ use rustc_session::impl_lint_pass;
 use rustc_span::source_map::{Span, Spanned};
 
 const HARD_CODED_ALLOWED: &[&str] = &[
+    "&str",
     "f32",
     "f64",
     "std::num::Saturating",
-    "std::string::String",
     "std::num::Wrapping",
+    "std::string::String",
 ];
 
 #[derive(Debug)]
@@ -42,60 +42,43 @@ impl ArithmeticSideEffects {
         }
     }
 
-    /// Checks assign operators (+=, -=, *=, /=) of integers in a non-constant environment that
-    /// won't overflow.
-    fn has_valid_assign_op(op: &Spanned<hir::BinOpKind>, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool {
-        if !Self::is_literal_integer(rhs, rhs_refs) {
-            return false;
-        }
-        if let hir::BinOpKind::Div | hir::BinOpKind::Mul = op.node
-            && let hir::ExprKind::Lit(ref lit) = rhs.kind
-            && let ast::LitKind::Int(1, _) = lit.node
-        {
-            return true;
-        }
-        false
-    }
-
-    /// Checks "raw" binary operators (+, -, *, /) of integers in a non-constant environment
-    /// already handled by the CTFE.
-    fn has_valid_bin_op(lhs: &hir::Expr<'_>, lhs_refs: Ty<'_>, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool {
-        Self::is_literal_integer(lhs, lhs_refs) && Self::is_literal_integer(rhs, rhs_refs)
-    }
-
     /// Checks if the given `expr` has any of the inner `allowed` elements.
-    fn is_allowed_ty(&self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
-        self.allowed.contains(
-            cx.typeck_results()
-                .expr_ty(expr)
-                .to_string()
-                .split('<')
-                .next()
-                .unwrap_or_default(),
-        )
+    fn is_allowed_ty(&self, ty: Ty<'_>) -> bool {
+        self.allowed
+            .contains(ty.to_string().split('<').next().unwrap_or_default())
     }
 
-    /// Explicit integers like `1` or `i32::MAX`. Does not take into consideration references.
-    fn is_literal_integer(expr: &hir::Expr<'_>, expr_refs: Ty<'_>) -> bool {
-        let is_integral = expr_refs.is_integral();
-        let is_literal = matches!(expr.kind, hir::ExprKind::Lit(_));
-        is_integral && is_literal
+    // For example, 8i32 or &i64::MAX.
+    fn is_integral(ty: Ty<'_>) -> bool {
+        ty.peel_refs().is_integral()
     }
 
+    // Common entry-point to avoid code duplication.
     fn issue_lint(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) {
-        span_lint(cx, ARITHMETIC_SIDE_EFFECTS, expr.span, "arithmetic detected");
+        let msg = "arithmetic operation that can potentially result in unexpected side-effects";
+        span_lint(cx, ARITHMETIC_SIDE_EFFECTS, expr.span, msg);
         self.expr_span = Some(expr.span);
     }
 
+    /// If `expr` is not a literal integer like `1`, returns `None`.
+    fn literal_integer(expr: &hir::Expr<'_>) -> Option<u128> {
+        if let hir::ExprKind::Lit(ref lit) = expr.kind && let ast::LitKind::Int(n, _) = lit.node {
+            Some(n)
+        }
+        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.
-    fn manage_bin_ops(
+    fn manage_bin_ops<'tcx>(
         &mut self,
-        cx: &LateContext<'_>,
-        expr: &hir::Expr<'_>,
+        cx: &LateContext<'tcx>,
+        expr: &hir::Expr<'tcx>,
         op: &Spanned<hir::BinOpKind>,
-        lhs: &hir::Expr<'_>,
-        rhs: &hir::Expr<'_>,
+        lhs: &hir::Expr<'tcx>,
+        rhs: &hir::Expr<'tcx>,
     ) {
         if constant_simple(cx, cx.typeck_results(), expr).is_some() {
             return;
@@ -112,32 +95,74 @@ impl ArithmeticSideEffects {
         ) {
             return;
         };
-        if self.is_allowed_ty(cx, lhs) || self.is_allowed_ty(cx, rhs) {
+        let lhs_ty = cx.typeck_results().expr_ty(lhs);
+        let rhs_ty = cx.typeck_results().expr_ty(rhs);
+        let lhs_and_rhs_have_the_same_ty = lhs_ty == rhs_ty;
+        if lhs_and_rhs_have_the_same_ty && self.is_allowed_ty(lhs_ty) && self.is_allowed_ty(rhs_ty) {
             return;
         }
-        let lhs_refs = cx.typeck_results().expr_ty(lhs).peel_refs();
-        let rhs_refs = cx.typeck_results().expr_ty(rhs).peel_refs();
-        let has_valid_assign_op = Self::has_valid_assign_op(op, rhs, rhs_refs);
-        if has_valid_assign_op || Self::has_valid_bin_op(lhs, lhs_refs, rhs, rhs_refs) {
+        let has_valid_op = if Self::is_integral(lhs_ty) && Self::is_integral(rhs_ty) {
+            let (actual_lhs, lhs_ref_counter) = peel_hir_expr_refs(lhs);
+            let (actual_rhs, rhs_ref_counter) = peel_hir_expr_refs(rhs);
+            match (Self::literal_integer(actual_lhs), Self::literal_integer(actual_rhs)) {
+                (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
+        };
+        if !has_valid_op {
+            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: &'tcx hir::Expr<'_>) {
-        if self.expr_span.is_some() || self.const_span.map_or(false, |sp| sp.contains(expr.span)) {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &hir::Expr<'tcx>) {
+        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, _) => {
-                if constant_simple(cx, cx.typeck_results(), expr).is_none() {
-                    self.issue_lint(cx, expr);
-                }
+            hir::ExprKind::Unary(un_op, un_expr) => {
+                self.manage_unary_ops(cx, expr, un_expr, *un_op);
             },
             _ => {},
         }