about summary refs log tree commit diff
diff options
context:
space:
mode:
authory21 <30553356+y21@users.noreply.github.com>2023-11-16 17:30:10 +0100
committery21 <30553356+y21@users.noreply.github.com>2023-11-16 17:30:10 +0100
commit7696c9d1d506933dd4da46115dd0fd713fed6df4 (patch)
tree7f72fa4cf2c3fd2090bb0279b6fb59feed8714be
parent1e0597cb68134ae221765deb3f3e9dd6d66f7dcc (diff)
downloadrust-7696c9d1d506933dd4da46115dd0fd713fed6df4.tar.gz
rust-7696c9d1d506933dd4da46115dd0fd713fed6df4.zip
allow more div and rem operations that can be checked
-rw-r--r--clippy_utils/src/eager_or_lazy.rs31
-rw-r--r--tests/ui/unnecessary_lazy_eval.fixed24
-rw-r--r--tests/ui/unnecessary_lazy_eval.rs24
-rw-r--r--tests/ui/unnecessary_lazy_eval.stderr90
4 files changed, 124 insertions, 45 deletions
diff --git a/clippy_utils/src/eager_or_lazy.rs b/clippy_utils/src/eager_or_lazy.rs
index 371fc029701..f7a8bd131eb 100644
--- a/clippy_utils/src/eager_or_lazy.rs
+++ b/clippy_utils/src/eager_or_lazy.rs
@@ -9,7 +9,7 @@
 //!  - or-fun-call
 //!  - option-if-let-else
 
-use crate::consts::constant;
+use crate::consts::{constant, FullInt};
 use crate::ty::{all_predicates_of, is_copy};
 use crate::visitors::is_const_evaluatable;
 use rustc_hir::def::{DefKind, Res};
@@ -227,15 +227,34 @@ fn expr_eagerness<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessS
                     self.eagerness |= NoChange;
                 },
 
-                // Similar to `>>` and `<<`, we only want to avoid linting entirely if both sides are unknown and the
+                ExprKind::Binary(op, left, right)
+                    if matches!(op.node, BinOpKind::Div | BinOpKind::Rem)
+                        && let left_ty = self.cx.typeck_results().expr_ty(left)
+                        && let right_ty = self.cx.typeck_results().expr_ty(right)
+                        && let left = constant(self.cx, self.cx.typeck_results(), left)
+                            .and_then(|c| c.int_value(self.cx, left_ty))
+                        && let right = constant(self.cx, self.cx.typeck_results(), right)
+                            .and_then(|c| c.int_value(self.cx, right_ty))
+                        && match (left, right) {
+                            // `1 / x` -- x might be zero
+                            (_, None) => true,
+                            // `x / -1` -- x might be T::MIN = panic
+                            #[expect(clippy::match_same_arms)]
+                            (None, Some(FullInt::S(-1))) => true,
+                            // anything else is either fine or checked by the compiler
+                            _ => false,
+                        } =>
+                {
+                    self.eagerness |= NoChange;
+                },
+
+                // Similar to `>>` and `<<`, we only want to avoid linting entirely if either side is unknown and the
                 // compiler can't emit an error for an overflowing expression.
                 // Suggesting eagerness for `true.then(|| i32::MAX + 1)` is okay because the compiler will emit an
                 // error and it's good to have the eagerness warning up front when the user fixes the logic error.
                 ExprKind::Binary(op, left, right)
