Skip to content

Commit 5ae684b

Browse files
committed
Put muldiv peeling in its own method
1 parent 0f1bfb2 commit 5ae684b

File tree

1 file changed

+45
-29
lines changed

1 file changed

+45
-29
lines changed

clippy_lints/src/casts/cast_sign_loss.rs

Lines changed: 45 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,15 @@ 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-
expr_sign(cx, cast_op, cast_from) == Sign::ZeroOrPositive
59+
if let Sign::ZeroOrPositive = expr_sign(cx, cast_op, cast_from) {
60+
return false;
61+
}
62+
63+
if let Sign::ZeroOrPositive = expr_muldiv_sign(cx, cast_op) {
64+
return false;
65+
}
66+
67+
true
6068
},
6169

6270
(false, true) => !cast_to.is_signed(),
@@ -134,32 +142,7 @@ fn expr_sign<'cx>(cx: &LateContext<'cx>, expr: &Expr<'_>, ty: impl Into<Option<T
134142
}
135143
}
136144

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-
}
145+
Sign::Uncertain
163146
}
164147

165148
/// Return the sign of the `pow` call's result, ignoring overflow.
@@ -195,13 +178,46 @@ fn pow_call_result_sign(cx: &LateContext<'_>, base: &Expr<'_>, exponent: &Expr<'
195178
}
196179
}
197180

181+
/// Peels binary operators such as [`BinOpKind::Mul`], [`BinOpKind::Div`] or [`BinOpKind::Rem`],
182+
/// which the result could always be positive under certain conditions, ignoring overflow.
183+
///
184+
/// Returns the sign of the list of peeled expressions.
185+
fn expr_muldiv_sign(cx: &LateContext<'_>, expr: &Expr<'_>) -> Sign {
186+
let mut uncertain_count = 0;
187+
let mut negative_count = 0;
188+
189+
// Peel off possible binary expressions, for example:
190+
// x * x / y => [x, x, y]
191+
// a % b => [a]
192+
let exprs = exprs_with_muldiv_binop_peeled(expr);
193+
for expr in exprs {
194+
match expr_sign(cx, expr, None) {
195+
Sign::Negative => negative_count += 1,
196+
Sign::Uncertain => uncertain_count += 1,
197+
Sign::ZeroOrPositive => (),
198+
};
199+
}
200+
201+
// A mul/div is:
202+
// - uncertain if there are any uncertain values (because they could be negative or positive),
203+
// - negative if there are an odd number of negative values,
204+
// - positive or zero otherwise.
205+
if uncertain_count > 0 {
206+
Sign::Uncertain
207+
} else if negative_count % 2 == 1 {
208+
Sign::Negative
209+
} else {
210+
Sign::ZeroOrPositive
211+
}
212+
}
213+
198214
/// Peels binary operators such as [`BinOpKind::Mul`], [`BinOpKind::Div`] or [`BinOpKind::Rem`],
199215
/// which the result could always be positive under certain conditions, ignoring overflow.
200216
///
201217
/// Expressions using other operators are preserved, so we can try to evaluate them later.
202-
fn exprs_with_muldiv_binop_peeled<'a>(expr: &'a Expr<'_>) -> Vec<&'a Expr<'a>> {
218+
fn exprs_with_muldiv_binop_peeled<'e>(expr: &'e Expr<'_>) -> Vec<&'e Expr<'e>> {
203219
#[inline]
204-
fn collect_operands<'a>(expr: &'a Expr<'a>, operands: &mut Vec<&'a Expr<'a>>) {
220+
fn collect_operands<'e>(expr: &'e Expr<'e>, operands: &mut Vec<&'e Expr<'e>>) {
205221
match expr.kind {
206222
ExprKind::Binary(op, lhs, rhs) => {
207223
if matches!(op.node, BinOpKind::Mul | BinOpKind::Div) {

0 commit comments

Comments
 (0)