Skip to content

Commit 4c5f230

Browse files
c410-f3rc410-f3r
committed
[arithmetic-side-effects] Consider references
1 parent 7248d06 commit 4c5f230

File tree

3 files changed

+172
-41
lines changed

3 files changed

+172
-41
lines changed

‎clippy_lints/src/operators/arithmetic_side_effects.rs

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use rustc_ast as ast;
99
use rustc_data_structures::fx::FxHashSet;
1010
use rustc_hir as hir;
1111
use rustc_lint::{LateContext, LateLintPass};
12-
use rustc_middle::ty::Ty;
1312
use rustc_session::impl_lint_pass;
1413
use rustc_span::source_map::{Span, Spanned};
1514

@@ -78,28 +77,47 @@ impl ArithmeticSideEffects {
7877
)
7978
}
8079

81-
/// Explicit integers like `1` or `i32::MAX`. Does not take into consideration references.
82-
fn is_literal_integer(expr: &hir::Expr<'_>, expr_refs: Ty<'_>) -> bool {
83-
let is_integral = expr_refs.is_integral();
84-
let is_literal = matches!(expr.kind, hir::ExprKind::Lit(_));
85-
is_integral && is_literal
86-
}
87-
80+
// Common entry-point to avoid code duplication.
8881
fn issue_lint(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) {
8982
let msg = "arithmetic operation that can potentially result in unexpected side-effects";
9083
span_lint(cx, ARITHMETIC_SIDE_EFFECTS, expr.span, msg);
9184
self.expr_span = Some(expr.span);
9285
}
9386