-                    if matches!(
-                        op.node,
-                        BinOpKind::Add | BinOpKind::Sub | BinOpKind::Mul | BinOpKind::Div | BinOpKind::Rem
-                    ) && !self.cx.typeck_results().expr_ty(e).is_floating_point()
+                    if matches!(op.node, BinOpKind::Add | BinOpKind::Sub | BinOpKind::Mul)
+                        && !self.cx.typeck_results().expr_ty(e).is_floating_point()
                         && (constant(self.cx, self.cx.typeck_results(), left).is_none()
                             || constant(self.cx, self.cx.typeck_results(), right).is_none()) =>
                 {
diff --git a/tests/ui/unnecessary_lazy_eval.fixed b/tests/ui/unnecessary_lazy_eval.fixed
index 07b43d3cc1d..66598f89208 100644
--- a/tests/ui/unnecessary_lazy_eval.fixed
+++ b/tests/ui/unnecessary_lazy_eval.fixed
@@ -201,11 +201,7 @@ fn issue9422(x: usize) -> Option<usize> {
 fn panicky_arithmetic_ops(x: usize, y: isize) {
     #![allow(clippy::identity_op, clippy::eq_op)]
 
-    // Even though some of these expressions overflow, they're entirely dependent on constants.
-    // So, the compiler already emits a warning about overflowing expressions.
-    // It's a logic error and we want both warnings up front.
-    // ONLY when a binop side that "matters" for overflow (for `>>`, that is always the right side and
-    // never the left side) has a non-constant value, avoid linting
+    // See comments in `eager_or_lazy.rs` for the rules that this is meant to follow
 
     let _x = false.then_some(i32::MAX + 1);
     //~^ ERROR: unnecessary closure used with `bool::then`
@@ -224,8 +220,6 @@ fn panicky_arithmetic_ops(x: usize, y: isize) {
     let _x = false.then_some(255u8 >> 8);
     //~^ ERROR: unnecessary closure used with `bool::then`
     let _x = false.then(|| 255u8 >> x);
-    let _x = false.then_some(i32::MIN / -1);
-    //~^ ERROR: unnecessary closure used with `bool::then`
     let _x = false.then_some(i32::MAX + -1);
     //~^ ERROR: unnecessary closure used with `bool::then`
     let _x = false.then_some(-i32::MAX);
@@ -251,6 +245,22 @@ fn panicky_arithmetic_ops(x: usize, y: isize) {
     let _x = false.then(|| x + 1);
     let _x = false.then(|| 1 + x);
 
+    let _x = false.then_some(x / 0);
+    //~^ ERROR: unnecessary closure used with `bool::then`
+    let _x = false.then_some(x % 0);
+    //~^ ERROR: unnecessary closure used with `bool::then`
+    let _x = false.then(|| y / -1);
+    let _x = false.then_some(1 / -1);
+    //~^ ERROR: unnecessary closure used with `bool::then`
+    let _x = false.then_some(i32::MIN / -1);
+    //~^ ERROR: unnecessary closure used with `bool::then`
+    let _x = false.then(|| i32::MIN / x as i32);
+    let _x = false.then_some(i32::MIN / 0);
+    //~^ ERROR: unnecessary closure used with `bool::then`
+    let _x = false.then_some(4 / 2);
+    //~^ ERROR: unnecessary closure used with `bool::then`
+    let _x = false.then(|| 1 / x);
+
     // const eval doesn't read variables, but floating point math never panics, so we can still emit a
     // warning
     let f1 = 1.0;
diff --git a/tests/ui/unnecessary_lazy_eval.rs b/tests/ui/unnecessary_lazy_eval.rs
index 232a94d9638..5045fcd790e 100644
--- a/tests/ui/unnecessary_lazy_eval.rs
+++ b/tests/ui/unnecessary_lazy_eval.rs
@@ -201,11 +201,7 @@ fn issue9422(x: usize) -> Option<usize> {
 fn panicky_arithmetic_ops(x: usize, y: isize) {
     #![allow(clippy::identity_op, clippy::eq_op)]
 
-    // Even though some of these expressions overflow, they're entirely dependent on constants.
-    // So, the compiler already emits a warning about overflowing expressions.
-    // It's a logic error and we want both warnings up front.
-    // ONLY when a binop side that "matters" for overflow (for `>>`, that is always the right side and
-    // never the left side) has a non-constant value, avoid linting
+    // See comments in `eager_or_lazy.rs` for the rules that this is meant to follow
 
     let _x = false.then(|| i32::MAX + 1);
     //~^ ERROR: unnecessary closure used with `bool::then`
@@ -224,8 +220,6 @@ fn panicky_arithmetic_ops(x: usize, y: isize) {
     let _x = false.then(|| 255u8 >> 8);
     //~^ ERROR: unnecessary closure used with `bool::then`
     let _x = false.then(|| 255u8 >> x);
-    let _x = false.then(|| i32::MIN / -1);
-    //~^ ERROR: unnecessary closure used with `bool::then`
     let _x = false.then(|| i32::MAX + -1);
     //~^ ERROR: unnecessary closure used with `bool::then`
     let _x = false.then(|| -i32::MAX);
@@ -251,6 +245,22 @@ fn panicky_arithmetic_ops(x: usize, y: isize) {
     let _x = false.then(|| x + 1);
     let _x = false.then(|| 1 + x);
 
+    let _x = false.then(|| x / 0);
+    //~^ ERROR: unnecessary closure used with `bool::then`
+    let _x = false.then(|| x % 0);
+    //~^ ERROR: unnecessary closure used with `bool::then`
+    let _x = false.then(|| y / -1);
+    let _x = false.then(|| 1 / -1);
+    //~^ ERROR: unnecessary closure used with `bool::then`
+    let _x = false.then(|| i32::MIN / -1);
+    //~^ ERROR: unnecessary closure used with `bool::then`
+    let _x = false.then(|| i32::MIN / x as i32);
+    let _x = false.then(|| i32::MIN / 0);
+    //~^ ERROR: unnecessary closure used with `bool::then`
+    let _x = false.then(|| 4 / 2);
+    //~^ ERROR: unnecessary closure used with `bool::then`
+    let _x = false.then(|| 1 / x);
+
     // const eval doesn't read variables, but floating point math never panics, so we can still emit a
     // warning
     let f1 = 1.0;
diff --git a/tests/ui/unnecessary_lazy_eval.stderr b/tests/ui/unnecessary_lazy_eval.stderr
index 6c7edb24fae..466664aee6c 100644
--- a/tests/ui/unnecessary_lazy_eval.stderr
+++ b/tests/ui/unnecessary_lazy_eval.stderr
@@ -329,7 +329,7 @@ LL | |     or_else(|_| Ok(ext_str.some_field));
    |       help: use `or(..)` instead: `or(Ok(ext_str.some_field))`
 
 error: unnecessary closure used with `bool::then`
-  --> $DIR/unnecessary_lazy_eval.rs:210:14
+  --> $DIR/unnecessary_lazy_eval.rs:206:14
    |
 LL |     let _x = false.then(|| i32::MAX + 1);
    |              ^^^^^^---------------------
@@ -337,7 +337,7 @@ LL |     let _x = false.then(|| i32::MAX + 1);
    |                    help: use `then_some(..)` instead: `then_some(i32::MAX + 1)`
 
 error: unnecessary closure used with `bool::then`
-  --> $DIR/unnecessary_lazy_eval.rs:212:14
+  --> $DIR/unnecessary_lazy_eval.rs:208:14
    |
 LL |     let _x = false.then(|| i32::MAX * 2);
    |              ^^^^^^---------------------
@@ -345,7 +345,7 @@ LL |     let _x = false.then(|| i32::MAX * 2);
    |                    help: use `then_some(..)` instead: `then_some(i32::MAX * 2)`
 
 error: unnecessary closure used with `bool::then`
-  --> $DIR/unnecessary_lazy_eval.rs:214:14
+  --> $DIR/unnecessary_lazy_eval.rs:210:14
    |
 LL |     let _x = false.then(|| i32::MAX - 1);
    |              ^^^^^^---------------------
@@ -353,7 +353,7 @@ LL |     let _x = false.then(|| i32::MAX - 1);
    |                    help: use `then_some(..)` instead: `then_some(i32::MAX - 1)`
 
 error: unnecessary closure used with `bool::then`
-  --> $DIR/unnecessary_lazy_eval.rs:216:14
+  --> $DIR/unnecessary_lazy_eval.rs:212:14
    |
 LL |     let _x = false.then(|| i32::MIN - 1);
    |              ^^^^^^---------------------
@@ -361,7 +361,7 @@ LL |     let _x = false.then(|| i32::MIN - 1);
    |                    help: use `then_some(..)` instead: `then_some(i32::MIN - 1)`
 
 error: unnecessary closure used with `bool::then`
-  --> $DIR/unnecessary_lazy_eval.rs:218:14
+  --> $DIR/unnecessary_lazy_eval.rs:214:14
    |
 LL |     let _x = false.then(|| (1 + 2 * 3 - 2 / 3 + 9) << 2);
    |              ^^^^^^-------------------------------------
@@ -369,7 +369,7 @@ LL |     let _x = false.then(|| (1 + 2 * 3 - 2 / 3 + 9) << 2);
    |                    help: use `then_some(..)` instead: `then_some((1 + 2 * 3 - 2 / 3 + 9) << 2)`
 
 error: unnecessary closure used with `bool::then`
-  --> $DIR/unnecessary_lazy_eval.rs:220:14
+  --> $DIR/unnecessary_lazy_eval.rs:216:14
    |
 LL |     let _x = false.then(|| 255u8 << 7);
    |              ^^^^^^-------------------
@@ -377,7 +377,7 @@ LL |     let _x = false.then(|| 255u8 << 7);
    |                    help: use `then_some(..)` instead: `then_some(255u8 << 7)`
 
 error: unnecessary closure used with `bool::then`
-  --> $DIR/unnecessary_lazy_eval.rs:222:14
+  --> $DIR/unnecessary_lazy_eval.rs:218:14
    |
 LL |     let _x = false.then(|| 255u8 << 8);
    |              ^^^^^^-------------------
@@ -385,7 +385,7 @@ LL |     let _x = false.then(|| 255u8 << 8);
    |                    help: use `then_some(..)` instead: `then_some(255u8 << 8)`
 
 error: unnecessary closure used with `bool::then`
-  --> $DIR/unnecessary_lazy_eval.rs:224:14
+  --> $DIR/unnecessary_lazy_eval.rs:220:14
    |
 LL |     let _x = false.then(|| 255u8 >> 8);
    |              ^^^^^^-------------------
@@ -393,15 +393,7 @@ LL |     let _x = false.then(|| 255u8 >> 8);
    |                    help: use `then_some(..)` instead: `then_some(255u8 >> 8)`
 
 error: unnecessary closure used with `bool::then`
-  --> $DIR/unnecessary_lazy_eval.rs:227:14
-   |
-LL |     let _x = false.then(|| i32::MIN / -1);
-   |              ^^^^^^----------------------
-   |                    |
-   |                    help: use `then_some(..)` instead: `then_some(i32::MIN / -1)`
-
-error: unnecessary closure used with `bool::then`
-  --> $DIR/unnecessary_lazy_eval.rs:229:14
+  --> $DIR/unnecessary_lazy_eval.rs:223:14
    |
 LL |     let _x = false.then(|| i32::MAX + -1);
    |              ^^^^^^----------------------
@@ -409,7 +401,7 @@ LL |     let _x = false.then(|| i32::MAX + -1);
    |                    help: use `then_some(..)` instead: `then_some(i32::MAX + -1)`
 
 error: unnecessary closure used with `bool::then`
-  --> $DIR/unnecessary_lazy_eval.rs:231:14
+  --> $DIR/unnecessary_lazy_eval.rs:225:14
    |
 LL |     let _x = false.then(|| -i32::MAX);
    |              ^^^^^^------------------
@@ -417,7 +409,7 @@ LL |     let _x = false.then(|| -i32::MAX);
    |                    help: use `then_some(..)` instead: `then_some(-i32::MAX)`
 
 error: unnecessary closure used with `bool::then`
-  --> $DIR/unnecessary_lazy_eval.rs:233:14
+  --> $DIR/unnecessary_lazy_eval.rs:227:14
    |
 LL |     let _x = false.then(|| -i32::MIN);
    |              ^^^^^^------------------
@@ -425,7 +417,7 @@ LL |     let _x = false.then(|| -i32::MIN);
    |                    help: use `then_some(..)` instead: `then_some(-i32::MIN)`
 
 error: unnecessary closure used with `bool::then`
-  --> $DIR/unnecessary_lazy_eval.rs:236:14
+  --> $DIR/unnecessary_lazy_eval.rs:230:14
    |
 LL |     let _x = false.then(|| 255 >> -7);
    |              ^^^^^^------------------
@@ -433,7 +425,7 @@ LL |     let _x = false.then(|| 255 >> -7);
    |                    help: use `then_some(..)` instead: `then_some(255 >> -7)`
 
 error: unnecessary closure used with `bool::then`
-  --> $DIR/unnecessary_lazy_eval.rs:238:14
+  --> $DIR/unnecessary_lazy_eval.rs:232:14
    |
 LL |     let _x = false.then(|| 255 << -1);
    |              ^^^^^^------------------
@@ -441,7 +433,7 @@ LL |     let _x = false.then(|| 255 << -1);
    |                    help: use `then_some(..)` instead: `then_some(255 << -1)`
 
 error: unnecessary closure used with `bool::then`
-  --> $DIR/unnecessary_lazy_eval.rs:240:14
+  --> $DIR/unnecessary_lazy_eval.rs:234:14
    |
 LL |     let _x = false.then(|| 1 / 0);
    |              ^^^^^^--------------
@@ -449,7 +441,7 @@ LL |     let _x = false.then(|| 1 / 0);
    |                    help: use `then_some(..)` instead: `then_some(1 / 0)`
 
 error: unnecessary closure used with `bool::then`
-  --> $DIR/unnecessary_lazy_eval.rs:242:14
+  --> $DIR/unnecessary_lazy_eval.rs:236:14
    |
 LL |     let _x = false.then(|| x << -1);
    |              ^^^^^^----------------
@@ -457,7 +449,7 @@ LL |     let _x = false.then(|| x << -1);
    |                    help: use `then_some(..)` instead: `then_some(x << -1)`
 
 error: unnecessary closure used with `bool::then`
-  --> $DIR/unnecessary_lazy_eval.rs:244:14
+  --> $DIR/unnecessary_lazy_eval.rs:238:14
    |
 LL |     let _x = false.then(|| x << 2);
    |              ^^^^^^---------------
@@ -465,12 +457,60 @@ LL |     let _x = false.then(|| x << 2);
    |                    help: use `then_some(..)` instead: `then_some(x << 2)`
 
 error: unnecessary closure used with `bool::then`
+  --> $DIR/unnecessary_lazy_eval.rs:248:14
+   |
+LL |     let _x = false.then(|| x / 0);
+   |              ^^^^^^--------------
+   |                    |
+   |                    help: use `then_some(..)` instead: `then_some(x / 0)`
+
+error: unnecessary closure used with `bool::then`
+  --> $DIR/unnecessary_lazy_eval.rs:250:14
+   |
+LL |     let _x = false.then(|| x % 0);
+   |              ^^^^^^--------------
+   |                    |
+   |                    help: use `then_some(..)` instead: `then_some(x % 0)`
+
+error: unnecessary closure used with `bool::then`
+  --> $DIR/unnecessary_lazy_eval.rs:253:14
+   |
+LL |     let _x = false.then(|| 1 / -1);
+   |              ^^^^^^---------------
+   |                    |
+   |                    help: use `then_some(..)` instead: `then_some(1 / -1)`
+
+error: unnecessary closure used with `bool::then`
+  --> $DIR/unnecessary_lazy_eval.rs:255:14
+   |
+LL |     let _x = false.then(|| i32::MIN / -1);
+   |              ^^^^^^----------------------
+   |                    |
+   |                    help: use `then_some(..)` instead: `then_some(i32::MIN / -1)`
+
+error: unnecessary closure used with `bool::then`
   --> $DIR/unnecessary_lazy_eval.rs:258:14
    |
+LL |     let _x = false.then(|| i32::MIN / 0);
+   |              ^^^^^^---------------------
+   |                    |
+   |                    help: use `then_some(..)` instead: `then_some(i32::MIN / 0)`
+
+error: unnecessary closure used with `bool::then`
+  --> $DIR/unnecessary_lazy_eval.rs:260:14
+   |
+LL |     let _x = false.then(|| 4 / 2);
+   |              ^^^^^^--------------
+   |                    |
+   |                    help: use `then_some(..)` instead: `then_some(4 / 2)`
+
+error: unnecessary closure used with `bool::then`
+  --> $DIR/unnecessary_lazy_eval.rs:268:14
+   |
 LL |     let _x = false.then(|| f1 + f2);
    |              ^^^^^^----------------
    |                    |
    |                    help: use `then_some(..)` instead: `then_some(f1 + f2)`
 
-error: aborting due to 58 previous errors
+error: aborting due to 63 previous errors