Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 45 additions & 1 deletion clippy_lints/src/manual_clamp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use clippy_utils::res::{MaybeDef as _, MaybeResPath as _, MaybeTypeckRes as _};
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::implements_trait;
use clippy_utils::visitors::is_const_evaluatable;
use clippy_utils::{eq_expr_value, is_in_const_context, peel_blocks, peel_blocks_with_stmt, sym};
use clippy_utils::{eq_expr_value, is_in_const_context, is_integer_literal, peel_blocks, peel_blocks_with_stmt, sym};
use itertools::Itertools as _;
use rustc_errors::{Applicability, Diag};
use rustc_hir::def::Res;
Expand Down Expand Up @@ -718,6 +718,9 @@ fn is_clamp_meta_pattern<'tcx>(
if exprs.iter().any(|e| peel_blocks(e).can_have_side_effects()) {
return None;
}
let first_bin = try_normalize_unsigned_eq_to_ord(cx, first_bin, first_expr).unwrap_or(*first_bin);
let second_bin = try_normalize_unsigned_eq_to_ord(cx, second_bin, second_expr).unwrap_or(*second_bin);
let (first_bin, second_bin) = (&first_bin, &second_bin);
if !(is_ord_op(first_bin.op) && is_ord_op(second_bin.op)) {
return None;
}
Expand Down Expand Up @@ -771,6 +774,47 @@ fn is_ord_op(op: BinOpKind) -> bool {
matches!(op, BinOpKind::Ge | BinOpKind::Gt | BinOpKind::Le | BinOpKind::Lt)
}

/// For unsigned integer types, `x == 0` is equivalent to `x < 1`. This normalizes such `==`
/// comparisons to `<` when the result expression evaluates to 1, enabling clamp detection for
/// patterns like:
///
/// ```ignore
/// if x == 0 { x = 1; } else if x > y { x = y; }
/// ```
fn try_normalize_unsigned_eq_to_ord<'tcx>(
cx: &LateContext<'tcx>,
bin: &BinaryOp<'tcx>,
result_expr: &'tcx Expr<'tcx>,
) -> Option<BinaryOp<'tcx>> {
if bin.op != BinOpKind::Eq {
return None;
}

// both sides of the expression have the same type, so checking either of them is sufficient
if !matches!(cx.typeck_results().expr_ty(bin.left).kind(), rustc_middle::ty::Uint(_)) {
return None;
}

// Handles both `x == 0` and the Yoda notation `0 == x`.
let input = if is_integer_literal(bin.right, 0) {
bin.left
} else if is_integer_literal(bin.left, 0) {
bin.right
} else {
return None;
};

if is_integer_literal(result_expr, 1) {
Some(BinaryOp {
op: BinOpKind::Lt,
left: input,
right: result_expr,
})
} else {
None
}
}

/// Really similar to Cow, but doesn't have a `Clone` requirement.
#[derive(Debug)]
enum MaybeBorrowedStmtKind<'a> {
Expand Down
38 changes: 38 additions & 0 deletions tests/ui/manual_clamp.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,44 @@ fn msrv_1_50() {
let _ = input.clamp(CONST_MIN, CONST_MAX);
}

const CONST_U32_MAX: u32 = 100;

fn eq_unsigned_zero_lint() {
// == 0 on unsigned with result 1: equivalent to < 1
let mut x0: u32 = 5;
x0 = x0.clamp(1, CONST_U32_MAX);

// Flipped: `0 == x`
let mut x1: u32 = 5;
x1 = x1.clamp(1, CONST_U32_MAX);

// Two separate if statements
let mut x2: u32 = 5;
x2 = x2.clamp(1, CONST_U32_MAX);

// if-elseif-else (value) pattern
let input: u32 = 5;
let x3 = input.clamp(1, CONST_U32_MAX);
}

fn eq_unsigned_zero_no_lint() {
// Result is not 1, so == 0 doesn't cover all values < 5.
let mut x0: u32 = 5;
if x0 == 0 {
x0 = 5;
} else if x0 > CONST_U32_MAX {
x0 = CONST_U32_MAX;
}

// Signed type: not applicable
let mut x1: i32 = 5;
if x1 == 0 {
x1 = 1;
} else if x1 > CONST_MAX {
x1 = CONST_MAX;
}
}

