From 9b783f74999be1bf29831dbf39caf5439c5657a5 Mon Sep 17 00:00:00 2001 From: Caio Date: Wed, 24 Apr 2024 10:39:39 -0300 Subject: [PATCH] [arithmetic_side_effects] Fix #11266 --- .../src/operators/arithmetic_side_effects.rs | 54 +++++++++++++------ tests/ui/arithmetic_side_effects.rs | 13 +++++ tests/ui/arithmetic_side_effects.stderr | 12 +++-- 3 files changed, 59 insertions(+), 20 deletions(-) diff --git a/clippy_lints/src/operators/arithmetic_side_effects.rs b/clippy_lints/src/operators/arithmetic_side_effects.rs index 7d6f26cde3e9..d147aaac780b 100644 --- a/clippy_lints/src/operators/arithmetic_side_effects.rs +++ b/clippy_lints/src/operators/arithmetic_side_effects.rs @@ -132,6 +132,16 @@ impl ArithmeticSideEffects { false } + /// For example, `fn foo() -> i32 { N + 1 }`. + fn is_const_param(expr: &hir::Expr<'_>) -> bool { + if let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind + && let hir::def::Res::Def(hir::def::DefKind::ConstParam, _) = path.res + { + return true; + } + false + } + // For example, 8i32 or &i64::MAX. fn is_integral(ty: Ty<'_>) -> bool { ty.peel_refs().is_integral() @@ -233,24 +243,34 @@ impl ArithmeticSideEffects { Self::literal_integer(cx, actual_lhs), Self::literal_integer(cx, actual_rhs), ) { - (None, None) => false, - (None, Some(n)) => match (&op, n) { - // Division and module are always valid if applied to non-zero integers - (hir::BinOpKind::Div | hir::BinOpKind::Rem, local_n) if local_n != 0 => true, - // Adding or subtracting zeros is always a no-op - (hir::BinOpKind::Add | hir::BinOpKind::Sub, 0) - // Multiplication by 1 or 0 will never overflow - | (hir::BinOpKind::Mul, 0 | 1) - => true, - _ => false, + (None, None) => Self::is_const_param(lhs) && Self::is_const_param(rhs), + (None, Some(n)) => { + if Self::is_const_param(lhs) { + return; + } + match (&op, n) { + // Division and module are always valid if applied to non-zero integers + (hir::BinOpKind::Div | hir::BinOpKind::Rem, local_n) if local_n != 0 => true, + // Adding or subtracting zeros is always a no-op + (hir::BinOpKind::Add | hir::BinOpKind::Sub, 0) + // Multiplication by 1 or 0 will never overflow + | (hir::BinOpKind::Mul, 0 | 1) + => true, + _ => false, + } }, - (Some(n), None) => match (&op, n) { - // Adding or subtracting zeros is always a no-op - (hir::BinOpKind::Add | hir::BinOpKind::Sub, 0) - // Multiplication by 1 or 0 will never overflow - | (hir::BinOpKind::Mul, 0 | 1) - => true, - _ => false, + (Some(n), None) => { + if Self::is_const_param(rhs) { + return; + } + match (&op, n) { + // Adding or subtracting zeros is always a no-op + (hir::BinOpKind::Add | hir::BinOpKind::Sub, 0) + // Multiplication by 1 or 0 will never overflow + | (hir::BinOpKind::Mul, 0 | 1) + => true, + _ => false, + } }, (Some(_), Some(_)) => { matches!((lhs_ref_counter, rhs_ref_counter), (0, 0)) diff --git a/tests/ui/arithmetic_side_effects.rs b/tests/ui/arithmetic_side_effects.rs index e6951826b2f3..3a5eb66fa27c 100644 --- a/tests/ui/arithmetic_side_effects.rs +++ b/tests/ui/arithmetic_side_effects.rs @@ -521,6 +521,19 @@ pub fn issue_11393() { example_rem(x, maybe_zero); } +pub fn issue_11266_literal() { + let _ = N + 1; + let _ = 1 + N; + let _ = N + N; +} + +pub fn issue_11266_runtime() { + fn runtime_number() -> usize { + 1 + } + let _ = N + runtime_number(); +} + pub fn issue_12318() { use core::ops::{AddAssign, DivAssign, MulAssign, RemAssign, SubAssign}; let mut one: i32 = 1; diff --git a/tests/ui/arithmetic_side_effects.stderr b/tests/ui/arithmetic_side_effects.stderr index 8039c0bfa248..f1b91cb7b0d7 100644 --- a/tests/ui/arithmetic_side_effects.stderr +++ b/tests/ui/arithmetic_side_effects.stderr @@ -716,16 +716,22 @@ LL | x % maybe_zero | ^^^^^^^^^^^^^^ error: arithmetic operation that can potentially result in unexpected side-effects - --> tests/ui/arithmetic_side_effects.rs:527:5 + --> tests/ui/arithmetic_side_effects.rs:534:13 + | +LL | let _ = N + runtime_number(); + | ^^^^^^^^^^^^^^^^^^^^ + +error: arithmetic operation that can potentially result in unexpected side-effects + --> tests/ui/arithmetic_side_effects.rs:540:5 | LL | one.add_assign(1); | ^^^^^^^^^^^^^^^^^ error: arithmetic operation that can potentially result in unexpected side-effects - --> tests/ui/arithmetic_side_effects.rs:531:5 + --> tests/ui/arithmetic_side_effects.rs:544:5 | LL | one.sub_assign(1); | ^^^^^^^^^^^^^^^^^ -error: aborting due to 121 previous errors +error: aborting due to 122 previous errors