about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJason Newcomb <jsnewcomb@pm.me>2024-06-16 01:34:25 -0400
committerJason Newcomb <jsnewcomb@pm.me>2024-07-07 10:22:29 -0400
commitaa371c37c2989e2c505865630179250ec528c19b (patch)
tree5d508ccd17dd54245a52943aeeeef556c193baf8
parentf2c74e220bef89dc62dfbd1d2a4eb49c6ae8b47a (diff)
downloadrust-aa371c37c2989e2c505865630179250ec528c19b.tar.gz
rust-aa371c37c2989e2c505865630179250ec528c19b.zip
Rewrite `overflow_check_conditional`
-rw-r--r--clippy_lints/src/overflow_check_conditional.rs66
-rw-r--r--tests/ui/overflow_check_conditional.rs21
-rw-r--r--tests/ui/overflow_check_conditional.stderr28
3 files changed, 45 insertions, 70 deletions
diff --git a/clippy_lints/src/overflow_check_conditional.rs b/clippy_lints/src/overflow_check_conditional.rs
index de789879331..6909c1f6f67 100644
--- a/clippy_lints/src/overflow_check_conditional.rs
+++ b/clippy_lints/src/overflow_check_conditional.rs
@@ -1,7 +1,9 @@
 use clippy_utils::diagnostics::span_lint;
-use clippy_utils::SpanlessEq;
-use rustc_hir::{BinOpKind, Expr, ExprKind, QPath};
+use clippy_utils::eq_expr_value;
+use rustc_hir::{BinOpKind, Expr, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::lint::in_external_macro;
+use rustc_middle::ty;
 use rustc_session::declare_lint_pass;
 
 declare_clippy_lint! {
@@ -26,45 +28,39 @@ declare_clippy_lint! {
 
 declare_lint_pass!(OverflowCheckConditional => [OVERFLOW_CHECK_CONDITIONAL]);
 
-const OVERFLOW_MSG: &str = "you are trying to use classic C overflow conditions that will fail in Rust";
-const UNDERFLOW_MSG: &str = "you are trying to use classic C underflow conditions that will fail in Rust";
-
 impl<'tcx> LateLintPass<'tcx> for OverflowCheckConditional {
     // a + b < a, a > a + b, a < a - b, a - b > a
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-        let eq = |l, r| SpanlessEq::new(cx).eq_path_segment(l, r);
-        if let ExprKind::Binary(ref op, first, second) = expr.kind
-            && let ExprKind::Binary(ref op2, ident1, ident2) = first.kind
-            && let ExprKind::Path(QPath::Resolved(_, path1)) = ident1.kind
-            && let ExprKind::Path(QPath::Resolved(_, path2)) = ident2.kind
-            && let ExprKind::Path(QPath::Resolved(_, path3)) = second.kind
-            && (eq(&path1.segments[0], &path3.segments[0]) || eq(&path2.segments[0], &path3.segments[0]))
-            && cx.typeck_results().expr_ty(ident1).is_integral()
-            && cx.typeck_results().expr_ty(ident2).is_integral()
-        {
-            if op.node == BinOpKind::Lt && op2.node == BinOpKind::Add {
-                span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, OVERFLOW_MSG);
+        if let ExprKind::Binary(op, lhs, rhs) = expr.kind
+            && let (lt, gt) = match op.node {
+                BinOpKind::Lt => (lhs, rhs),
+                BinOpKind::Gt => (rhs, lhs),
+                _ => return,
             }
-            if op.node == BinOpKind::Gt && op2.node == BinOpKind::Sub {
-                span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, UNDERFLOW_MSG);
+            && let ctxt = expr.span.ctxt()
+            && let (op_lhs, op_rhs, other, commutative) = match (&lt.kind, &gt.kind) {
+                (&ExprKind::Binary(op, lhs, rhs), _) if op.node == BinOpKind::Add && ctxt == lt.span.ctxt() => {
+                    (lhs, rhs, gt, true)
+                },
+                (_, &ExprKind::Binary(op, lhs, rhs)) if op.node == BinOpKind::Sub && ctxt == gt.span.ctxt() => {
+                    (lhs, rhs, lt, false)
+                },
+                _ => return,
             }
-        }
-
-        if let ExprKind::Binary(ref op, first, second) = expr.kind
-            && let ExprKind::Binary(ref op2, ident1, ident2) = second.kind
-            && let ExprKind::Path(QPath::Resolved(_, path1)) = ident1.kind
-            && let ExprKind::Path(QPath::Resolved(_, path2)) = ident2.kind
-            && let ExprKind::Path(QPath::Resolved(_, path3)) = first.kind
-            && (eq(&path1.segments[0], &path3.segments[0]) || eq(&path2.segments[0], &path3.segments[0]))
-            && cx.typeck_results().expr_ty(ident1).is_integral()
-            && cx.typeck_results().expr_ty(ident2).is_integral()
+            && let typeck = cx.typeck_results()
+            && let ty = typeck.expr_ty(op_lhs)
+            && matches!(ty.kind(), ty::Uint(_))
+            && ty == typeck.expr_ty(op_rhs)
+            && ty == typeck.expr_ty(other)
+            && !in_external_macro(cx.tcx.sess, expr.span)
+            && (eq_expr_value(cx, op_lhs, other) || (commutative && eq_expr_value(cx, op_rhs, other)))
         {
-            if op.node == BinOpKind::Gt && op2.node == BinOpKind::Add {
-                span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, OVERFLOW_MSG);
-            }
-            if op.node == BinOpKind::Lt && op2.node == BinOpKind::Sub {
-                span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, UNDERFLOW_MSG);
-            }
+            span_lint(
+                cx,
+                OVERFLOW_CHECK_CONDITIONAL,
+                expr.span,
+                "you are trying to use classic C overflow conditions that will fail in Rust",
+            );
         }
     }
 }
