Skip to content

Commit aa371c3

Browse files
JarchoJarcho
committed
Rewrite overflow_check_conditional 
1 parent f2c74e2 commit aa371c3

File tree

3 files changed

+45
-70
lines changed

3 files changed

+45
-70
lines changed

‎clippy_lints/src/overflow_check_conditional.rs‎

Lines changed: 31 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
use clippy_utils::diagnostics::span_lint;
2-
use clippy_utils::SpanlessEq;
3-
use rustc_hir::{BinOpKind, Expr, ExprKind, QPath};
2+
use clippy_utils::eq_expr_value;
3+
use rustc_hir::{BinOpKind, Expr, ExprKind};
44
use rustc_lint::{LateContext, LateLintPass};
5+
use rustc_middle::lint::in_external_macro;
6+
use rustc_middle::ty;
57
use rustc_session::declare_lint_pass;
68

79
declare_clippy_lint! {
@@ -26,45 +28,39 @@ declare_clippy_lint! {
2628

2729
declare_lint_pass!(OverflowCheckConditional => [OVERFLOW_CHECK_CONDITIONAL]);
2830

29-
const OVERFLOW_MSG: &str = "you are trying to use classic C overflow conditions that will fail in Rust";
30-
const UNDERFLOW_MSG: &str = "you are trying to use classic C underflow conditions that will fail in Rust";
31-
3231
impl<'tcx> LateLintPass<'tcx> for OverflowCheckConditional {
3332
// a + b < a, a > a + b, a < a - b, a - b > a
3433
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
35-
let eq = |l, r| SpanlessEq::new(cx).eq_path_segment(l, r);
36-
if let ExprKind::Binary(ref op, first, second) = expr.kind
37-
&& let ExprKind::Binary(ref op2, ident1, ident2) = first.kind
38-
&& let ExprKind::Path(QPath::Resolved(_, path1)) = ident1.kind
39-
&& let ExprKind::Path(QPath::Resolved(_, path2)) = ident2.kind
40-
&& let ExprKind::Path(QPath::Resolved(_, path3)) = second.kind
41-
&& (eq(&path1.segments[0], &path3.segments[0]) || eq(&path2.segments[0], &path3.segments[0]))
42-
&& cx.typeck_results().expr_ty(ident1).is_integral()
43-
&& cx.typeck_results().expr_ty(ident2).is_integral()
44-
{
45-
if op.node == BinOpKind::Lt && op2.node == BinOpKind::Add {
46-
span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, OVERFLOW_MSG);
34+
if let ExprKind::Binary(op, lhs, rhs) = expr.kind
35+
&& let (lt, gt) = match op.node {
36+
BinOpKind::Lt => (lhs, rhs),
37+
BinOpKind::Gt => (rhs, lhs),
38+
_ => return,
4739
}
48-
if op.node == BinOpKind::Gt && op2.node == BinOpKind::Sub {
49-
span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, UNDERFLOW_MSG);
40+
&& let ctxt = expr.span.ctxt()
41+
&& let (op_lhs, op_rhs, other, commutative) = match (&lt.kind, &gt.kind) {
42+
(&ExprKind::Binary(op, lhs, rhs), _) if op.node == BinOpKind::Add && ctxt == lt.span.ctxt() => {
43+
(lhs, rhs, gt, true)
44+
},
45+
(_, &ExprKind::Binary(op, lhs, rhs)) if op.node == BinOpKind::Sub && ctxt == gt.span.ctxt() => {
46+
(lhs, rhs, lt, false)
47+
},
48+
_ => return,
5049
}
51-
}
52-
53-
if let ExprKind::Binary(ref op, first, second) = expr.kind
54-
&& let ExprKind::Binary(ref op2, ident1, ident2) = second.kind
55-
&& let ExprKind::Path(QPath::Resolved(_, path1)) = ident1.kind
56-
&& let ExprKind::Path(QPath::Resolved(_, path2)) = ident2.kind
57-
&& let ExprKind::Path(QPath::Resolved(_, path3)) = first.kind
58-
&& (eq(&path1.segments[0], &path3.segments[0]) || eq(&path2.segments[0], &path3.segments[0]))
59-
&& cx.typeck_results().expr_ty(ident1).is_integral()
60-
&& cx.typeck_results().expr_ty(ident2).is_integral()
50+
&& let typeck = cx.typeck_results()
51+
&& let ty = typeck.expr_ty(op_lhs)
52+
&& matches!(ty.kind(), ty::Uint(_))
53+
&& ty == typeck.expr_ty(op_rhs)
54+
&& ty == typeck.expr_ty(other)
55+
&& !in_external_macro(cx.tcx.sess, expr.span)
56+
&& (eq_expr_value(cx, op_lhs, other) || (commutative && eq_expr_value(cx, op_rhs, other)))
6157
{
62-
if op.node == BinOpKind::Gt && op2.node == BinOpKind::Add {
63-
span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, OVERFLOW_MSG);
64-
}
65-
if op.node == BinOpKind::Lt && op2.node == BinOpKind::Sub {
66-
span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, UNDERFLOW_MSG);
67-
}
58+
span_lint(
59+
cx,
60+
OVERFLOW_CHECK_CONDITIONAL,
61+
expr.span,
62+
"you are trying to use classic C overflow conditions that will fail in Rust",
63+
);
6864
}
6965
}
7066
}

