From 0164f7e8b20d749d38902e9d63f873fe0a74b1fd Mon Sep 17 00:00:00 2001 From: Caio Date: Thu, 31 Aug 2023 15:10:51 -0300 Subject: [PATCH] [`clippy`] Use symbols intended for `arithmetic_side_effects` --- library/core/src/num/saturating.rs | 1 + library/core/src/num/wrapping.rs | 1 + .../src/operators/arithmetic_side_effects.rs | 49 +++++++++++++++---- .../tests/ui/arithmetic_side_effects.rs | 30 +++++++++++- .../tests/ui/arithmetic_side_effects.stderr | 14 +++++- 5 files changed, 84 insertions(+), 11 deletions(-) diff --git a/library/core/src/num/saturating.rs b/library/core/src/num/saturating.rs index 8982473b2dc03..e9f794e7d6211 100644 --- a/library/core/src/num/saturating.rs +++ b/library/core/src/num/saturating.rs @@ -35,6 +35,7 @@ use crate::ops::{Shl, ShlAssign, Shr, ShrAssign, Sub, SubAssign}; #[unstable(feature = "saturating_int_impl", issue = "87920")] #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default, Hash)] #[repr(transparent)] +#[rustc_diagnostic_item = "Saturating"] pub struct Saturating(#[unstable(feature = "saturating_int_impl", issue = "87920")] pub T); #[unstable(feature = "saturating_int_impl", issue = "87920")] diff --git a/library/core/src/num/wrapping.rs b/library/core/src/num/wrapping.rs index ed354a2e50bda..16f0b6d913dfb 100644 --- a/library/core/src/num/wrapping.rs +++ b/library/core/src/num/wrapping.rs @@ -39,6 +39,7 @@ use crate::ops::{Shl, ShlAssign, Shr, ShrAssign, Sub, SubAssign}; #[stable(feature = "rust1", since = "1.0.0")] #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default, Hash)] #[repr(transparent)] +#[rustc_diagnostic_item = "Wrapping"] pub struct Wrapping(#[stable(feature = "rust1", since = "1.0.0")] pub T); #[stable(feature = "rust1", since = "1.0.0")] diff --git a/src/tools/clippy/clippy_lints/src/operators/arithmetic_side_effects.rs b/src/tools/clippy/clippy_lints/src/operators/arithmetic_side_effects.rs index f9108145cdb6d..a687c7d29b56e 100644 --- a/src/tools/clippy/clippy_lints/src/operators/arithmetic_side_effects.rs +++ b/src/tools/clippy/clippy_lints/src/operators/arithmetic_side_effects.rs @@ -1,24 +1,20 @@ use super::ARITHMETIC_SIDE_EFFECTS; use clippy_utils::consts::{constant, constant_simple, Constant}; use clippy_utils::diagnostics::span_lint; +use clippy_utils::ty::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"]; +const INTEGER_METHODS: &[Symbol] = &[sym::saturating_div, sym::wrapping_div, sym::wrapping_rem, sym::wrapping_rem_euclid]; #[derive(Debug)] pub struct ArithmeticSideEffects { @@ -53,7 +49,7 @@ impl ArithmeticSideEffects { allowed_unary, const_span: None, expr_span: None, - integer_methods: INTEGER_METHODS.iter().map(|el| Symbol::intern(el)).collect(), + integer_methods: INTEGER_METHODS.iter().copied().collect(), } } @@ -86,6 +82,38 @@ 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 { + let is_div_or_rem = matches!(op.node, hir::BinOpKind::Div | hir::BinOpKind::Rem); + let is_non_zero_u = |symbol: Option| { + matches!( + symbol, + Some(sym::NonZeroU128 | sym::NonZeroU16 | sym::NonZeroU32 | sym::NonZeroU64 | sym::NonZeroU8 | sym::NonZeroUsize) + ) + }; + let is_sat_or_wrap = |ty: Ty<'_>| { + let is_sat = type_diagnostic_name(cx, ty) == Some(sym::Saturating); + let is_wrap = type_diagnostic_name(cx, ty) == Some(sym::Wrapping); + is_sat || is_wrap + }; + + // If the RHS is NonZeroU*, then division or module by zero will never occur + if is_non_zero_u(type_diagnostic_name(cx, rhs_ty)) && is_div_or_rem { + return true; + } + // `Saturation` and `Wrapping` can overflow if the RHS is zero in a division or module + if is_sat_or_wrap(lhs_ty) { + return !is_div_or_rem; + } + + false + } + // For example, 8i32 or &i64::MAX. fn is_integral(ty: Ty<'_>) -> bool { ty.peel_refs().is_integral() @@ -147,6 +175,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/src/tools/clippy/tests/ui/arithmetic_side_effects.rs b/src/tools/clippy/tests/ui/arithmetic_side_effects.rs index 8485b3eab71b4..def30f5903d74 100644 --- a/src/tools/clippy/tests/ui/arithmetic_side_effects.rs +++ b/src/tools/clippy/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/src/tools/clippy/tests/ui/arithmetic_side_effects.stderr b/src/tools/clippy/tests/ui/arithmetic_side_effects.stderr index e9a626643ff78..7f490748160b8 100644 --- a/src/tools/clippy/tests/ui/arithmetic_side_effects.stderr +++ b/src/tools/clippy/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:512:9 + | +LL | x / maybe_zero + | ^^^^^^^^^^^^^^ + +error: arithmetic operation that can potentially result in unexpected side-effects + --> $DIR/arithmetic_side_effects.rs:516:9 + | +LL | x % maybe_zero + | ^^^^^^^^^^^^^^ + +error: aborting due to 119 previous errors