diff --git a/tests/ui/overflow_check_conditional.rs b/tests/ui/overflow_check_conditional.rs
index a70bb3bc47b..acabe9a37e0 100644
--- a/tests/ui/overflow_check_conditional.rs
+++ b/tests/ui/overflow_check_conditional.rs
@@ -2,23 +2,14 @@
 #![allow(clippy::needless_if)]
 
 fn test(a: u32, b: u32, c: u32) {
-    if a + b < a {}
-    //~^ ERROR: you are trying to use classic C overflow conditions that will fail in Rust
-    //~| NOTE: `-D clippy::overflow-check-conditional` implied by `-D warnings`
-    if a > a + b {}
-    //~^ ERROR: you are trying to use classic C overflow conditions that will fail in Rust
-    if a + b < b {}
-    //~^ ERROR: you are trying to use classic C overflow conditions that will fail in Rust
-    if b > a + b {}
-    //~^ ERROR: you are trying to use classic C overflow conditions that will fail in Rust
+    if a + b < a {} //~ overflow_check_conditional
+    if a > a + b {} //~ overflow_check_conditional
+    if a + b < b {} //~ overflow_check_conditional
+    if b > a + b {} //~ overflow_check_conditional
     if a - b > b {}
-    //~^ ERROR: you are trying to use classic C underflow conditions that will fail in Rus
     if b < a - b {}
-    //~^ ERROR: you are trying to use classic C underflow conditions that will fail in Rus
-    if a - b > a {}
-    //~^ ERROR: you are trying to use classic C underflow conditions that will fail in Rus
-    if a < a - b {}
-    //~^ ERROR: you are trying to use classic C underflow conditions that will fail in Rus
+    if a - b > a {} //~ overflow_check_conditional
+    if a < a - b {} //~ overflow_check_conditional
     if a + b < c {}
     if c > a + b {}
     if a - b < c {}
diff --git a/tests/ui/overflow_check_conditional.stderr b/tests/ui/overflow_check_conditional.stderr
index c14532bad5a..e71768d3745 100644
--- a/tests/ui/overflow_check_conditional.stderr
+++ b/tests/ui/overflow_check_conditional.stderr
@@ -8,46 +8,34 @@ LL |     if a + b < a {}
    = help: to override `-D warnings` add `#[allow(clippy::overflow_check_conditional)]`
 
 error: you are trying to use classic C overflow conditions that will fail in Rust
-  --> tests/ui/overflow_check_conditional.rs:8:8
+  --> tests/ui/overflow_check_conditional.rs:6:8
    |
 LL |     if a > a + b {}
    |        ^^^^^^^^^
 
 error: you are trying to use classic C overflow conditions that will fail in Rust
-  --> tests/ui/overflow_check_conditional.rs:10:8
+  --> tests/ui/overflow_check_conditional.rs:7:8
    |
 LL |     if a + b < b {}
    |        ^^^^^^^^^
 
 error: you are trying to use classic C overflow conditions that will fail in Rust
-  --> tests/ui/overflow_check_conditional.rs:12:8
+  --> tests/ui/overflow_check_conditional.rs:8:8
    |
 LL |     if b > a + b {}
    |        ^^^^^^^^^
 
-error: you are trying to use classic C underflow conditions that will fail in Rust
-  --> tests/ui/overflow_check_conditional.rs:14:8
-   |
-LL |     if a - b > b {}
-   |        ^^^^^^^^^
-
-error: you are trying to use classic C underflow conditions that will fail in Rust
-  --> tests/ui/overflow_check_conditional.rs:16:8
-   |
-LL |     if b < a - b {}
-   |        ^^^^^^^^^
-
-error: you are trying to use classic C underflow conditions that will fail in Rust
-  --> tests/ui/overflow_check_conditional.rs:18:8
+error: you are trying to use classic C overflow conditions that will fail in Rust
+  --> tests/ui/overflow_check_conditional.rs:11:8
    |
 LL |     if a - b > a {}
    |        ^^^^^^^^^
 
-error: you are trying to use classic C underflow conditions that will fail in Rust
-  --> tests/ui/overflow_check_conditional.rs:20:8
+error: you are trying to use classic C overflow conditions that will fail in Rust
+  --> tests/ui/overflow_check_conditional.rs:12:8
    |
 LL |     if a < a - b {}
    |        ^^^^^^^^^
 
-error: aborting due to 8 previous errors
+error: aborting due to 6 previous errors