Skip to content

Commit 0f1bfb2

Browse files
committed
Move muldiv peeling into expr_sign
1 parent 40c1b32 commit 0f1bfb2

File tree

1 file changed

+28
-22
lines changed

1 file changed

+28
-22
lines changed

clippy_lints/src/casts/cast_sign_loss.rs

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -56,26 +56,7 @@ fn should_lint<'cx>(cx: &LateContext<'cx>, cast_op: &Expr<'_>, cast_from: Ty<'cx
5656
}
5757

5858
// Don't lint if `cast_op` is known to be positive, ignoring overflow.
59-
if let Sign::ZeroOrPositive = expr_sign(cx, cast_op, cast_from) {
60-
return false;
61-
}
62-
63-
let (mut uncertain_count, mut negative_count) = (0, 0);
64-
// Peel off possible binary expressions, for example:
65-
// x * x / y => [x, x, y]
66-
// a % b => [a]
67-
let exprs = exprs_with_selected_binop_peeled(cast_op);
68-
for expr in exprs {
69-
match expr_sign(cx, expr, None) {
70-
Sign::Negative => negative_count += 1,
71-
Sign::Uncertain => uncertain_count += 1,
72-
Sign::ZeroOrPositive => (),
73-
};
74-
}
75-
76-
// Lint if there are any uncertain results (because they could be negative or positive),
77-
// or an odd number of negative results.
78-
uncertain_count > 0 || negative_count % 2 == 1
59+
expr_sign(cx, cast_op, cast_from) == Sign::ZeroOrPositive
7960
},
8061

8162
(false, true) => !cast_to.is_signed(),
@@ -153,7 +134,32 @@ fn expr_sign<'cx>(cx: &LateContext<'cx>, expr: &Expr<'_>, ty: impl Into<Option<T
153134
}
154135
}
155136

156-
Sign::Uncertain
137+
let mut uncertain_count = 0;
138+
let mut negative_count = 0;
139+
140+
// Peel off possible binary expressions, for example:
141+
// x * x / y => [x, x, y]
142+
// a % b => [a]
143+
let exprs = exprs_with_muldiv_binop_peeled(expr);
144+
for expr in exprs {
145+
match expr_sign(cx, expr, None) {
146+
Sign::Negative => negative_count += 1,
147+
Sign::Uncertain => uncertain_count += 1,
148+
Sign::ZeroOrPositive => (),
149+
};
150+
}
151+
152+
// A mul/div is:
153+
// - uncertain if there are any uncertain values (because they could be negative or positive),
154+
// - negative if there are an odd number of negative values,
155+
// - positive or zero otherwise.
156+
if uncertain_count > 0 {
157+
Sign::Uncertain
158+
} else if negative_count % 2 == 1 {
159+
Sign::Negative
160+
} else {
161+
Sign::ZeroOrPositive
162+
}
157163
}
158164

159165
/// Return the sign of the `pow` call's result, ignoring overflow.
@@ -193,7 +199,7 @@ fn pow_call_result_sign(cx: &LateContext<'_>, base: &Expr<'_>, exponent: &Expr<'
193199
/// which the result could always be positive under certain conditions, ignoring overflow.
194200
///
195201
/// Expressions using other operators are preserved, so we can try to evaluate them later.
196-
fn exprs_with_selected_binop_peeled<'a>(expr: &'a Expr<'_>) -> Vec<&'a Expr<'a>> {
202+
fn exprs_with_muldiv_binop_peeled<'a>(expr: &'a Expr<'_>) -> Vec<&'a Expr<'a>> {
197203
#[inline]
198204
fn collect_operands<'a>(expr: &'a Expr<'a>, operands: &mut Vec<&'a Expr<'a>>) {
199205
match expr.kind {

0 commit comments

Comments
 (0)