From 2faa43c8b455bd66a0d93f9af5beac4fb7698234 Mon Sep 17 00:00:00 2001 From: Caio Date: Thu, 24 Aug 2023 13:22:27 -0300 Subject: [PATCH 1/2] [arithmetic_side_effects] Fix #11393 --- .../src/operators/arithmetic_side_effects.rs | 55 ++++++++++++++++--- tests/ui/arithmetic_side_effects.rs | 30 +++++++++- tests/ui/arithmetic_side_effects.stderr | 14 ++++- 3 files changed, 90 insertions(+), 9 deletions(-) diff --git a/clippy_lints/src/operators/arithmetic_side_effects.rs b/clippy_lints/src/operators/arithmetic_side_effects.rs index f9108145c..377b56fad 100644 --- a/clippy_lints/src/operators/arithmetic_side_effects.rs +++ b/clippy_lints/src/operators/arithmetic_side_effects.rs @@ -1,22 +1,18 @@ use super::ARITHMETIC_SIDE_EFFECTS; use clippy_utils::consts::{constant, constant_simple, Constant}; use clippy_utils::diagnostics::span_lint; +use clippy_utils::ty::{match_type, type_diagnostic_name}; use clippy_utils::{expr_or_init, is_from_proc_macro, is_lint_allowed, peel_hir_expr_refs, peel_hir_expr_unary}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::Ty; use rustc_session::impl_lint_pass; use rustc_span::source_map::{Span, Spanned}; +use rustc_span::symbol::sym; use rustc_span::Symbol; use {rustc_ast as ast, rustc_hir as hir}; -const HARD_CODED_ALLOWED_BINARY: &[[&str; 2]] = &[ - ["f32", "f32"], - ["f64", "f64"], - ["std::num::Saturating", "*"], - ["std::num::Wrapping", "*"], - ["std::string::String", "str"], -]; +const HARD_CODED_ALLOWED_BINARY: &[[&str; 2]] = &[["f32", "f32"], ["f64", "f64"], ["std::string::String", "str"]]; const HARD_CODED_ALLOWED_UNARY: &[&str] = &["f32", "f64", "std::num::Saturating", "std::num::Wrapping"]; const INTEGER_METHODS: &[&str] = &["saturating_div", "wrapping_div", "wrapping_rem", "wrapping_rem_euclid"]; @@ -86,6 +82,48 @@ impl ArithmeticSideEffects { self.allowed_unary.contains(ty_string_elem) } + /// Verifies built-in types that have specific allowed operations + fn has_specific_allowed_type_and_operation( + cx: &LateContext<'_>, + lhs_ty: Ty<'_>, + op: &Spanned, + rhs_ty: Ty<'_>, + ) -> bool { + const SATURATING: &[&str] = &["core", "num", "saturating", "Saturating"]; + const WRAPPING: &[&str] = &["core", "num", "wrapping", "Wrapping"]; + let is_non_zero = |symbol: Option| { + matches!( + symbol, + Some( + sym::NonZeroI128 + | sym::NonZeroI16 + | sym::NonZeroI32 + | sym::NonZeroI64 + | sym::NonZeroI8 + | sym::NonZeroU128 + | sym::NonZeroU16 + | sym::NonZeroU32 + | sym::NonZeroU64 + | sym::NonZeroU8 + ) + ) + }; + // If the RHS is NonZero*, then division or module by zero will never occur + if is_non_zero(type_diagnostic_name(cx, rhs_ty)) && let hir::BinOpKind::Div | hir::BinOpKind::Rem = op.node { + return true; + } + // For `Saturation` or `Wrapping` (RHS), all but division and module are allowed. + let is_div_or_rem = matches!(op.node, hir::BinOpKind::Div | hir::BinOpKind::Rem); + if (match_type(cx, rhs_ty, SATURATING) || match_type(cx, rhs_ty, WRAPPING)) && !is_div_or_rem { + return true; + } + // For `Saturation` or `Wrapping` (LHS), everything is allowed + if match_type(cx, lhs_ty, SATURATING) || match_type(cx, lhs_ty, WRAPPING) { + return true; + } + return false; + } + // For example, 8i32 or &i64::MAX. fn is_integral(ty: Ty<'_>) -> bool { ty.peel_refs().is_integral() @@ -147,6 +185,9 @@ impl ArithmeticSideEffects { if self.has_allowed_binary(lhs_ty, rhs_ty) { return; } + if Self::has_specific_allowed_type_and_operation(cx, lhs_ty, op, rhs_ty) { + return; + } let has_valid_op = if Self::is_integral(lhs_ty) && Self::is_integral(rhs_ty) { if let hir::BinOpKind::Shl | hir::BinOpKind::Shr = op.node { // At least for integers, shifts are already handled by the CTFE diff --git a/tests/ui/arithmetic_side_effects.rs b/tests/ui/arithmetic_side_effects.rs index 8485b3eab..def30f590 100644 --- a/tests/ui/arithmetic_side_effects.rs +++ b/tests/ui/arithmetic_side_effects.rs @@ -15,7 +15,7 @@ extern crate proc_macro_derive; -use core::num::{Saturating, Wrapping}; +use core::num::{NonZeroUsize, Saturating, Wrapping}; const ONE: i32 = 1; const ZERO: i32 = 0; @@ -493,4 +493,32 @@ pub fn issue_11262() { let _ = 2 / zero; } +pub fn issue_11392() { + fn example_div(unsigned: usize, nonzero_unsigned: NonZeroUsize) -> usize { + unsigned / nonzero_unsigned + } + + fn example_rem(unsigned: usize, nonzero_unsigned: NonZeroUsize) -> usize { + unsigned % nonzero_unsigned + } + + let (unsigned, nonzero_unsigned) = (0, NonZeroUsize::new(1).unwrap()); + example_div(unsigned, nonzero_unsigned); + example_rem(unsigned, nonzero_unsigned); +} + +pub fn issue_11393() { + fn example_div(x: Wrapping, maybe_zero: Wrapping) -> Wrapping { + x / maybe_zero + } + + fn example_rem(x: Wrapping, maybe_zero: Wrapping) -> Wrapping { + x % maybe_zero + } + + let [x, maybe_zero] = [1, 0].map(Wrapping); + example_div(x, maybe_zero); + example_rem(x, maybe_zero); +} + fn main() {} diff --git a/tests/ui/arithmetic_side_effects.stderr b/tests/ui/arithmetic_side_effects.stderr index e9a626643..af6b5521c 100644 --- a/tests/ui/arithmetic_side_effects.stderr +++ b/tests/ui/arithmetic_side_effects.stderr @@ -702,5 +702,17 @@ error: arithmetic operation that can potentially result in unexpected side-effec LL | 10 / a | ^^^^^^ -error: aborting due to 117 previous errors +error: arithmetic operation that can potentially result in unexpected side-effects + --> $DIR/arithmetic_side_effects.rs:498:9 + | +LL | unsigned / nonzero_unsigned + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: arithmetic operation that can potentially result in unexpected side-effects + --> $DIR/arithmetic_side_effects.rs:502:9 + | +LL | unsigned % nonzero_unsigned + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 119 previous errors From d802ab28de8c7e8095fd00fe6cec48ed809ad3b3 Mon Sep 17 00:00:00 2001 From: Caio Date: Thu, 24 Aug 2023 13:34:36 -0300 Subject: [PATCH 2/2] Dogfood --- clippy_lints/src/operators/arithmetic_side_effects.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/operators/arithmetic_side_effects.rs b/clippy_lints/src/operators/arithmetic_side_effects.rs index 377b56fad..edd18b86b 100644 --- a/clippy_lints/src/operators/arithmetic_side_effects.rs +++ b/clippy_lints/src/operators/arithmetic_side_effects.rs @@ -121,7 +121,7 @@ impl ArithmeticSideEffects { if match_type(cx, lhs_ty, SATURATING) || match_type(cx, lhs_ty, WRAPPING) { return true; } - return false; + false } // For example, 8i32 or &i64::MAX.