Skip to content

Commit 8f91440

Browse files
c410-f3rc410-f3r
committed
[Arithmetic] Consider literals
1 parent 5735a3b commit 8f91440

File tree

3 files changed

+98
-35
lines changed

3 files changed

+98
-35
lines changed

‎clippy_lints/src/operators/arithmetic.rs

Lines changed: 56 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@ use clippy_utils::{consts::constant_simple, diagnostics::span_lint};
88
use rustc_data_structures::fx::FxHashSet;
99
use rustc_hir as hir;
1010
use rustc_lint::{LateContext, LateLintPass};
11+
use rustc_middle::ty::Ty;
1112
use rustc_session::impl_lint_pass;
12-
use rustc_span::source_map::Span;
13+
use rustc_span::source_map::{Span, Spanned};
1314

1415
const HARD_CODED_ALLOWED: &[&str] = &["std::num::Saturating", "std::string::String", "std::num::Wrapping"];
1516

@@ -46,40 +47,65 @@ impl Arithmetic {
4647
)
4748
}
4849

50+
/// Explicit integers like `1` or `i32::MAX`. Does not take into consideration references.
51+
fn is_literal_integer(expr: &hir::Expr<'_>, expr_refs: Ty<'_>) -> bool {
52+
let is_integral = expr_refs.is_integral();
53+
let is_literal = matches!(expr.kind, hir::ExprKind::Lit(_));
54+
is_integral && is_literal
55+
}
56+
4957
fn issue_lint(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) {
5058
span_lint(cx, ARITHMETIC, expr.span, "arithmetic detected");
5159
self.expr_span = Some(expr.span);
5260
}
61+
62+
/// Manages when the lint should be triggered. Hard coded types, custom allowed types and
63+
/// integer literals already handled by the CTFE are ignored.
64+
///
65+
/// It is sufficient to only check if one of the left or right hand side types are integers
66+
/// because the compiler will warn operations of incompatible types. Such behavior fits for
67+
/// both "raw" (-, +, *) and assign (-=, +=, *=) binary operators.
68+
fn manage_bin_ops(
69+
&mut self,
70+
cx: &LateContext<'_>,
71+
expr: &hir::Expr<'_>,
72+
op: &Spanned<hir::BinOpKind>,
73+
lhs: &hir::Expr<'_>,
74+
rhs: &hir::Expr<'_>,
75+
) {
76+
let (
77+
hir::BinOpKind::Add
78+
| hir::BinOpKind::Sub
79+
| hir::BinOpKind::Mul
80+
| hir::BinOpKind::Div
81+
| hir::BinOpKind::Rem
82+
| hir::BinOpKind::Shl
83+
| hir::BinOpKind::Shr
84+
) = op.node else {
85+
return;
86+
};
87+
if self.is_allowed_ty(cx, lhs) || self.is_allowed_ty(cx, rhs) {
88+
return;
89+
}
90+
let lhs_refs = cx.typeck_results().expr_ty(lhs).peel_refs();
91+
let rhs_refs = cx.typeck_results().expr_ty(rhs).peel_refs();
92+
if Self::is_literal_integer(lhs, lhs_refs) || Self::is_literal_integer(rhs, rhs_refs) {
93+
return;
94+
}
95+
self.issue_lint(cx, expr);
96+
}
5397
}
5498

