Skip to content

Commit

Permalink
[clippy] Use symbols intended for arithmetic_side_effects
Browse files Browse the repository at this point in the history
  • Loading branch information
c410-f3r authored and flip1995 committed Sep 1, 2023
1 parent af02b43 commit b3136a8
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 11 deletions.
49 changes: 40 additions & 9 deletions clippy_lints/src/operators/arithmetic_side_effects.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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(),
}
}

Expand Down Expand Up @@ -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<hir::BinOpKind>,
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<Symbol>| {
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()
Expand Down Expand Up @@ -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
Expand Down
30 changes: 29 additions & 1 deletion tests/ui/arithmetic_side_effects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<i32>, maybe_zero: Wrapping<i32>) -> Wrapping<i32> {
x / maybe_zero
}

fn example_rem(x: Wrapping<i32>, maybe_zero: Wrapping<i32>) -> Wrapping<i32> {
x % maybe_zero
}

let [x, maybe_zero] = [1, 0].map(Wrapping);
example_div(x, maybe_zero);
example_rem(x, maybe_zero);
}

fn main() {}
14 changes: 13 additions & 1 deletion tests/ui/arithmetic_side_effects.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit b3136a8

Please sign in to comment.