Skip to content

Commit 5c3c6a2

Browse files
borsbors
committed
Auto merge of #9483 - c410-f3r:arith, r=Jarcho
[arithmetic-side-effects] Finish non-overflowing ops Extends #9474 to also take into consideration "raw" binary operations. For example, `let a = b / 2` and `let a = 1 * b` won't trigger the lint. changelog: [arithmetic-side-effects] Finish non-overflowing ops
2 parents e120fb1 + b7bef4c commit 5c3c6a2

File tree

3 files changed

+127
-82
lines changed

3 files changed

+127
-82
lines changed

‎clippy_lints/src/operators/arithmetic_side_effects.rs

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -42,39 +42,30 @@ impl ArithmeticSideEffects {
4242
}
4343
}
4444

45-
/// Checks assign operators (+=, -=, *=, /=) of integers in a non-constant environment that
46-
/// won't overflow.
47-
fn has_valid_assign_op(op: &Spanned<hir::BinOpKind>, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool {
48-
if !Self::is_literal_integer(rhs, rhs_refs) {
49-
return false;
50-
}
45+
/// Assuming that `expr` is a literal integer, checks operators (+=, -=, *, /) in a
46+
/// non-constant environment that won't overflow.
47+
fn has_valid_op(op: &Spanned<hir::BinOpKind>, expr: &hir::Expr<'_>) -> bool {
5148
if let hir::BinOpKind::Add | hir::BinOpKind::Sub = op.node
52-
&& let hir::ExprKind::Lit(ref lit) = rhs.kind
49+
&& let hir::ExprKind::Lit(ref lit) = expr.kind
5350
&& let ast::LitKind::Int(0, _) = lit.node
5451
{
5552
return true;
5653
}
5754
if let hir::BinOpKind::Div | hir::BinOpKind::Rem = op.node
58-
&& let hir::ExprKind::Lit(ref lit) = rhs.kind
55+
&& let hir::ExprKind::Lit(ref lit) = expr.kind
5956
&& !matches!(lit.node, ast::LitKind::Int(0, _))
6057
{
6158
return true;
6259
}
6360
if let hir::BinOpKind::Mul = op.node
64-
&& let hir::ExprKind::Lit(ref lit) = rhs.kind
61+
&& let hir::ExprKind::Lit(ref lit) = expr.kind
6562
&& let ast::LitKind::Int(0 | 1, _) = lit.node
6663
{
6764
return true;
6865
}
6966
false
7067
}
7168

72-
/// Checks "raw" binary operators (+, -, *, /) of integers in a non-constant environment
73-
/// already handled by the CTFE.
74-
fn has_valid_bin_op(lhs: &hir::Expr<'_>, lhs_refs: Ty<'_>, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool {
75-
Self::is_literal_integer(lhs, lhs_refs) && Self::is_literal_integer(rhs, rhs_refs)
76-
}
77-
7869
/// Checks if the given `expr` has any of the inner `allowed` elements.
7970
fn is_allowed_ty(&self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
8071
self.allowed.contains(
@@ -95,7 +86,8 @@ impl ArithmeticSideEffects {
9586
}
9687

9788
fn issue_lint(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) {
98-
span_lint(cx, ARITHMETIC_SIDE_EFFECTS, expr.span, "arithmetic detected");
89+
let msg = "arithmetic operation that can potentially result in unexpected side-effects";
90+
span_lint(cx, ARITHMETIC_SIDE_EFFECTS, expr.span, msg);
9991
self.expr_span = Some(expr.span);
10092
}
10193

@@ -127,13 +119,18 @@ impl ArithmeticSideEffects {
127119
if self.is_allowed_ty(cx, lhs) || self.is_allowed_ty(cx, rhs) {
128120
return;
129121
}
130-
let lhs_refs = cx.typeck_results().expr_ty(lhs).peel_refs();
131-
let rhs_refs = cx.typeck_results().expr_ty(rhs).peel_refs();
132-
let has_valid_assign_op = Self::has_valid_assign_op(op, rhs, rhs_refs);
133-
if has_valid_assign_op || Self::has_valid_bin_op(lhs, lhs_refs, rhs, rhs_refs) {
134-
return;
122+
let has_valid_op = match (
123+
Self::is_literal_integer(lhs, cx.typeck_results().expr_ty(lhs).peel_refs()),
124+
Self::is_literal_integer(rhs, cx.typeck_results().expr_ty(rhs).peel_refs()),
125+
) {
126+
(true, true) => true,
127+
(true, false) => Self::has_valid_op(op, lhs),
128+
(false, true) => Self::has_valid_op(op, rhs),
129+
(false, false) => false,
130+
};
131+
if !has_valid_op {
132+
self.issue_lint(cx, expr);
135133
}
136-
self.issue_lint(cx, expr);
137134
}
138135
}
139136

‎tests/ui/arithmetic_side_effects.rs

Lines changed: 49 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
#![allow(
22
clippy::assign_op_pattern,
3-
unconditional_panic,
4-
clippy::unnecessary_owned_empty_strings
3+
clippy::erasing_op,
4+
clippy::identity_op,
5+
clippy::unnecessary_owned_empty_strings,
6+
arithmetic_overflow,
7+
unconditional_panic
58
)]
69
#![feature(inline_const, saturating_int_impl)]
710
#![warn(clippy::arithmetic_side_effects)]
@@ -30,7 +33,7 @@ pub fn hard_coded_allowed() {
3033
}
3134

3235
#[rustfmt::skip]
33-
pub fn non_overflowing_ops() {
36+
pub fn non_overflowing_const_ops() {
3437
const _: i32 = { let mut n = 1; n += 1; n };
3538
let _ = const { let mut n = 1; n += 1; n };
3639

@@ -41,33 +44,54 @@ pub fn non_overflowing_ops() {
4144
let _ = const { let mut n = 1; n = 1 + n; n };
4245

4346
const _: i32 = 1 + 1;
44-
let _ = 1 + 1;
4547
let _ = const { 1 + 1 };
48+
}
4649

47-
let mut _a = 1;
48-
_a += 0;
49-
_a -= 0;
50-
_a /= 99;
51-
_a %= 99;
52-
_a *= 0;
53-
_a *= 1;
50+
pub fn non_overflowing_runtime_ops() {
51+
let mut _n = i32::MAX;
52+
53+
// Assign
54+
_n += 0;
55+
_n -= 0;
56+
_n /= 99;
57+
_n %= 99;
58+
_n *= 0;
59+
_n *= 1;
60+
61+
// Binary
62+
_n = _n + 0;
63+
_n = 0 + _n;
64+
_n = _n - 0;
65+
_n = 0 - _n;
66+
_n = _n / 99;
67+
_n = _n % 99;
68+
_n = _n * 0;
69+
_n = 0 * _n;
70+
_n = _n * 1;
71+
_n = 1 * _n;
72+
_n = 23 + 85;
5473
}
5574

5675
#[rustfmt::skip]
57-
pub fn overflowing_ops() {
58-
let mut _a = 1; _a += 1;
59-
60-
let mut _b = 1; _b = _b + 1;
61-
62-
let mut _c = 1; _c = 1 + _c;
63-
64-
let mut _a = 1;
65-
_a += 1;
66-
_a -= 1;
67-
_a /= 0;
68-
_a %= 0;
69-
_a *= 2;
70-
_a *= 3;
76+
pub fn overflowing_runtime_ops() {
77+
let mut _n = i32::MAX;
78+
79+
// Assign
80+
_n += 1;
81+
_n -= 1;
82+
_n /= 0;
83+
_n %= 0;
84+
_n *= 2;
85+
86+
// Binary
87+
_n = _n + 1;
88+
_n = 1 + _n;
89+
_n = _n - 1;
90+
_n = 1 - _n;
91+
_n = _n / 0;
92+
_n = _n % 0;
93+
_n = _n * 2;
94+
_n = 2 * _n;
7195
}
7296

7397
fn main() {}

‎tests/ui/arithmetic_side_effects.stderr

Lines changed: 59 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,82 @@
1-
error: arithmetic detected
2-
--> $DIR/arithmetic_side_effects.rs:58:21
1+
error: arithmetic operation that can potentially result in unexpected side-effects
2+
--> $DIR/arithmetic_side_effects.rs:80:5
33
|
4-
LL | let mut _a = 1; _a += 1;
5-
| ^^^^^^^
4+
LL | _n += 1;
5+
| ^^^^^^^
66
|
77
= note: `-D clippy::arithmetic-side-effects` implied by `-D warnings`
88

9-
error: arithmetic detected
10-
--> $DIR/arithmetic_side_effects.rs:60:26
9+
error: arithmetic operation that can potentially result in unexpected side-effects
10+
--> $DIR/arithmetic_side_effects.rs:81:5
1111
|
12-
LL | let mut _b = 1; _b = _b + 1;
13-
| ^^^^^^
12+
LL | _n -= 1;
13+
| ^^^^^^^
1414

15-
error: arithmetic detected
16-
--> $DIR/arithmetic_side_effects.rs:62:26
15+
error: arithmetic operation that can potentially result in unexpected side-effects
16+
--> $DIR/arithmetic_side_effects.rs:82:5
1717
|
18-
LL | let mut _c = 1; _c = 1 + _c;
19-
| ^^^^^^
18+
LL | _n /= 0;
19+
| ^^^^^^^
2020

21-
error: arithmetic detected
22-
--> $DIR/arithmetic_side_effects.rs:65:5
21+
error: arithmetic operation that can potentially result in unexpected side-effects
22+
--> $DIR/arithmetic_side_effects.rs:83:5
2323
|
24-
LL | _a += 1;
24+
LL | _n %= 0;
2525
| ^^^^^^^
2626

27-
error: arithmetic detected
28-
--> $DIR/arithmetic_side_effects.rs:66:5
27+
error: arithmetic operation that can potentially result in unexpected side-effects
28+
--> $DIR/arithmetic_side_effects.rs:84:5
2929
|
30-
LL | _a -= 1;
30+
LL | _n *= 2;
3131
| ^^^^^^^
3232

33-
error: arithmetic detected
34-
--> $DIR/arithmetic_side_effects.rs:67:5
33+
error: arithmetic operation that can potentially result in unexpected side-effects
34+
--> $DIR/arithmetic_side_effects.rs:87:10
3535
|
36-
LL | _a /= 0;
37-
| ^^^^^^^
36+
LL | _n = _n + 1;
37+
| ^^^^^^
3838

39-
error: arithmetic detected
40-
--> $DIR/arithmetic_side_effects.rs:68:5
39+
error: arithmetic operation that can potentially result in unexpected side-effects
40+
--> $DIR/arithmetic_side_effects.rs:88:10
4141
|
42-
LL | _a %= 0;
43-
| ^^^^^^^
42+
LL | _n = 1 + _n;
43+
| ^^^^^^
4444

45-
error: arithmetic detected
46-
--> $DIR/arithmetic_side_effects.rs:69:5
45+
error: arithmetic operation that can potentially result in unexpected side-effects
46+
--> $DIR/arithmetic_side_effects.rs:89:10
4747
|
48-
LL | _a *= 2;
49-
| ^^^^^^^
48+
LL | _n = _n - 1;
49+
| ^^^^^^
5050

51-
error: arithmetic detected
52-
--> $DIR/arithmetic_side_effects.rs:70:5
51+
error: arithmetic operation that can potentially result in unexpected side-effects
52+
--> $DIR/arithmetic_side_effects.rs:90:10
5353
|
54-
LL | _a *= 3;
55-
| ^^^^^^^
54+
LL | _n = 1 - _n;
55+
| ^^^^^^
56+
57+
error: arithmetic operation that can potentially result in unexpected side-effects
58+
--> $DIR/arithmetic_side_effects.rs:91:10
59+
|
60+
LL | _n = _n / 0;
61+
| ^^^^^^
62+
63+
error: arithmetic operation that can potentially result in unexpected side-effects
64+
--> $DIR/arithmetic_side_effects.rs:92:10
65+
|
66+
LL | _n = _n % 0;
67+
| ^^^^^^
68+
69+
error: arithmetic operation that can potentially result in unexpected side-effects
70+
--> $DIR/arithmetic_side_effects.rs:93:10
71+
|
72+
LL | _n = _n * 2;
73+
| ^^^^^^
74+
75+
error: arithmetic operation that can potentially result in unexpected side-effects
76+
--> $DIR/arithmetic_side_effects.rs:94:10
77+
|
78+
LL | _n = 2 * _n;
79+
| ^^^^^^
5680

57-
error: aborting due to 9 previous errors
81+
error: aborting due to 13 previous errors
5882

0 commit comments

Comments
 (0)