const fn _const() {
let (input, min, max) = (0, -1, 2);
let _ = if input < min {
Expand Down
61 changes: 61 additions & 0 deletions tests/ui/manual_clamp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,67 @@ fn msrv_1_50() {
};
}

const CONST_U32_MAX: u32 = 100;

fn eq_unsigned_zero_lint() {
// == 0 on unsigned with result 1: equivalent to < 1
let mut x0: u32 = 5;
if x0 == 0 {
//~^ manual_clamp
x0 = 1;
} else if x0 > CONST_U32_MAX {
x0 = CONST_U32_MAX;
}

// Flipped: `0 == x`
let mut x1: u32 = 5;
if 0 == x1 {
//~^ manual_clamp
x1 = 1;
} else if x1 > CONST_U32_MAX {
x1 = CONST_U32_MAX;
}

// Two separate if statements
let mut x2: u32 = 5;
if x2 == 0 {
//~^ manual_clamp
x2 = 1;
}
if x2 > CONST_U32_MAX {
x2 = CONST_U32_MAX;
}

// if-elseif-else (value) pattern
let input: u32 = 5;
let x3 = if input == 0 {
//~^ manual_clamp
1
} else if input > CONST_U32_MAX {
CONST_U32_MAX
} else {
input
};
}

fn eq_unsigned_zero_no_lint() {
// Result is not 1, so == 0 doesn't cover all values < 5.
let mut x0: u32 = 5;
if x0 == 0 {
x0 = 5;
} else if x0 > CONST_U32_MAX {
x0 = CONST_U32_MAX;
}

// Signed type: not applicable
let mut x1: i32 = 5;
if x1 == 0 {
x1 = 1;
} else if x1 > CONST_MAX {
x1 = CONST_MAX;
}
}

const fn _const() {
let (input, min, max) = (0, -1, 2);
let _ = if input < min {
Expand Down
56 changes: 55 additions & 1 deletion tests/ui/manual_clamp.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -398,5 +398,59 @@ LL | | };
|
= note: clamp will panic if max < min

error: aborting due to 35 previous errors
error: clamp-like pattern without using clamp function
--> tests/ui/manual_clamp.rs:553:5
|
LL | / if x2 == 0 {
LL | |
LL | | x2 = 1;
... |
LL | | x2 = CONST_U32_MAX;
LL | | }
| |_____^ help: replace with clamp: `x2 = x2.clamp(1, CONST_U32_MAX);`
|
= note: clamp will panic if max < min

error: clamp-like pattern without using clamp function
--> tests/ui/manual_clamp.rs:535:5
|
LL | / if x0 == 0 {
LL | |
LL | | x0 = 1;
LL | | } else if x0 > CONST_U32_MAX {
LL | | x0 = CONST_U32_MAX;
LL | | }
| |_____^ help: replace with clamp: `x0 = x0.clamp(1, CONST_U32_MAX);`
|
= note: clamp will panic if max < min

error: clamp-like pattern without using clamp function
--> tests/ui/manual_clamp.rs:544:5
|
LL | / if 0 == x1 {
LL | |
LL | | x1 = 1;
LL | | } else if x1 > CONST_U32_MAX {
LL | | x1 = CONST_U32_MAX;
LL | | }
| |_____^ help: replace with clamp: `x1 = x1.clamp(1, CONST_U32_MAX);`
|
= note: clamp will panic if max < min

error: clamp-like pattern without using clamp function
--> tests/ui/manual_clamp.rs:563:14
|
LL | let x3 = if input == 0 {
| ______________^
LL | |
LL | | 1
LL | | } else if input > CONST_U32_MAX {
... |
LL | | input
LL | | };
| |_____^ help: replace with clamp: `input.clamp(1, CONST_U32_MAX)`
|
= note: clamp will panic if max < min

error: aborting due to 39 previous errors