mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix uncertain sign and remainder op handling in cast_sign_loss.rs

Browse Source
This commit is contained in:
teor 2024-01-11 20:26:07 +10:00
parent 6aa5f1ac6f
commit ee8fd82f4c
1 changed files with 61 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

92
clippy_lints/src/casts/cast_sign_loss.rs
View File

@ -7,7 +7,12 @@
use super::CAST_SIGN_LOSS;
const METHODS_RET_POSITIVE: &[&str] = &["abs", "checked_abs", "rem_euclid", "checked_rem_euclid"];
/// A list of methods that can never return a negative value.
/// Includes methods that panic rather than returning a negative value.
///
/// Methods that can overflow and return a negative value must not be included in this list,
/// because checking for negative return values from those functions can be useful.
const METHODS_RET_POSITIVE: &[&str] = &["checked_abs", "rem_euclid", "checked_rem_euclid"];
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
if should_lint(cx, cast_op, cast_from, cast_to) {
@ -27,13 +32,15 @@ fn should_lint(cx: &LateContext<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast
return false;
}
// Don't lint if `cast_op` is known to be positive.
// Don't lint if `cast_op` is known to be positive, ignoring overflow.
if let Sign::ZeroOrPositive = expr_sign(cx, cast_op, cast_from) {
return false;
}
let (mut uncertain_count, mut negative_count) = (0, 0);
// Peel off possible binary expressions, e.g. x * x * y => [x, x, y]
// Peel off possible binary expressions, for example:
// x * x * y => [x, x, y]
// a % b => [a]
let Some(exprs) = exprs_with_selected_binop_peeled(cast_op) else {
// Assume cast sign lose if we cannot determine the sign of `cast_op`
return true;
@ -47,8 +54,9 @@ fn should_lint(cx: &LateContext<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast
};
}
// Lint if there are odd number of uncertain or negative results
uncertain_count % 2 == 1 || negative_count % 2 == 1
// Lint if there are any uncertain results (because they could be negative or positive),
// or an odd number of negative results.
uncertain_count > 0 || negative_count % 2 == 1
},
(false, true) => !cast_to.is_signed(),
@ -87,6 +95,12 @@ fn expr_sign(cx: &LateContext<'_>, expr: &Expr<'_>, ty: Ty<'_>) -> Sign {
{
method_name = inner_path.ident.name.as_str();
}
if method_name == "expect"
&& let Some(arglist) = method_chain_args(expr, &["expect"])
&& let ExprKind::MethodCall(inner_path, ..) = &arglist[0].0.kind
{
method_name = inner_path.ident.name.as_str();
}
if method_name == "pow"
&& let [arg] = args
@ -100,53 +114,69 @@ fn expr_sign(cx: &LateContext<'_>, expr: &Expr<'_>, ty: Ty<'_>) -> Sign {
Sign::Uncertain
}
/// Return the sign of the `pow` call's result.
/// Return the sign of the `pow` call's result, ignoring overflow.
///
/// If the caller is a positive number, the result is always positive,
/// If the `power_of` is a even number, the result is always positive as well,
/// Otherwise a [`Sign::Uncertain`] will be returned.
/// If the base is positive, the result is always positive.
/// If the base is negative, and the exponent is a even number, the result is always positive,
/// otherwise if the exponent is an odd number, the result is always negative.
///
/// If either value can't be evaluated, [`Sign::Uncertain`] will be returned.
fn pow_call_result_sign(cx: &LateContext<'_>, caller: &Expr<'_>, power_of: &Expr<'_>) -> Sign {
let caller_ty = cx.typeck_results().expr_ty(caller);
if let Some(caller_val) = get_const_int_eval(cx, caller, caller_ty)
&& caller_val >= 0
{
let Some(caller_val) = get_const_int_eval(cx, caller, caller_ty) else {
return Sign::Uncertain;
}
// Non-negative values raised to non-negative exponents are always non-negative, ignoring overflow.
// (Rust's integer pow() function takes an unsigned exponent.)
if caller_val >= 0 {
return Sign::ZeroOrPositive;
}
if let Some(Constant::Int(n)) = constant(cx, cx.typeck_results(), power_of)
&& clip(cx.tcx, n, UintTy::U32) % 2 == 0
{
return Sign::ZeroOrPositive;
let Some(Constant::Int(n)) = constant(cx, cx.typeck_results(), power_of) else {
return Sign::Uncertain;
}
// A negative value raised to an even exponent is non-negative, and an odd exponent
// is negative, ignoring overflow.
if clip(cx.tcx, n, UintTy::U32) % 2 == 0 0 {
return Sign::ZeroOrPositive;
} else {
return Sign::Negative;
}
Sign::Uncertain
}
/// Peels binary operators such as [`BinOpKind::Mul`], [`BinOpKind::Div`] or [`BinOpKind::Rem`],
/// which the result could always be positive under certain condition.
/// which the result could always be positive under certain conditions, ignoring overflow.
///
/// Other operators such as `+`/`-` causing the result's sign hard to determine, which we will
/// return `None`
fn exprs_with_selected_binop_peeled<'a>(expr: &'a Expr<'_>) -> Option<Vec<&'a Expr<'a>>> {
/// Expressions using other operators are preserved, so we can try to evaluate them later.
fn exprs_with_selected_binop_peeled<'a>(expr: &'a Expr<'_>) -> Vec<&'a Expr<'a>> {
#[inline]
fn collect_operands<'a>(expr: &'a Expr<'a>, operands: &mut Vec<&'a Expr<'a>>) -> Option<()> {
fn collect_operands<'a>(expr: &'a Expr<'a>, operands: &mut Vec<&'a Expr<'a>>) {
match expr.kind {
ExprKind::Binary(op, lhs, rhs) => {
if matches!(op.node, BinOpKind::Mul | BinOpKind::Div | BinOpKind::Rem) {
if matches!(op.node, BinOpKind::Mul | BinOpKind::Div) {
// For binary operators which both contribute to the sign of the result,
// collect all their operands, recursively. This ignores overflow.
collect_operands(lhs, operands);
collect_operands(rhs, operands);
} else if matches!(op.node, BinOpKind::Rem) {
// For binary operators where the left hand side determines the sign of the result,
// only collect that side, recursively. Overflow panics, so this always holds.
//
// > Given remainder = dividend % divisor, the remainder will have the same sign as the dividend
// https://doc.rust-lang.org/reference/expressions/operator-expr.html#arithmetic-and-logical-binary-operators
collect_operands(lhs, operands);
operands.push(rhs);
} else {
// Things are complicated when there are other binary ops exist,
// abort checking by returning `None` for now.
return None;
// The sign of the result of other binary operators depends on the values of the operands,
// so try to evaluate the expression.
operands.push(expr);
}
},
// For other expressions, including unary operators and constants, try to evaluate the expression.
_ => operands.push(expr),
}
Some(())
}
let mut res = vec![];
collect_operands(expr, &mut res)?;
Some(res)
collect_operands(expr, &mut res);
res
}