Skip to content

Commit d6b4d35

Browse files
committed
Auto merge of rust-lang#115415 - c410-f3r:t3st3ss, r=flip1995
[`Clippy`] Use symbols intended for `arithmetic_side_effects` rust-lang#115177 added the symbols for `arithmetic_side_effects` and now this PR actually uses them to prevent an eventual removal. All because rust-lang#115183 was recently merged and next Clippy update will probably take some time to happen. Fixes rust-lang/rust-clippy#11392
2 parents 96f62fc + 0164f7e commit d6b4d35

File tree

5 files changed

+84
-11
lines changed

5 files changed

+84
-11
lines changed

library/core/src/num/saturating.rs

+1
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ use crate::ops::{Shl, ShlAssign, Shr, ShrAssign, Sub, SubAssign};
3535
#[unstable(feature = "saturating_int_impl", issue = "87920")]
3636
#[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default, Hash)]
3737
#[repr(transparent)]
38+
#[rustc_diagnostic_item = "Saturating"]
3839
pub struct Saturating<T>(#[unstable(feature = "saturating_int_impl", issue = "87920")] pub T);
3940

4041
#[unstable(feature = "saturating_int_impl", issue = "87920")]

library/core/src/num/wrapping.rs

+1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ use crate::ops::{Shl, ShlAssign, Shr, ShrAssign, Sub, SubAssign};
3939
#[stable(feature = "rust1", since = "1.0.0")]
4040
#[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default, Hash)]
4141
#[repr(transparent)]
42+
#[rustc_diagnostic_item = "Wrapping"]
4243
pub struct Wrapping<T>(#[stable(feature = "rust1", since = "1.0.0")] pub T);
4344

4445
#[stable(feature = "rust1", since = "1.0.0")]

src/tools/clippy/clippy_lints/src/operators/arithmetic_side_effects.rs

+40-9
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,20 @@
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::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"];
21-
const INTEGER_METHODS: &[&str] = &["saturating_div", "wrapping_div", "wrapping_rem", "wrapping_rem_euclid"];
17+
const INTEGER_METHODS: &[Symbol] = &[sym::saturating_div, sym::wrapping_div, sym::wrapping_rem, sym::wrapping_rem_euclid];
2218

2319
#[derive(Debug)]
2420
pub struct ArithmeticSideEffects {
@@ -53,7 +49,7 @@ impl ArithmeticSideEffects {
5349
allowed_unary,
5450
const_span: None,
5551
expr_span: None,
56-
integer_methods: INTEGER_METHODS.iter().map(|el| Symbol::intern(el)).collect(),
52+
integer_methods: INTEGER_METHODS.iter().copied().collect(),
5753
}
5854
}
5955

@@ -86,6 +82,38 @@ 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+
let is_div_or_rem = matches!(op.node, hir::BinOpKind::Div | hir::BinOpKind::Rem);
93+
let is_non_zero_u = |symbol: Option<Symbol>| {
94+
matches!(
95+
symbol,
96+
Some(sym::NonZeroU128 | sym::NonZeroU16 | sym::NonZeroU32 | sym::NonZeroU64 | sym::NonZeroU8 | sym::NonZeroUsize)
97+
)
98+
};
99+
let is_sat_or_wrap = |ty: Ty<'_>| {
100+
let is_sat = type_diagnostic_name(cx, ty) == Some(sym::Saturating);
101+
let is_wrap = type_diagnostic_name(cx, ty) == Some(sym::Wrapping);
102+
is_sat || is_wrap
103+
};
104+
105+
// If the RHS is NonZeroU*, then division or module by zero will never occur
106+
if is_non_zero_u(type_diagnostic_name(cx, rhs_ty)) && is_div_or_rem {
107+
return true;
108+
}
109+
// `Saturation` and `Wrapping` can overflow if the RHS is zero in a division or module
110+
if is_sat_or_wrap(lhs_ty) {
111+
return !is_div_or_rem;
112+
}
113+
114+
false
115+
}
116+
89117
// For example, 8i32 or &i64::MAX.
90118
fn is_integral(ty: Ty<'_>) -> bool {
91119
ty.peel_refs().is_integral()
@@ -147,6 +175,9 @@ impl ArithmeticSideEffects {
147175
if self.has_allowed_binary(lhs_ty, rhs_ty) {
148176
return;
149177
}
178+
if Self::has_specific_allowed_type_and_operation(cx, lhs_ty, op, rhs_ty) {
179+
return;
180+
}
150181
let has_valid_op = if Self::is_integral(lhs_ty) && Self::is_integral(rhs_ty) {
151182
if let hir::BinOpKind::Shl | hir::BinOpKind::Shr = op.node {
152183
// At least for integers, shifts are already handled by the CTFE

src/tools/clippy/tests/ui/arithmetic_side_effects.rs

+29-1
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() {}

src/tools/clippy/tests/ui/arithmetic_side_effects.stderr

+13-1
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:512:9
707+
|
708+
LL | x / maybe_zero
709+
| ^^^^^^^^^^^^^^
710+
711+
error: arithmetic operation that can potentially result in unexpected side-effects
712+
--> $DIR/arithmetic_side_effects.rs:516:9
713+
|
714+
LL | x % maybe_zero
715+
| ^^^^^^^^^^^^^^
716+
717+
error: aborting due to 119 previous errors
706718

0 commit comments

Comments
 (0)