87+
/// * If `expr` is a literal integer like `1` or `i32::MAX`, returns itself.
88+
/// * Is `expr` is a literal integer reference like `&199`, returns the literal integer without
89+
/// references.
90+
/// * If `expr` is anything else, returns `None`.
91+
fn literal_integer<'expr, 'tcx>(
92+
cx: &LateContext<'tcx>,
93+
expr: &'expr hir::Expr<'tcx>,
94+
) -> Option<&'expr hir::Expr<'tcx>> {
95+
let expr_refs = cx.typeck_results().expr_ty(expr).peel_refs();
96+
97+
if !expr_refs.is_integral() {
98+
return None;
99+
}
100+
101+
if matches!(expr.kind, hir::ExprKind::Lit(_)) {
102+
return Some(expr);
103+
}
104+
105+
if let hir::ExprKind::AddrOf(.., inn) = expr.kind && let hir::ExprKind::Lit(_) = inn.kind {
106+
return Some(inn)
107+
}
108+
109+
None
110+
}
111+
94112
/// Manages when the lint should be triggered. Operations in constant environments, hard coded
95113
/// types, custom allowed types and non-constant operations that won't overflow are ignored.
96-
fn manage_bin_ops(
114+
fn manage_bin_ops<'tcx>(
97115
&mut self,
98-
cx: &LateContext<'_>,
99-
expr: &hir::Expr<'_>,
116+
cx: &LateContext<'tcx>,
117+
expr: &hir::Expr<'tcx>,
100118
op: &Spanned<hir::BinOpKind>,
101-
lhs: &hir::Expr<'_>,
102-
rhs: &hir::Expr<'_>,
119+
lhs: &hir::Expr<'tcx>,
120+
rhs: &hir::Expr<'tcx>,
103121
) {
104122
if constant_simple(cx, cx.typeck_results(), expr).is_some() {
105123
return;
@@ -119,14 +137,11 @@ impl ArithmeticSideEffects {
119137
if self.is_allowed_ty(cx, lhs) || self.is_allowed_ty(cx, rhs) {
120138
return;
121139
}
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,
140+
let has_valid_op = match (Self::literal_integer(cx, lhs), Self::literal_integer(cx, rhs)) {
141+
(None, None) => false,
142+
(None, Some(local_expr)) => Self::has_valid_op(op, local_expr),
143+
(Some(local_expr), None) => Self::has_valid_op(op, local_expr),
144+
(Some(_), Some(_)) => true,
130145
};
131146
if !has_valid_op {
132147
self.issue_lint(cx, expr);
@@ -135,7 +150,7 @@ impl ArithmeticSideEffects {
135150
}
136151

137152
impl<'tcx> LateLintPass<'tcx> for ArithmeticSideEffects {
138-
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
153+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &hir::Expr<'tcx>) {
139154
if self.expr_span.is_some() || self.const_span.map_or(false, |sp| sp.contains(expr.span)) {
140155
return;
141156
}

‎tests/ui/arithmetic_side_effects.rs

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@
22
clippy::assign_op_pattern,
33
clippy::erasing_op,
44
clippy::identity_op,
5+
clippy::op_ref,
56
clippy::unnecessary_owned_empty_strings,
67
arithmetic_overflow,
78
unconditional_panic
89
)]
9-
#![feature(inline_const, saturating_int_impl)]
10+
#![feature(const_mut_refs, inline_const, saturating_int_impl)]
1011
#![warn(clippy::arithmetic_side_effects)]
1112

1213
use core::num::{Saturating, Wrapping};
@@ -79,33 +80,50 @@ pub fn const_ops_should_not_trigger_the_lint() {
7980
const _: i32 = 1 + 1;
8081
let _ = const { 1 + 1 };
8182

82-
const _: i32 = { let mut n = -1; n = -(-1); n = -n; n };
83-
let _ = const { let mut n = -1; n = -(-1); n = -n; n };
83+
const _: i32 = { let mut n = 1; n = -1; n = -(-1); n = -n; n };
84+
let _ = const { let mut n = 1; n = -1; n = -(-1); n = -n; n };
8485
}
8586

86-
pub fn non_overflowing_runtime_ops_or_ops_already_handled_by_the_compiler() {
87+
pub fn non_overflowing_ops_or_ops_already_handled_by_the_compiler_should_not_trigger_the_lint() {
8788
let mut _n = i32::MAX;
8889

8990
// Assign
9091
_n += 0;
92+
_n += &0;
9193
_n -= 0;
94+
_n -= &0;
9295
_n /= 99;
96+
_n /= &99;
9397
_n %= 99;
98+
_n %= &99;
9499
_n *= 0;
100+
_n *= &0;
95101
_n *= 1;
102+
_n *= &1;
96103

97104
// Binary
98105
_n = _n + 0;
106+
_n = _n + &0;
99107
_n = 0 + _n;
108+
_n = &0 + _n;
100109
_n = _n - 0;
110+
_n = _n - &0;
101111
_n = 0 - _n;
112+
_n = &0 - _n;
102113
_n = _n / 99;
114+
_n = _n / &99;
103115
_n = _n % 99;
116+
_n = _n % &99;
104117
_n = _n * 0;
118+
_n = _n * &0;
105119
_n = 0 * _n;
120+
_n = &0 * _n;
106121
_n = _n * 1;
122+
_n = _n * &1;
107123
_n = 1 * _n;
124+
_n = &1 * _n;
108125
_n = 23 + 85;
126+
_n = &23 + &85;
109127

110128
// Unary
111129
_n = -1;
@@ -117,23 +135,37 @@ pub fn overflowing_runtime_ops() {
117135

118136
// Assign
119137
_n += 1;
138+
_n += &1;
120139
_n -= 1;
140+
_n -= &1;
121141
_n /= 0;
142+
_n /= &0;
122143
_n %= 0;
144+
_n %= &0;
123145
_n *= 2;
146+
_n *= &2;
124147

125148
// Binary
126149
_n = _n + 1;
150+
_n = _n + &1;
127151
_n = 1 + _n;
152+
_n = &1 + _n;
128153
_n = _n - 1;
154+
_n = _n - &1;
129155
_n = 1 - _n;
156+
_n = &1 - _n;
130157
_n = _n / 0;
158+
_n = _n / &0;
131159
_n = _n % 0;
160+
_n = _n % &0;
132161
_n = _n * 2;
162+
_n = _n * &2;
133163
_n = 2 * _n;
164+
_n = &2 * _n;
134165

135166
// Unary
136167
_n = -_n;
168+
_n = -&_n;
137169
}
138170

139171
fn main() {}

‎tests/ui/arithmetic_side_effects.stderr

Lines changed: 99 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,88 +1,172 @@
11
error: arithmetic operation that can potentially result in unexpected side-effects
2-
--> $DIR/arithmetic_side_effects.rs:119:5
2+
--> $DIR/arithmetic_side_effects.rs:137:5
33
|
44
LL | _n += 1;
55
| ^^^^^^^
66
|
77
= note: `-D clippy::arithmetic-side-effects` implied by `-D warnings`
88

99
error: arithmetic operation that can potentially result in unexpected side-effects
10-
--> $DIR/arithmetic_side_effects.rs:120:5
10+
--> $DIR/arithmetic_side_effects.rs:138:5
11+
|
12+
LL | _n += &1;
13+
| ^^^^^^^^
14+
15+
error: arithmetic operation that can potentially result in unexpected side-effects
16+
--> $DIR/arithmetic_side_effects.rs:139:5
1117
|
1218
LL | _n -= 1;
1319
| ^^^^^^^
1420

1521
error: arithmetic operation that can potentially result in unexpected side-effects
16-
--> $DIR/arithmetic_side_effects.rs:121:5
22+
--> $DIR/arithmetic_side_effects.rs:140:5
23+
|
24+
LL | _n -= &1;
25+
| ^^^^^^^^
26+
27+
error: arithmetic operation that can potentially result in unexpected side-effects
28+
--> $DIR/arithmetic_side_effects.rs:141:5
1729
|
1830
LL | _n /= 0;
1931
| ^^^^^^^
2032

2133
error: arithmetic operation that can potentially result in unexpected side-effects
22-
--> $DIR/arithmetic_side_effects.rs:122:5
34+
--> $DIR/arithmetic_side_effects.rs:142:5
35+
|
36+
LL | _n /= &0;
37+
| ^^^^^^^^
38+
39+
error: arithmetic operation that can potentially result in unexpected side-effects
40+
--> $DIR/arithmetic_side_effects.rs:143:5
2341
|
2442
LL | _n %= 0;
2543
| ^^^^^^^
2644

2745
error: arithmetic operation that can potentially result in unexpected side-effects
28-
--> $DIR/arithmetic_side_effects.rs:123:5
46+
--> $DIR/arithmetic_side_effects.rs:144:5
47+
|
48+
LL | _n %= &0;
49+
| ^^^^^^^^
50+
51+
error: arithmetic operation that can potentially result in unexpected side-effects
52+
--> $DIR/arithmetic_side_effects.rs:145:5
2953
|
3054
LL | _n *= 2;
3155
| ^^^^^^^
3256

3357
error: arithmetic operation that can potentially result in unexpected side-effects
34-
--> $DIR/arithmetic_side_effects.rs:126:10
58+
--> $DIR/arithmetic_side_effects.rs:146:5
59+
|
60+
LL | _n *= &2;
61+
| ^^^^^^^^
62+
63+
error: arithmetic operation that can potentially result in unexpected side-effects
64+
--> $DIR/arithmetic_side_effects.rs:149:10
3565
|
3666
LL | _n = _n + 1;
3767
| ^^^^^^
3868

3969
error: arithmetic operation that can potentially result in unexpected side-effects
40-
--> $DIR/arithmetic_side_effects.rs:127:10
70+
--> $DIR/arithmetic_side_effects.rs:150:10
71+
|
72+
LL | _n = _n + &1;
73+
| ^^^^^^^
74+
75+
error: arithmetic operation that can potentially result in unexpected side-effects
76+
--> $DIR/arithmetic_side_effects.rs:151:10
4177
|
4278
LL | _n = 1 + _n;
4379
| ^^^^^^
4480

4581
error: arithmetic operation that can potentially result in unexpected side-effects
46-
--> $DIR/arithmetic_side_effects.rs:128:10
82+
--> $DIR/arithmetic_side_effects.rs:152:10
83+
|
84+
LL | _n = &1 + _n;
85+
| ^^^^^^^
86+
87+
error: arithmetic operation that can potentially result in unexpected side-effects
88+
--> $DIR/arithmetic_side_effects.rs:153:10
4789
|
4890
LL | _n = _n - 1;
4991
| ^^^^^^
5092

5193
error: arithmetic operation that can potentially result in unexpected side-effects
52-
--> $DIR/arithmetic_side_effects.rs:129:10
94+
--> $DIR/arithmetic_side_effects.rs:154:10
95+
|
96+
LL | _n = _n - &1;
97+
| ^^^^^^^
98+
99+
error: arithmetic operation that can potentially result in unexpected side-effects
100+
--> $DIR/arithmetic_side_effects.rs:155:10
53101
|
54102
LL | _n = 1 - _n;
55103
| ^^^^^^
56104

57105
error: arithmetic operation that can potentially result in unexpected side-effects
58-
--> $DIR/arithmetic_side_effects.rs:130:10
106+
--> $DIR/arithmetic_side_effects.rs:156:10
107+
|
108+
LL | _n = &1 - _n;
109+
| ^^^^^^^
110+
111+
error: arithmetic operation that can potentially result in unexpected side-effects
112+
--> $DIR/arithmetic_side_effects.rs:157:10
59113
|
60114
LL | _n = _n / 0;
61115
| ^^^^^^
62116

63117
error: arithmetic operation that can potentially result in unexpected side-effects
64-
--> $DIR/arithmetic_side_effects.rs:131:10
118+
--> $DIR/arithmetic_side_effects.rs:158:10
119+
|
120+
LL | _n = _n / &0;
121+
| ^^^^^^^
122+
123+
error: arithmetic operation that can potentially result in unexpected side-effects
124+
--> $DIR/arithmetic_side_effects.rs:159:10
65125
|
66126
LL | _n = _n % 0;
67127
| ^^^^^^
68128

69129
error: arithmetic operation that can potentially result in unexpected side-effects
70-
--> $DIR/arithmetic_side_effects.rs:132:10
130+
--> $DIR/arithmetic_side_effects.rs:160:10
131+
|
132+
LL | _n = _n % &0;
133+
| ^^^^^^^
134+
135+
error: arithmetic operation that can potentially result in unexpected side-effects
136+
--> $DIR/arithmetic_side_effects.rs:161:10
71137
|
72138
LL | _n = _n * 2;
73139
| ^^^^^^
74140

75141
error: arithmetic operation that can potentially result in unexpected side-effects
76-
--> $DIR/arithmetic_side_effects.rs:133:10
142+
--> $DIR/arithmetic_side_effects.rs:162:10
143+
|
144+
LL | _n = _n * &2;
145+
| ^^^^^^^
146+
147+
error: arithmetic operation that can potentially result in unexpected side-effects
148+
--> $DIR/arithmetic_side_effects.rs:163:10
77149
|
78150
LL | _n = 2 * _n;
79151
| ^^^^^^
80152

81153
error: arithmetic operation that can potentially result in unexpected side-effects
82-
--> $DIR/arithmetic_side_effects.rs:136:10
154+
--> $DIR/arithmetic_side_effects.rs:164:10
155+
|
156+
LL | _n = &2 * _n;
157+
| ^^^^^^^
158+
159+
error: arithmetic operation that can potentially result in unexpected side-effects
160+
--> $DIR/arithmetic_side_effects.rs:167:10
83161
|
84162
LL | _n = -_n;
85163
| ^^^
86164

87-
error: aborting due to 14 previous errors
165+
error: arithmetic operation that can potentially result in unexpected side-effects
166+
--> $DIR/arithmetic_side_effects.rs:168:10
167+
|
168+
LL | _n = -&_n;
169+
| ^^^^
170+
171+
error: aborting due to 28 previous errors
88172

0 commit comments

Comments
 (0)