5599
impl<'tcx> LateLintPass<'tcx> for Arithmetic {
56100
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
57-
if self.expr_span.is_some() {
58-
return;
59-
}
60-
if let Some(span) = self.const_span && span.contains(expr.span) {
101+
if self.expr_span.is_some() || self.const_span.map_or(false, |sp| sp.contains(expr.span)) {
61102
return;
62103
}
63104
match &expr.kind {
64105
hir::ExprKind::Binary(op, lhs, rhs) | hir::ExprKind::AssignOp(op, lhs, rhs) => {
65-
let (
66-
hir::BinOpKind::Add
67-
| hir::BinOpKind::Sub
68-
| hir::BinOpKind::Mul
69-
| hir::BinOpKind::Div
70-
| hir::BinOpKind::Rem
71-
| hir::BinOpKind::Shl
72-
| hir::BinOpKind::Shr
73-
) = op.node else {
74-
return;
75-
};
76-
if self.is_allowed_ty(cx, lhs) || self.is_allowed_ty(cx, rhs) {
77-
return;
78-
}
79-
self.issue_lint(cx, expr);
106+
self.manage_bin_ops(cx, expr, op, lhs, rhs);
80107
},
81108
hir::ExprKind::Unary(hir::UnOp::Neg, _) => {
82-
// CTFE already takes care of things like `-1` that do not overflow.
83109
if constant_simple(cx, cx.typeck_results(), expr).is_none() {
84110
self.issue_lint(cx, expr);
85111
}
@@ -89,16 +115,15 @@ impl<'tcx> LateLintPass<'tcx> for Arithmetic {
89115
}
90116

91117
fn check_body(&mut self, cx: &LateContext<'_>, body: &hir::Body<'_>) {
92-
let body_owner = cx.tcx.hir().body_owner_def_id(body.id());
93-
match cx.tcx.hir().body_owner_kind(body_owner) {
94-
hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(_) => {
95-
let body_span = cx.tcx.def_span(body_owner);
96-
if let Some(span) = self.const_span && span.contains(body_span) {
97-
return;
98-
}
99-
self.const_span = Some(body_span);
100-
},
101-
hir::BodyOwnerKind::Closure | hir::BodyOwnerKind::Fn => {},
118+
let body_owner = cx.tcx.hir().body_owner(body.id());
119+
let body_owner_def_id = cx.tcx.hir().local_def_id(body_owner);
120+
let body_owner_kind = cx.tcx.hir().body_owner_kind(body_owner_def_id);
121+
if let hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(_) = body_owner_kind {
122+
let body_span = cx.tcx.hir().span_with_body(body_owner);
123+
if let Some(span) = self.const_span && span.contains(body_span) {
124+
return;
125+
}
126+
self.const_span = Some(body_span);
102127
}
103128
}
104129

‎tests/ui/arithmetic.fixed

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// run-rustfix
22

3-
#![allow(clippy::unnecessary_owned_empty_strings)]
4-
#![feature(saturating_int_impl)]
3+
#![allow(clippy::assign_op_pattern, clippy::unnecessary_owned_empty_strings)]
4+
#![feature(inline_const, saturating_int_impl)]
55
#![warn(clippy::arithmetic)]
66

77
use core::num::{Saturating, Wrapping};
@@ -24,4 +24,23 @@ pub fn hard_coded_allowed() {
2424
let _ = inferred_wrapping + inferred_wrapping;
2525
}
2626

27+
#[rustfmt::skip]
28+
pub fn non_overflowing_ops() {
29+
const _A: i32 = { let mut n = 1; n += 1; n };
30+
let mut _b = 1; _b += 1;
31+
let _c = const { let mut n = 1; n += 1; n };
32+
33+
const _D: i32 = { let mut n = 1; n = n + 1; n };
34+
let mut _e = 1; _e = _e + 1;
35+
let _f = const { let mut n = 1; n = n + 1; n };
36+
37+
const _G: i32 = { let mut n = 1; n = 1 + n; n };
38+
let mut _h = 1; _h = 1 + _h;
39+
let _i = const { let mut n = 1; n = 1 + n; n };
40+
41+
const _J: i32 = 1 + 1;
42+
let _k = 1 + 1;
43+
let _l = const { 1 + 1 };
44+
}
45+
2746
fn main() {}

‎tests/ui/arithmetic.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// run-rustfix
22

3-
#![allow(clippy::unnecessary_owned_empty_strings)]
4-
#![feature(saturating_int_impl)]
3+
#![allow(clippy::assign_op_pattern, clippy::unnecessary_owned_empty_strings)]
4+
#![feature(inline_const, saturating_int_impl)]
55
#![warn(clippy::arithmetic)]
66

77
use core::num::{Saturating, Wrapping};
@@ -24,4 +24,23 @@ pub fn hard_coded_allowed() {
2424
let _ = inferred_wrapping + inferred_wrapping;
2525
}
2626

27+
#[rustfmt::skip]
28+
pub fn non_overflowing_ops() {
29+
const _A: i32 = { let mut n = 1; n += 1; n };
30+
let mut _b = 1; _b += 1;
31+
let _c = const { let mut n = 1; n += 1; n };
32+
33+
const _D: i32 = { let mut n = 1; n = n + 1; n };
34+
let mut _e = 1; _e = _e + 1;
35+
let _f = const { let mut n = 1; n = n + 1; n };
36+
37+
const _G: i32 = { let mut n = 1; n = 1 + n; n };
38+
let mut _h = 1; _h = 1 + _h;
39+
let _i = const { let mut n = 1; n = 1 + n; n };
40+
41+
const _J: i32 = 1 + 1;
42+
let _k = 1 + 1;
43+
let _l = const { 1 + 1 };
44+
}
45+
2746
fn main() {}

0 commit comments

Comments
 (0)