Skip to content

Commit 706c48b

Browse files
committed
Auto merge of #11395 - c410-f3r:let-chain, r=Manishearth
[`arithmetic_side_effects`] Fix #11393 Fix #11393 ``` changelog: [`arithmetic_side_effects`]: Detect division by zero for `Wrapping` and `Saturating` ```
2 parents 19eaafb + d802ab2 commit 706c48b

File tree

3 files changed

+90
-9
lines changed

3 files changed

+90
-9
lines changed

clippy_lints/src/operators/arithmetic_side_effects.rs

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,18 @@
11
use super::ARITHMETIC_SIDE_EFFECTS;
22
use clippy_utils::consts::{constant, constant_simple, Constant};
33
use clippy_utils::diagnostics::span_lint;
4+
use clippy_utils::ty::{match_type, type_diagnostic_name};
45
use clippy_utils::{expr_or_init, is_from_proc_macro, is_lint_allowed, peel_hir_expr_refs, peel_hir_expr_unary};
56
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
67
use rustc_lint::{LateContext, LateLintPass};
78
use rustc_middle::ty::Ty;
89
use rustc_session::impl_lint_pass;
910
use rustc_span::source_map::{Span, Spanned};
11+
use rustc_span::symbol::sym;
1012
use rustc_span::Symbol;
1113
use {rustc_ast as ast, rustc_hir as hir};
1214

13-
const HARD_CODED_ALLOWED_BINARY: &[[&str; 2]] = &[
14-
["f32", "f32"],
15-
["f64", "f64"],
16-
["std::num::Saturating", "*"],
17-
["std::num::Wrapping", "*"],
18-
["std::string::String", "str"],
19-
];
15+
const HARD_CODED_ALLOWED_BINARY: &[[&str; 2]] = &[["f32", "f32"], ["f64", "f64"], ["std::string::String", "str"]];
2016
const HARD_CODED_ALLOWED_UNARY: &[&str] = &["f32", "f64", "std::num::Saturating", "std::num::Wrapping"];
2117
const INTEGER_METHODS: &[&str] = &["saturating_div", "wrapping_div", "wrapping_rem", "wrapping_rem_euclid"];
2218

@@ -86,6 +82,48 @@ impl ArithmeticSideEffects {
8682
self.allowed_unary.contains(ty_string_elem)
8783
}
8884

85+
/// Verifies built-in types that have specific allowed operations
86+
fn has_specific_allowed_type_and_operation(
87+
cx: &LateContext<'_>,
88+
lhs_ty: Ty<'_>,
89+
op: &Spanned<hir::BinOpKind>,
90+
rhs_ty: Ty<'_>,
91+
) -> bool {
92+
const SATURATING: &[&str] = &["core", "num", "saturating", "Saturating"];
93+
const WRAPPING: &[&str] = &["core", "num", "wrapping", "Wrapping"];
94+
let is_non_zero = |symbol: Option<Symbol>| {
95+
matches!(
96+
symbol,
97+
Some(
98+
sym::NonZeroI128
99+
| sym::NonZeroI16
100+
| sym::NonZeroI32
101+
| sym::NonZeroI64
102+
| sym::NonZeroI8
103+
| sym::NonZeroU128
104+
| sym::NonZeroU16
105+
| sym::NonZeroU32
106+
| sym::NonZeroU64
107+
| sym::NonZeroU8
108+
)
109+
)
110+
};
111+
// If the RHS is NonZero*, then division or module by zero will never occur
112+
if is_non_zero(type_diagnostic_name(cx, rhs_ty)) && let hir::BinOpKind::Div | hir::BinOpKind::Rem = op.node {
113+
return true;
114+
}
115+
// For `Saturation` or `Wrapping` (RHS), all but division and module are allowed.
116+
let is_div_or_rem = matches!(op.node, hir::BinOpKind::Div | hir::BinOpKind::Rem);
117+
if (match_type(cx, rhs_ty, SATURATING) || match_type(cx, rhs_ty, WRAPPING)) && !is_div_or_rem {
118+
return true;
119+
}
120+
// For `Saturation` or `Wrapping` (LHS), everything is allowed
121+
if match_type(cx, lhs_ty, SATURATING) || match_type(cx, lhs_ty, WRAPPING) {
122+
return true;
123+
}
124+
false
125+
}
126+
89127
// For example, 8i32 or &i64::MAX.
90128
fn is_integral(ty: Ty<'_>) -> bool {
91129
ty.peel_refs().is_integral()
@@ -147,6 +185,9 @@ impl ArithmeticSideEffects {
147185
if self.has_allowed_binary(lhs_ty, rhs_ty) {
148186
return;
149187
}
188+
if Self::has_specific_allowed_type_and_operation(cx, lhs_ty, op, rhs_ty) {
189+
return;
190+
}
150191
let has_valid_op = if Self::is_integral(lhs_ty) && Self::is_integral(rhs_ty) {
151192
if let hir::BinOpKind::Shl | hir::BinOpKind::Shr = op.node {
152193
// At least for integers, shifts are already handled by the CTFE

tests/ui/arithmetic_side_effects.rs

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
extern crate proc_macro_derive;
1717

18-
use core::num::{Saturating, Wrapping};
18+
use core::num::{NonZeroUsize, Saturating, Wrapping};
1919

2020
const ONE: i32 = 1;
2121
const ZERO: i32 = 0;
@@ -493,4 +493,32 @@ pub fn issue_11262() {
493493
let _ = 2 / zero;
494494
}
495495

496+
pub fn issue_11392() {
497+
fn example_div(unsigned: usize, nonzero_unsigned: NonZeroUsize) -> usize {
498+
unsigned / nonzero_unsigned
499+
}
500+
501+
fn example_rem(unsigned: usize, nonzero_unsigned: NonZeroUsize) -> usize {
502+
unsigned % nonzero_unsigned
503+
}
504+
505+
let (unsigned, nonzero_unsigned) = (0, NonZeroUsize::new(1).unwrap());
506+
example_div(unsigned, nonzero_unsigned);
507+
example_rem(unsigned, nonzero_unsigned);
508+
}
509+
510+
pub fn issue_11393() {
511+
fn example_div(x: Wrapping<i32>, maybe_zero: Wrapping<i32>) -> Wrapping<i32> {
512+
x / maybe_zero
513+
}
514+
515+
fn example_rem(x: Wrapping<i32>, maybe_zero: Wrapping<i32>) -> Wrapping<i32> {
516+
x % maybe_zero
517+
}
518+
519+
let [x, maybe_zero] = [1, 0].map(Wrapping);
520+
example_div(x, maybe_zero);
521+
example_rem(x, maybe_zero);
522+
}
523+
496524
fn main() {}

tests/ui/arithmetic_side_effects.stderr

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -702,5 +702,17 @@ error: arithmetic operation that can potentially result in unexpected side-effec
702702
LL | 10 / a
703703
| ^^^^^^
704704

705-
error: aborting due to 117 previous errors
705+
error: arithmetic operation that can potentially result in unexpected side-effects
706+
--> $DIR/arithmetic_side_effects.rs:498:9
707+
|
708+
LL | unsigned / nonzero_unsigned
709+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
710+
711+
error: arithmetic operation that can potentially result in unexpected side-effects
712+
--> $DIR/arithmetic_side_effects.rs:502:9
713+
|
714+
LL | unsigned % nonzero_unsigned
715+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
716+
717+
error: aborting due to 119 previous errors
706718

0 commit comments

Comments
 (0)