Skip to content

Commit b012421

Browse files
committed
Auto merge of #12944 - Jarcho:overflow_check, r=y21
Fix and rename `overflow_check_conditional` fixes #2457 Other changes: * Limit the lint to unsigned types. * Actually check if the operands are the same rather than using only the first part of the path. * Allow the repeated expression to be anything as long as there are no side effects. changelog: Rename `overflow_check_conditional` to `panicking_overflow_check` and move to `correctness`
2 parents ab7c910 + d2ff2b9 commit b012421

13 files changed

+229
-222
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5705,6 +5705,7 @@ Released 2018-09-13
57055705
[`panic`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic
57065706
[`panic_in_result_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_in_result_fn
57075707
[`panic_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_params
5708+
[`panicking_overflow_checks`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_overflow_checks
57085709
[`panicking_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_unwrap
57095710
[`partial_pub_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#partial_pub_fields
57105711
[`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -587,12 +587,12 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
587587
crate::operators::VERBOSE_BIT_MASK_INFO,
588588
crate::option_env_unwrap::OPTION_ENV_UNWRAP_INFO,
589589
crate::option_if_let_else::OPTION_IF_LET_ELSE_INFO,
590-
crate::overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL_INFO,
591590
crate::panic_in_result_fn::PANIC_IN_RESULT_FN_INFO,
592591
crate::panic_unimplemented::PANIC_INFO,
593592
crate::panic_unimplemented::TODO_INFO,
594593
crate::panic_unimplemented::UNIMPLEMENTED_INFO,
595594
crate::panic_unimplemented::UNREACHABLE_INFO,
595+
crate::panicking_overflow_checks::PANICKING_OVERFLOW_CHECKS_INFO,
596596
crate::partial_pub_fields::PARTIAL_PUB_FIELDS_INFO,
597597
crate::partialeq_ne_impl::PARTIALEQ_NE_IMPL_INFO,
598598
crate::partialeq_to_none::PARTIALEQ_TO_NONE_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -280,9 +280,9 @@ mod only_used_in_recursion;
280280
mod operators;
281281
mod option_env_unwrap;
282282
mod option_if_let_else;
283-
mod overflow_check_conditional;
284283
mod panic_in_result_fn;
285284
mod panic_unimplemented;
285+
mod panicking_overflow_checks;
286286
mod partial_pub_fields;
287287
mod partialeq_ne_impl;
288288
mod partialeq_to_none;
@@ -790,7 +790,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
790790
let format_args = format_args_storage.clone();
791791
store.register_late_pass(move |_| Box::new(format::UselessFormat::new(format_args.clone())));
792792
store.register_late_pass(|_| Box::new(swap::Swap));
793-
store.register_late_pass(|_| Box::new(overflow_check_conditional::OverflowCheckConditional));
793+
store.register_late_pass(|_| Box::new(panicking_overflow_checks::PanickingOverflowChecks));
794794
store.register_late_pass(|_| Box::<new_without_default::NewWithoutDefault>::default());
795795
store.register_late_pass(move |_| Box::new(disallowed_names::DisallowedNames::new(disallowed_names)));
796796
store.register_late_pass(move |_| {

clippy_lints/src/overflow_check_conditional.rs

Lines changed: 0 additions & 70 deletions
This file was deleted.
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
use clippy_utils::diagnostics::span_lint;
2+
use clippy_utils::eq_expr_value;
3+
use rustc_hir::{BinOpKind, Expr, ExprKind};
4+
use rustc_lint::{LateContext, LateLintPass};
5+
use rustc_middle::lint::in_external_macro;
6+
use rustc_middle::ty;
7+
use rustc_session::declare_lint_pass;
8+
9+
declare_clippy_lint! {
10+
/// ### What it does
11+
/// Detects C-style underflow/overflow checks.
12+
///
13+
/// ### Why is this bad?
14+
/// These checks will, by default, panic in debug builds rather than check
15+
/// whether the operation caused an overflow.
16+
///
17+
/// ### Example
18+
/// ```no_run
19+
/// # let a = 1i32;
20+
/// # let b = 2i32;
21+
/// if a + b < a {
22+
/// // handle overflow
23+
/// }
24+
/// ```
25+
///
26+
/// Use instead:
27+
/// ```no_run
28+
/// # let a = 1i32;
29+
/// # let b = 2i32;
30+
/// if a.checked_add(b).is_none() {
31+
/// // handle overflow
32+
/// }
33+
/// ```
34+
///
35+
/// Or:
36+
/// ```no_run
37+
/// # let a = 1i32;
38+
/// # let b = 2i32;
39+
/// if a.overflowing_add(b).1 {
40+
/// // handle overflow
41+
/// }
42+
/// ```
43+
#[clippy::version = "pre 1.29.0"]
44+
pub PANICKING_OVERFLOW_CHECKS,
45+
correctness,
46+
"overflow checks which will panic in debug mode"
47+
}
48+
49+
declare_lint_pass!(PanickingOverflowChecks => [PANICKING_OVERFLOW_CHECKS]);
50+
51+
impl<'tcx> LateLintPass<'tcx> for PanickingOverflowChecks {
52+
// a + b < a, a > a + b, a < a - b, a - b > a
53+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
54+
if let ExprKind::Binary(op, lhs, rhs) = expr.kind
55+
&& let (lt, gt) = match op.node {
56+
BinOpKind::Lt => (lhs, rhs),
57+
BinOpKind::Gt => (rhs, lhs),
58+
_ => return,
59+
}
60+
&& let ctxt = expr.span.ctxt()
61+
&& let (op_lhs, op_rhs, other, commutative) = match (&lt.kind, &gt.kind) {
62+
(&ExprKind::Binary(op, lhs, rhs), _) if op.node == BinOpKind::Add && ctxt == lt.span.ctxt() => {
63+
(lhs, rhs, gt, true)
64+
},
65+
(_, &ExprKind::Binary(op, lhs, rhs)) if op.node == BinOpKind::Sub && ctxt == gt.span.ctxt() => {
66+
(lhs, rhs, lt, false)
67+
},
68+
_ => return,
69+
}
70+
&& let typeck = cx.typeck_results()
71+
&& let ty = typeck.expr_ty(op_lhs)
72+
&& matches!(ty.kind(), ty::Uint(_))
73+
&& ty == typeck.expr_ty(op_rhs)
74+
&& ty == typeck.expr_ty(other)
75+
&& !in_external_macro(cx.tcx.sess, expr.span)
76+
&& (eq_expr_value(cx, op_lhs, other) || (commutative && eq_expr_value(cx, op_rhs, other)))
77+
{
78+
span_lint(
79+
cx,
80+
PANICKING_OVERFLOW_CHECKS,
81+
expr.span,
82+
"you are trying to use classic C overflow conditions that will fail in Rust",
83+
);
84+
}
85+
}
86+
}

clippy_lints/src/renamed_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[
2626
("clippy::option_map_unwrap_or", "clippy::map_unwrap_or"),
2727
("clippy::option_map_unwrap_or_else", "clippy::map_unwrap_or"),
2828
("clippy::option_unwrap_used", "clippy::unwrap_used"),
29+
("clippy::overflow_check_conditional", "clippy::panicking_overflow_checks"),
2930
("clippy::ref_in_deref", "clippy::needless_borrow"),
3031
("clippy::result_expect_used", "clippy::expect_used"),
3132
("clippy::result_map_unwrap_or_else", "clippy::map_unwrap_or"),

tests/ui/overflow_check_conditional.rs

Lines changed: 0 additions & 36 deletions
This file was deleted.

tests/ui/overflow_check_conditional.stderr

Lines changed: 0 additions & 53 deletions
This file was deleted.

tests/ui/panicking_overflow_checks.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#![warn(clippy::panicking_overflow_checks)]
2+
#![allow(clippy::needless_if)]
3+
4+
fn test(a: u32, b: u32, c: u32) {
5+
if a + b < a {} //~ panicking_overflow_checks
6+
if a > a + b {} //~ panicking_overflow_checks
7+
if a + b < b {} //~ panicking_overflow_checks
8+
if b > a + b {} //~ panicking_overflow_checks
9+
if a - b > b {}
10+
if b < a - b {}
11+
if a - b > a {} //~ panicking_overflow_checks
12+
if a < a - b {} //~ panicking_overflow_checks
13+
if a + b < c {}
14+
if c > a + b {}
15+
if a - b < c {}
16+
if c > a - b {}
17+
let i = 1.1;
18+
let j = 2.2;
19+
if i + j < i {}
20+
if i - j < i {}
21+
if i > i + j {}
22+
if i - j < i {}
23+
}
24+
25+
fn main() {
26+
test(1, 2, 3)
27+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
error: you are trying to use classic C overflow conditions that will fail in Rust
2+
--> tests/ui/panicking_overflow_checks.rs:5:8
3+
|
4+
LL | if a + b < a {}
5+
| ^^^^^^^^^
6+
|
7+
= note: `-D clippy::panicking-overflow-checks` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::panicking_overflow_checks)]`
9+
10+
error: you are trying to use classic C overflow conditions that will fail in Rust
11+
--> tests/ui/panicking_overflow_checks.rs:6:8
12+
|
13+
LL | if a > a + b {}
14+
| ^^^^^^^^^
15+
16+
error: you are trying to use classic C overflow conditions that will fail in Rust
17+
--> tests/ui/panicking_overflow_checks.rs:7:8
18+
|
19+
LL | if a + b < b {}
20+
| ^^^^^^^^^
21+
22+
error: you are trying to use classic C overflow conditions that will fail in Rust
23+
--> tests/ui/panicking_overflow_checks.rs:8:8
24+
|
25+
LL | if b > a + b {}
26+
| ^^^^^^^^^
27+
28+
error: you are trying to use classic C overflow conditions that will fail in Rust
29+
--> tests/ui/panicking_overflow_checks.rs:11:8
30+
|
31+
LL | if a - b > a {}
32+
| ^^^^^^^^^
33+
34+
error: you are trying to use classic C overflow conditions that will fail in Rust
35+
--> tests/ui/panicking_overflow_checks.rs:12:8
36+
|
37+
LL | if a < a - b {}
38+
| ^^^^^^^^^
39+
40+
error: aborting due to 6 previous errors
41+

tests/ui/rename.fixed

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#![allow(clippy::expect_used)]
2525
#![allow(clippy::map_unwrap_or)]
2626
#![allow(clippy::unwrap_used)]
27+
#![allow(clippy::panicking_overflow_checks)]
2728
#![allow(clippy::needless_borrow)]
2829
#![allow(clippy::single_char_add_str)]
2930
#![allow(clippy::module_name_repetitions)]
@@ -78,6 +79,7 @@
7879
#![warn(clippy::map_unwrap_or)]
7980
#![warn(clippy::map_unwrap_or)]
8081
#![warn(clippy::unwrap_used)]
82+
#![warn(clippy::panicking_overflow_checks)]
8183
#![warn(clippy::needless_borrow)]
8284
#![warn(clippy::expect_used)]
8385
#![warn(clippy::map_unwrap_or)]

0 commit comments

Comments
 (0)