diff --git a/clippy_lints/src/operators/arithmetic.rs b/clippy_lints/src/operators/arithmetic.rs
index 800cf249f5c..27eef92deef 100644
--- a/clippy_lints/src/operators/arithmetic.rs
+++ b/clippy_lints/src/operators/arithmetic.rs
@@ -5,13 +5,21 @@
use super::ARITHMETIC;
use clippy_utils::{consts::constant_simple, diagnostics::span_lint};
+use rustc_ast as ast;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir as hir;
use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::Ty;
use rustc_session::impl_lint_pass;
-use rustc_span::source_map::Span;
+use rustc_span::source_map::{Span, Spanned};
-const HARD_CODED_ALLOWED: &[&str] = &["std::num::Saturating", "std::string::String", "std::num::Wrapping"];
+const HARD_CODED_ALLOWED: &[&str] = &[
+ "f32",
+ "f64",
+ "std::num::Saturating",
+ "std::string::String",
+ "std::num::Wrapping",
+];
#[derive(Debug)]
pub struct Arithmetic {
@@ -34,6 +42,27 @@ impl Arithmetic {
}
}
+ /// Checks assign operators (+=, -=, *=, /=) of integers in a non-constant environment that
+ /// won't overflow.
+ fn has_valid_assign_op(op: &Spanned<hir::BinOpKind>, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool {
+ if !Self::is_literal_integer(rhs, rhs_refs) {
+ return false;
+ }
+ if let hir::BinOpKind::Div | hir::BinOpKind::Mul = op.node
+ && let hir::ExprKind::Lit(ref lit) = rhs.kind
+ && let ast::LitKind::Int(1, _) = lit.node
+ {
+ return true;
+ }
+ false
+ }
+
+ /// Checks "raw" binary operators (+, -, *, /) of integers in a non-constant environment
+ /// already handled by the CTFE.
+ fn has_valid_bin_op(lhs: &hir::Expr<'_>, lhs_refs: Ty<'_>, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool {
+ Self::is_literal_integer(lhs, lhs_refs) && Self::is_literal_integer(rhs, rhs_refs)
+ }
+
/// Checks if the given `expr` has any of the inner `allowed` elements.
fn is_allowed_ty(&self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
self.allowed.contains(
@@ -46,40 +75,66 @@ impl Arithmetic {
)
}
+ /// Explicit integers like `1` or `i32::MAX`. Does not take into consideration references.
+ fn is_literal_integer(expr: &hir::Expr<'_>, expr_refs: Ty<'_>) -> bool {
+ let is_integral = expr_refs.is_integral();
+ let is_literal = matches!(expr.kind, hir::ExprKind::Lit(_));
+ is_integral && is_literal
+ }
+
fn issue_lint(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) {
span_lint(cx, ARITHMETIC, expr.span, "arithmetic detected");
self.expr_span = Some(expr.span);
}
+
+ /// Manages when the lint should be triggered. Operations in constant environments, hard coded
+ /// types, custom allowed types and non-constant operations that won't overflow are ignored.
+ fn manage_bin_ops(
+ &mut self,
+ cx: &LateContext<'_>,
+ expr: &hir::Expr<'_>,
+ op: &Spanned<hir::BinOpKind>,
+ lhs: &hir::Expr<'_>,
+ rhs: &hir::Expr<'_>,
+ ) {
+ if constant_simple(cx, cx.typeck_results(), expr).is_some() {
+ return;
+ }
+ if !matches!(
+ op.node,
+ hir::BinOpKind::Add
+ | hir::BinOpKind::Sub
+ | hir::BinOpKind::Mul
+ | hir::BinOpKind::Div
+ | hir::BinOpKind::Rem
+ | hir::BinOpKind::Shl
+ | hir::BinOpKind::Shr
+ ) {
+ return;
+ };
+ if self.is_allowed_ty(cx, lhs) || self.is_allowed_ty(cx, rhs) {
+ return;
+ }
+ let lhs_refs = cx.typeck_results().expr_ty(lhs).peel_refs();
+ let rhs_refs = cx.typeck_results().expr_ty(rhs).peel_refs();
+ let has_valid_assign_op = Self::has_valid_assign_op(op, rhs, rhs_refs);
+ if has_valid_assign_op || Self::has_valid_bin_op(lhs, lhs_refs, rhs, rhs_refs) {
+ return;
+ }
+ self.issue_lint(cx, expr);
+ }
}
impl<'tcx> LateLintPass<'tcx> for Arithmetic {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
- if self.expr_span.is_some() {
- return;
- }
- if let Some(span) = self.const_span && span.contains(expr.span) {
+ if self.expr_span.is_some() || self.const_span.map_or(false, |sp| sp.contains(expr.span)) {
return;
}
match &expr.kind {
hir::ExprKind::Binary(op, lhs, rhs) | hir::ExprKind::AssignOp(op, lhs, rhs) => {
- let (
- hir::BinOpKind::Add
- | hir::BinOpKind::Sub
- | hir::BinOpKind::Mul
- | hir::BinOpKind::Div
- | hir::BinOpKind::Rem
- | hir::BinOpKind::Shl
- | hir::BinOpKind::Shr
- ) = op.node else {
- return;
- };
- if self.is_allowed_ty(cx, lhs) || self.is_allowed_ty(cx, rhs) {
- return;
- }
- self.issue_lint(cx, expr);
+ self.manage_bin_ops(cx, expr, op, lhs, rhs);
},
hir::ExprKind::Unary(hir::UnOp::Neg, _) => {
- // CTFE already takes care of things like `-1` that do not overflow.
if constant_simple(cx, cx.typeck_results(), expr).is_none() {
self.issue_lint(cx, expr);
}
@@ -89,16 +144,15 @@ impl<'tcx> LateLintPass<'tcx> for Arithmetic {
}
fn check_body(&mut self, cx: &LateContext<'_>, body: &hir::Body<'_>) {
- let body_owner = cx.tcx.hir().body_owner_def_id(body.id());
- match cx.tcx.hir().body_owner_kind(body_owner) {
- hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(_) => {
- let body_span = cx.tcx.def_span(body_owner);
- if let Some(span) = self.const_span && span.contains(body_span) {
- return;
- }
- self.const_span = Some(body_span);
- },
- hir::BodyOwnerKind::Closure | hir::BodyOwnerKind::Fn => {},
+ let body_owner = cx.tcx.hir().body_owner(body.id());
+ let body_owner_def_id = cx.tcx.hir().local_def_id(body_owner);
+ let body_owner_kind = cx.tcx.hir().body_owner_kind(body_owner_def_id);
+ if let hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(_) = body_owner_kind {
+ let body_span = cx.tcx.hir().span_with_body(body_owner);
+ if let Some(span) = self.const_span && span.contains(body_span) {
+ return;
+ }
+ self.const_span = Some(body_span);
}
}
diff --git a/clippy_lints/src/operators/mod.rs b/clippy_lints/src/operators/mod.rs
index bb6d99406b4..c7b0633b79c 100644
--- a/clippy_lints/src/operators/mod.rs
+++ b/clippy_lints/src/operators/mod.rs
@@ -61,25 +61,29 @@ declare_clippy_lint! {
declare_clippy_lint! {
/// ### What it does
- /// Checks for any kind of arithmetic operation of any type.
+ /// Checks any kind of arithmetic operation of any type.
///
/// Operators like `+`, `-`, `*` or `<<` are usually capable of overflowing according to the [Rust
/// Reference](https://doc.rust-lang.org/reference/expressions/operator-expr.html#overflow),
- /// or can panic (`/`, `%`). Known safe built-in types like `Wrapping` or `Saturing` are filtered
- /// away.
+ /// or can panic (`/`, `%`).
+ ///
+ /// Known safe built-in types like `Wrapping` or `Saturing`, floats, operations in constant
+ /// environments, allowed types and non-constant operations that won't overflow are ignored.
///
/// ### Why is this bad?
- /// Integer overflow will trigger a panic in debug builds or will wrap in
- /// release mode. Division by zero will cause a panic in either mode. In some applications one
- /// wants explicitly checked, wrapping or saturating arithmetic.
+ /// For integers, overflow will trigger a panic in debug builds or wrap the result in
+ /// release mode; division by zero will cause a panic in either mode. As a result, it is
+ /// desirable to explicitly call checked, wrapping or saturating arithmetic methods.
///
/// #### Example
/// ```rust
- /// # let a = 0;
- /// a + 1;
+ /// // `n` can be any number, including `i32::MAX`.
+ /// fn foo(n: i32) -> i32 {
+ /// n + 1
+ /// }
/// ```
///
- /// Third-party types also tend to overflow.
+ /// Third-party types can also overflow or present unwanted side-effects.
///
/// #### Example
/// ```ignore,rust
diff --git a/src/docs/arithmetic.txt b/src/docs/arithmetic.txt
index 0b3f07d9505..f04125060fb 100644
--- a/src/docs/arithmetic.txt
+++ b/src/docs/arithmetic.txt
@@ -1,22 +1,27 @@
### What it does
-Checks for any kind of arithmetic operation of any type.
+Checks any kind of arithmetic operation of any type.
Operators like `+`, `-`, `*` or `<<` are usually capable of overflowing according to the [Rust
Reference](https://doc.rust-lang.org/reference/expressions/operator-expr.html#overflow),
-or can panic (`/`, `%`). Known safe built-in types like `Wrapping` or `Saturing` are filtered
-away.
+or can panic (`/`, `%`).
+
+Known safe built-in types like `Wrapping` or `Saturing`, floats, operations in constant
+environments, allowed types and non-constant operations that won't overflow are ignored.
### Why is this bad?
-Integer overflow will trigger a panic in debug builds or will wrap in
-release mode. Division by zero will cause a panic in either mode. In some applications one
-wants explicitly checked, wrapping or saturating arithmetic.
+For integers, overflow will trigger a panic in debug builds or wrap the result in
+release mode; division by zero will cause a panic in either mode. As a result, it is
+desirable to explicitly call checked, wrapping or saturating arithmetic methods.
#### Example
```
-a + 1;
+// `n` can be any number, including `i32::MAX`.
+fn foo(n: i32) -> i32 {
+ n + 1
+}
```
-Third-party types also tend to overflow.
+Third-party types can also overflow or present unwanted side-effects.
#### Example
```
diff --git a/tests/ui/arithmetic.fixed b/tests/ui/arithmetic.fixed
deleted file mode 100644
index a2a1c4394c2..00000000000
--- a/tests/ui/arithmetic.fixed
+++ /dev/null
@@ -1,27 +0,0 @@
-// run-rustfix
-
-#![allow(clippy::unnecessary_owned_empty_strings)]
-#![feature(saturating_int_impl)]
-#![warn(clippy::arithmetic)]
-
-use core::num::{Saturating, Wrapping};
-
-pub fn hard_coded_allowed() {
- let _ = Saturating(0u32) + Saturating(0u32);
- let _ = String::new() + "";
- let _ = Wrapping(0u32) + Wrapping(0u32);
-
- let saturating: Saturating<u32> = Saturating(0u32);
- let string: String = String::new();
- let wrapping: Wrapping<u32> = Wrapping(0u32);
-
- let inferred_saturating = saturating + saturating;
- let inferred_string = string + "";
- let inferred_wrapping = wrapping + wrapping;
-
- let _ = inferred_saturating + inferred_saturating;
- let _ = inferred_string + "";
- let _ = inferred_wrapping + inferred_wrapping;
-}
-
-fn main() {}
diff --git a/tests/ui/arithmetic.rs b/tests/ui/arithmetic.rs
index a2a1c4394c2..a9ac46e9a19 100644
--- a/tests/ui/arithmetic.rs
+++ b/tests/ui/arithmetic.rs
@@ -1,12 +1,13 @@
-// run-rustfix
-
-#![allow(clippy::unnecessary_owned_empty_strings)]
-#![feature(saturating_int_impl)]
+#![allow(clippy::assign_op_pattern, clippy::unnecessary_owned_empty_strings)]
+#![feature(inline_const, saturating_int_impl)]
#![warn(clippy::arithmetic)]
use core::num::{Saturating, Wrapping};
pub fn hard_coded_allowed() {
+ let _ = 1f32 + 1f32;
+ let _ = 1f64 + 1f64;
+
let _ = Saturating(0u32) + Saturating(0u32);
let _ = String::new() + "";
let _ = Wrapping(0u32) + Wrapping(0u32);
@@ -24,4 +25,33 @@ pub fn hard_coded_allowed() {
let _ = inferred_wrapping + inferred_wrapping;
}
+#[rustfmt::skip]
+pub fn non_overflowing_ops() {
+ const _: i32 = { let mut n = 1; n += 1; n };
+ let _ = const { let mut n = 1; n += 1; n };
+
+ const _: i32 = { let mut n = 1; n = n + 1; n };
+ let _ = const { let mut n = 1; n = n + 1; n };
+
+ const _: i32 = { let mut n = 1; n = 1 + n; n };
+ let _ = const { let mut n = 1; n = 1 + n; n };
+
+ const _: i32 = 1 + 1;
+ let _ = 1 + 1;
+ let _ = const { 1 + 1 };
+
+ let mut _a = 1;
+ _a *= 1;
+ _a /= 1;
+}
+
+#[rustfmt::skip]
+pub fn overflowing_ops() {
+ let mut _a = 1; _a += 1;
+
+ let mut _b = 1; _b = _b + 1;
+
+ let mut _c = 1; _c = 1 + _c;
+}
+
fn main() {}
diff --git a/tests/ui/arithmetic.stderr b/tests/ui/arithmetic.stderr
new file mode 100644
index 00000000000..a51cf6ba600
--- /dev/null
+++ b/tests/ui/arithmetic.stderr
@@ -0,0 +1,22 @@
+error: arithmetic detected
+ --> $DIR/arithmetic.rs:50:21
+ |
+LL | let mut _a = 1; _a += 1;
+ | ^^^^^^^
+ |
+ = note: `-D clippy::arithmetic` implied by `-D warnings`
+
+error: arithmetic detected
+ --> $DIR/arithmetic.rs:52:26
+ |
+LL | let mut _b = 1; _b = _b + 1;
+ | ^^^^^^
+
+error: arithmetic detected
+ --> $DIR/arithmetic.rs:54:26
+ |
+LL | let mut _c = 1; _c = 1 + _c;
+ | ^^^^^^
+
+error: aborting due to 3 previous errors
+