‎tests/ui/overflow_check_conditional.rs‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,14 @@
22
#![allow(clippy::needless_if)]
33

44
fn test(a: u32, b: u32, c: u32) {
5-
if a + b < a {}
6-
//~^ ERROR: you are trying to use classic C overflow conditions that will fail in Rust
7-
//~| NOTE: `-D clippy::overflow-check-conditional` implied by `-D warnings`
8-
if a > a + b {}
9-
//~^ ERROR: you are trying to use classic C overflow conditions that will fail in Rust
10-
if a + b < b {}
11-
//~^ ERROR: you are trying to use classic C overflow conditions that will fail in Rust
12-
if b > a + b {}
13-
//~^ ERROR: you are trying to use classic C overflow conditions that will fail in Rust
5+
if a + b < a {} //~ overflow_check_conditional
6+
if a > a + b {} //~ overflow_check_conditional
7+
if a + b < b {} //~ overflow_check_conditional
8+
if b > a + b {} //~ overflow_check_conditional
149
if a - b > b {}
15-
//~^ ERROR: you are trying to use classic C underflow conditions that will fail in Rus
1610
if b < a - b {}
17-
//~^ ERROR: you are trying to use classic C underflow conditions that will fail in Rus
18-
if a - b > a {}
19-
//~^ ERROR: you are trying to use classic C underflow conditions that will fail in Rus
20-
if a < a - b {}
21-
//~^ ERROR: you are trying to use classic C underflow conditions that will fail in Rus
11+
if a - b > a {} //~ overflow_check_conditional
12+
if a < a - b {} //~ overflow_check_conditional
2213
if a + b < c {}
2314
if c > a + b {}
2415
if a - b < c {}

‎tests/ui/overflow_check_conditional.stderr‎

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,46 +8,34 @@ LL | if a + b < a {}
88
= help: to override `-D warnings` add `#[allow(clippy::overflow_check_conditional)]`
99

1010
error: you are trying to use classic C overflow conditions that will fail in Rust
11-
--> tests/ui/overflow_check_conditional.rs:8:8
11+
--> tests/ui/overflow_check_conditional.rs:6:8
1212
|
1313
LL | if a > a + b {}
1414
| ^^^^^^^^^
1515

1616
error: you are trying to use classic C overflow conditions that will fail in Rust
17-
--> tests/ui/overflow_check_conditional.rs:10:8
17+
--> tests/ui/overflow_check_conditional.rs:7:8
1818
|
1919
LL | if a + b < b {}
2020
| ^^^^^^^^^
2121

2222
error: you are trying to use classic C overflow conditions that will fail in Rust
23-
--> tests/ui/overflow_check_conditional.rs:12:8
23+
--> tests/ui/overflow_check_conditional.rs:8:8
2424
|
2525
LL | if b > a + b {}
2626
| ^^^^^^^^^
2727

28-
error: you are trying to use classic C underflow conditions that will fail in Rust
29-
--> tests/ui/overflow_check_conditional.rs:14:8
30-
|
31-
LL | if a - b > b {}
32-
| ^^^^^^^^^
33-
34-
error: you are trying to use classic C underflow conditions that will fail in Rust
35-
--> tests/ui/overflow_check_conditional.rs:16:8
36-
|
37-
LL | if b < a - b {}
38-
| ^^^^^^^^^
39-
40-
error: you are trying to use classic C underflow conditions that will fail in Rust
41-
--> tests/ui/overflow_check_conditional.rs:18:8
28+
error: you are trying to use classic C overflow conditions that will fail in Rust
29+
--> tests/ui/overflow_check_conditional.rs:11:8
4230
|
4331
LL | if a - b > a {}
4432
| ^^^^^^^^^
4533

46-
error: you are trying to use classic C underflow conditions that will fail in Rust
47-
--> tests/ui/overflow_check_conditional.rs:20:8
34+
error: you are trying to use classic C overflow conditions that will fail in Rust
35+
--> tests/ui/overflow_check_conditional.rs:12:8
4836
|
4937
LL | if a < a - b {}
5038
| ^^^^^^^^^
5139

52-
error: aborting due to 8 previous errors
40+
error: aborting due to 6 previous errors
5341

0 commit comments

Comments
 (0)