Put muldiv peeling in its own method

This commit is contained in:
teor 2024-01-16 18:33:20 +10:00
parent 11e0d6acca
commit 4ab2ed33a0

View file

@ -56,7 +56,15 @@ fn should_lint<'cx>(cx: &LateContext<'cx>, cast_op: &Expr<'_>, cast_from: Ty<'cx
}
// Don't lint if `cast_op` is known to be positive, ignoring overflow.
expr_sign(cx, cast_op, cast_from) == Sign::ZeroOrPositive
if let Sign::ZeroOrPositive = expr_sign(cx, cast_op, cast_from) {
return false;
}
if let Sign::ZeroOrPositive = expr_muldiv_sign(cx, cast_op) {
return false;
}
true
},
(false, true) => !cast_to.is_signed(),
@ -134,32 +142,7 @@ fn expr_sign<'cx>(cx: &LateContext<'cx>, expr: &Expr<'_>, ty: impl Into<Option<T
}
}
let mut uncertain_count = 0;
let mut negative_count = 0;
// Peel off possible binary expressions, for example:
// x * x / y => [x, x, y]
// a % b => [a]
let exprs = exprs_with_muldiv_binop_peeled(expr);
for expr in exprs {
match expr_sign(cx, expr, None) {
Sign::Negative => negative_count += 1,
Sign::Uncertain => uncertain_count += 1,
Sign::ZeroOrPositive => (),
};
}
// A mul/div is:
// - uncertain if there are any uncertain values (because they could be negative or positive),
// - negative if there are an odd number of negative values,
// - positive or zero otherwise.
if uncertain_count > 0 {
Sign::Uncertain
} else if negative_count % 2 == 1 {
Sign::Negative
} else {
Sign::ZeroOrPositive
}
Sign::Uncertain
}
/// 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<'
}
}
/// Peels binary operators such as [`BinOpKind::Mul`], [`BinOpKind::Div`] or [`BinOpKind::Rem`],
/// which the result could always be positive under certain conditions, ignoring overflow.
///
/// Returns the sign of the list of peeled expressions.
fn expr_muldiv_sign(cx: &LateContext<'_>, expr: &Expr<'_>) -> Sign {
let mut uncertain_count = 0;
let mut negative_count = 0;
// Peel off possible binary expressions, for example:
// x * x / y => [x, x, y]
// a % b => [a]
let exprs = exprs_with_muldiv_binop_peeled(expr);
for expr in exprs {
match expr_sign(cx, expr, None) {
Sign::Negative => negative_count += 1,
Sign::Uncertain => uncertain_count += 1,
Sign::ZeroOrPositive => (),
};
}
// A mul/div is:
// - uncertain if there are any uncertain values (because they could be negative or positive),
// - negative if there are an odd number of negative values,
// - positive or zero otherwise.
if uncertain_count > 0 {
Sign::Uncertain
} else if negative_count % 2 == 1 {
Sign::Negative
} else {
Sign::ZeroOrPositive
}
}
/// Peels binary operators such as [`BinOpKind::Mul`], [`BinOpKind::Div`] or [`BinOpKind::Rem`],
/// which the result could always be positive under certain conditions, ignoring overflow.
///
/// Expressions using other operators are preserved, so we can try to evaluate them later.
fn exprs_with_muldiv_binop_peeled<'a>(expr: &'a Expr<'_>) -> Vec<&'a Expr<'a>> {
fn exprs_with_muldiv_binop_peeled<'e>(expr: &'e Expr<'_>) -> Vec<&'e Expr<'e>> {
#[inline]
fn collect_operands<'a>(expr: &'a Expr<'a>, operands: &mut Vec<&'a Expr<'a>>) {
fn collect_operands<'e>(expr: &'e Expr<'e>, operands: &mut Vec<&'e Expr<'e>>) {
match expr.kind {
ExprKind::Binary(op, lhs, rhs) => {
if matches!(op.node, BinOpKind::Mul | BinOpKind::Div) {