Skip to content

Commit 3f0d0c1

Browse files
committed
Auto merge of #10112 - c410-f3r:arith, r=xFrednet
[arithmetic-side-effects] Consider negative numbers and add more tests Same as #9867. Opening again because it is not possible to randomly choose a reviewer in an ongoing PR like in the rust repo. --- changelog: PF: [`arithmetic_side_effects`]: No longer lints on corner cases with negative number literals [#9867](#9867) <!-- changelog_checked -->
2 parents 179a22f + 4262aeb commit 3f0d0c1

File tree

5 files changed

+304
-62
lines changed

5 files changed

+304
-62
lines changed

clippy_lints/src/operators/arithmetic_side_effects.rs

+7-4
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use super::ARITHMETIC_SIDE_EFFECTS;
22
use clippy_utils::{
33
consts::{constant, constant_simple},
44
diagnostics::span_lint,
5-
peel_hir_expr_refs,
5+
peel_hir_expr_refs, peel_hir_expr_unary,
66
};
77
use rustc_ast as ast;
88
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
@@ -98,8 +98,11 @@ impl ArithmeticSideEffects {
9898
}
9999

100100
/// If `expr` is not a literal integer like `1`, returns `None`.
101+
///
102+
/// Returns the absolute value of the expression, if this is an integer literal.
101103
fn literal_integer(expr: &hir::Expr<'_>) -> Option<u128> {
102-
if let hir::ExprKind::Lit(ref lit) = expr.kind && let ast::LitKind::Int(n, _) = lit.node {
104+
let actual = peel_hir_expr_unary(expr).0;
105+
if let hir::ExprKind::Lit(ref lit) = actual.kind && let ast::LitKind::Int(n, _) = lit.node {
103106
Some(n)
104107
}
105108
else {
@@ -123,12 +126,12 @@ impl ArithmeticSideEffects {
123126
if !matches!(
124127
op.node,
125128
hir::BinOpKind::Add
126-
| hir::BinOpKind::Sub
127-
| hir::BinOpKind::Mul
128129
| hir::BinOpKind::Div
130+
| hir::BinOpKind::Mul
129131
| hir::BinOpKind::Rem
130132
| hir::BinOpKind::Shl
131133
| hir::BinOpKind::Shr
134+
| hir::BinOpKind::Sub
132135
) {
133136
return;
134137
};

clippy_utils/src/lib.rs

+12
Original file line numberDiff line numberDiff line change
@@ -2258,6 +2258,18 @@ pub fn peel_n_hir_expr_refs<'a>(expr: &'a Expr<'a>, count: usize) -> (&'a Expr<'
22582258
(e, count - remaining)
22592259
}
22602260

2261+
/// Peels off all unary operators of an expression. Returns the underlying expression and the number
2262+
/// of operators removed.
2263+
pub fn peel_hir_expr_unary<'a>(expr: &'a Expr<'a>) -> (&'a Expr<'a>, usize) {
2264+
let mut count: usize = 0;
2265+
let mut curr_expr = expr;
2266+
while let ExprKind::Unary(_, local_expr) = curr_expr.kind {
2267+
count = count.wrapping_add(1);
2268+
curr_expr = local_expr;
2269+
}
2270+
(curr_expr, count)
2271+
}
2272+
22612273
/// Peels off all references on the expression. Returns the underlying expression and the number of
22622274
/// references removed.
22632275
pub fn peel_hir_expr_refs<'a>(expr: &'a Expr<'a>) -> (&'a Expr<'a>, usize) {

tests/ui-toml/arithmetic_side_effects_allowed/arithmetic_side_effects_allowed.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ fn rhs_is_different() {
107107
fn unary() {
108108
// is explicitly on the list
109109
let _ = -OutOfNames;
110-
// is specifically on the list
110+
// is explicitly on the list
111111
let _ = -Foo;
112112
// not on the list
113113
let _ = -Bar;

tests/ui/arithmetic_side_effects.rs

+66-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
clippy::assign_op_pattern,
33
clippy::erasing_op,
44
clippy::identity_op,
5+
clippy::no_effect,
56
clippy::op_ref,
67
clippy::unnecessary_owned_empty_strings,
78
arithmetic_overflow,
@@ -125,6 +126,18 @@ pub fn non_overflowing_ops_or_ops_already_handled_by_the_compiler_should_not_tri
125126
_n *= &0;
126127
_n *= 1;
127128
_n *= &1;
129+
_n += -0;
130+
_n += &-0;
131+
_n -= -0;
132+
_n -= &-0;
133+
_n /= -99;
134+
_n /= &-99;
135+
_n %= -99;
136+
_n %= &-99;
137+
_n *= -0;
138+
_n *= &-0;
139+
_n *= -1;
140+
_n *= &-1;
128141

129142
// Binary
130143
_n = _n + 0;
@@ -158,7 +171,7 @@ pub fn non_overflowing_ops_or_ops_already_handled_by_the_compiler_should_not_tri
158171
_n = -&i32::MIN;
159172
}
160173

161-
pub fn runtime_ops() {
174+
pub fn unknown_ops_or_runtime_ops_that_can_overflow() {
162175
let mut _n = i32::MAX;
163176

164177
// Assign
@@ -172,6 +185,16 @@ pub fn runtime_ops() {
172185
_n %= &0;
173186
_n *= 2;
174187
_n *= &2;
188+
_n += -1;
189+
_n += &-1;
190+
_n -= -1;
191+
_n -= &-1;
192+
_n /= -0;
193+
_n /= &-0;
194+
_n %= -0;
195+
_n %= &-0;
196+
_n *= -2;
197+
_n *= &-2;
175198

176199
// Binary
177200
_n = _n + 1;
@@ -225,4 +248,46 @@ pub fn runtime_ops() {
225248
_n = -&_n;
226249
}
227250

251+
// Copied and pasted from the `integer_arithmetic` lint for comparison.
252+
pub fn integer_arithmetic() {
253+
let mut i = 1i32;
254+
let mut var1 = 0i32;
255+
let mut var2 = -1i32;
256+
257+
1 + i;
258+
i * 2;
259+
1 % i / 2;
260+
i - 2 + 2 - i;
261+
-i;
262+
i >> 1;
263+
i << 1;
264+
265+
-1;
266+
-(-1);
267+
268+
i & 1;
269+
i | 1;
270+
i ^ 1;
271+
272+
i += 1;
273+
i -= 1;
274+
i *= 2;
275+
i /= 2;
276+
i /= 0;
277+
i /= -1;
278+
i /= var1;
279+
i /= var2;
280+
i %= 2;
281+
i %= 0;
282+
i %= -1;
283+
i %= var1;
284+
i %= var2;
285+
i <<= 3;
286+
i >>= 2;
287+
288+
i |= 1;
289+
i &= 1;
290+
i ^= i;
291+
}
292+
228293
fn main() {}

0 commit comments

Comments
 (0)