Skip to content

Commit

Permalink
Suggest manual_div_ceil even when right operand is a constant (#13951)
Browse files Browse the repository at this point in the history
changelog: [`manual_div_ceil`]: lint constants as well

Fix #13950
  • Loading branch information
blyxyas authored Jan 14, 2025
2 parents 8c1ea9f + dc23fa5 commit 7e83ec5
Show file tree
Hide file tree
Showing 7 changed files with 124 additions and 2 deletions.
36 changes: 36 additions & 0 deletions clippy_lints/src/manual_div_ceil.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,48 @@ impl<'tcx> LateLintPass<'tcx> for ManualDivCeil {
{
build_suggestion(cx, expr, add_lhs, div_rhs, &mut applicability);
}

// (x + (Y - 1)) / Y
if inner_op.node == BinOpKind::Add && differ_by_one(inner_rhs, div_rhs) {
build_suggestion(cx, expr, inner_lhs, div_rhs, &mut applicability);
}

// ((Y - 1) + x) / Y
if inner_op.node == BinOpKind::Add && differ_by_one(inner_lhs, div_rhs) {
build_suggestion(cx, expr, inner_rhs, div_rhs, &mut applicability);
}

// (x - (-Y - 1)) / Y
if inner_op.node == BinOpKind::Sub
&& let ExprKind::Unary(UnOp::Neg, abs_div_rhs) = div_rhs.kind
&& differ_by_one(abs_div_rhs, inner_rhs)
{
build_suggestion(cx, expr, inner_lhs, div_rhs, &mut applicability);
}
}
}

extract_msrv_attr!(LateContext);
}

/// Checks if two expressions represent non-zero integer literals such that `small_expr + 1 ==
/// large_expr`.
fn differ_by_one(small_expr: &Expr<'_>, large_expr: &Expr<'_>) -> bool {
if let ExprKind::Lit(small) = small_expr.kind
&& let ExprKind::Lit(large) = large_expr.kind
&& let LitKind::Int(s, _) = small.node
&& let LitKind::Int(l, _) = large.node
{
Some(l.get()) == s.get().checked_add(1)
} else if let ExprKind::Unary(UnOp::Neg, small_inner_expr) = small_expr.kind
&& let ExprKind::Unary(UnOp::Neg, large_inner_expr) = large_expr.kind
{
differ_by_one(large_inner_expr, small_inner_expr)
} else {
false
}
}

fn check_int_ty_and_feature(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
let expr_ty = cx.typeck_results().expr_ty(expr);
match expr_ty.peel_refs().kind() {
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/manual_div_ceil.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,14 @@ fn issue_13843() {

let _ = 1_000_000_u32.div_ceil(6u32);
}

fn issue_13950() {
let x = 33u32;
let _ = x.div_ceil(8);
let _ = x.div_ceil(8);

let y = -33i32;
let _ = (y + -8) / -7;
let _ = (-8 + y) / -7;
let _ = (y - 8) / -7;
}
11 changes: 11 additions & 0 deletions tests/ui/manual_div_ceil.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,14 @@ fn issue_13843() {

let _ = (1_000_000 + 6u32 - 1) / 6u32;
}

fn issue_13950() {
let x = 33u32;
let _ = (x + 7) / 8;
let _ = (7 + x) / 8;

let y = -33i32;
let _ = (y + -8) / -7;
let _ = (-8 + y) / -7;
let _ = (y - 8) / -7;
}
14 changes: 13 additions & 1 deletion tests/ui/manual_div_ceil.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -85,5 +85,17 @@ error: manually reimplementing `div_ceil`
LL | let _ = (1_000_000 + 6u32 - 1) / 6u32;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.div_ceil()`: `1_000_000_u32.div_ceil(6u32)`

error: aborting due to 14 previous errors
error: manually reimplementing `div_ceil`
--> tests/ui/manual_div_ceil.rs:56:13
|
LL | let _ = (x + 7) / 8;
| ^^^^^^^^^^^ help: consider using `.div_ceil()`: `x.div_ceil(8)`

error: manually reimplementing `div_ceil`
--> tests/ui/manual_div_ceil.rs:57:13
|
LL | let _ = (7 + x) / 8;
| ^^^^^^^^^^^ help: consider using `.div_ceil()`: `x.div_ceil(8)`

error: aborting due to 16 previous errors

11 changes: 11 additions & 0 deletions tests/ui/manual_div_ceil_with_feature.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,14 @@ fn issue_13843() {

let _ = 1_000_000_u32.div_ceil(6u32);
}

fn issue_13950() {
let x = 33u32;
let _ = x.div_ceil(8);
let _ = x.div_ceil(8);

let y = -33i32;
let _ = y.div_ceil(-7);
let _ = y.div_ceil(-7);
let _ = y.div_ceil(-7);
}
11 changes: 11 additions & 0 deletions tests/ui/manual_div_ceil_with_feature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,14 @@ fn issue_13843() {

let _ = (1_000_000 + 6u32 - 1) / 6u32;
}

fn issue_13950() {
let x = 33u32;
let _ = (x + 7) / 8;
let _ = (7 + x) / 8;

let y = -33i32;
let _ = (y + -8) / -7;
let _ = (-8 + y) / -7;
let _ = (y - 8) / -7;
}
32 changes: 31 additions & 1 deletion tests/ui/manual_div_ceil_with_feature.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -109,5 +109,35 @@ error: manually reimplementing `div_ceil`
LL | let _ = (1_000_000 + 6u32 - 1) / 6u32;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.div_ceil()`: `1_000_000_u32.div_ceil(6u32)`

error: aborting due to 18 previous errors
error: manually reimplementing `div_ceil`
--> tests/ui/manual_div_ceil_with_feature.rs:56:13
|
LL | let _ = (x + 7) / 8;
| ^^^^^^^^^^^ help: consider using `.div_ceil()`: `x.div_ceil(8)`

error: manually reimplementing `div_ceil`
--> tests/ui/manual_div_ceil_with_feature.rs:57:13
|
LL | let _ = (7 + x) / 8;
| ^^^^^^^^^^^ help: consider using `.div_ceil()`: `x.div_ceil(8)`

error: manually reimplementing `div_ceil`
--> tests/ui/manual_div_ceil_with_feature.rs:60:13
|
LL | let _ = (y + -8) / -7;
| ^^^^^^^^^^^^^ help: consider using `.div_ceil()`: `y.div_ceil(-7)`

error: manually reimplementing `div_ceil`
--> tests/ui/manual_div_ceil_with_feature.rs:61:13
|
LL | let _ = (-8 + y) / -7;
| ^^^^^^^^^^^^^ help: consider using `.div_ceil()`: `y.div_ceil(-7)`

error: manually reimplementing `div_ceil`
--> tests/ui/manual_div_ceil_with_feature.rs:62:13
|
LL | let _ = (y - 8) / -7;
| ^^^^^^^^^^^^ help: consider using `.div_ceil()`: `y.div_ceil(-7)`

error: aborting due to 23 previous errors

0 comments on commit 7e83ec5

Please sign in to comment.