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

improve [cast_sign_loss], to skip warning on mathmatical expression that is always positive

Browse Source
This commit is contained in:
J-ZhengLi 2023-12-14 09:27:01 +08:00
parent 29bdc8b2bc
commit eae2317977
3 changed files with 238 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1,12 +1,14 @@
use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::span_lint;
use clippy_utils::{method_chain_args, sext};
use rustc_hir::{Expr, ExprKind};
use clippy_utils::{clip, method_chain_args, sext};
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty};
use rustc_middle::ty::{self, Ty, UintTy};
use super::CAST_SIGN_LOSS;
const METHODS_RET_POSITIVE: &[&str] = &["abs", "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) {
span_lint(
@ -25,33 +27,28 @@ fn should_lint(cx: &LateContext<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast
return false;
}
// Don't lint for positive constants.
let const_val = constant(cx, cx.typeck_results(), cast_op);
if let Some(Constant::Int(n)) = const_val
&& let ty::Int(ity) = *cast_from.kind()
&& sext(cx.tcx, n, ity) >= 0
{
// Don't lint if `cast_op` is known to be positive.
if let Sign::ZeroOrPositive = expr_sign(cx, cast_op, cast_from) {
return false;
}
// Don't lint for the result of methods that always return non-negative values.
if let ExprKind::MethodCall(path, ..) = cast_op.kind {
let mut method_name = path.ident.name.as_str();
let allowed_methods = ["abs", "checked_abs", "rem_euclid", "checked_rem_euclid"];
if method_name == "unwrap"
&& let Some(arglist) = method_chain_args(cast_op, &["unwrap"])
&& let ExprKind::MethodCall(inner_path, ..) = &arglist[0].0.kind
{
method_name = inner_path.ident.name.as_str();
}
if allowed_methods.iter().any(|&name| method_name == name) {
return false;
}
let (mut uncertain_count, mut negative_count) = (0, 0);
// Peel off possible binary expressions, e.g. x * x * y => [x, x, y]
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;
};
for expr in exprs {
let ty = cx.typeck_results().expr_ty(expr);
match expr_sign(cx, expr, ty) {
Sign::Negative => negative_count += 1,
Sign::Uncertain => uncertain_count += 1,
Sign::ZeroOrPositive => (),
};
}
true
// Lint if there are odd number of uncertain or negative results
uncertain_count % 2 == 1 || negative_count % 2 == 1
},
(false, true) => !cast_to.is_signed(),
@ -59,3 +56,97 @@ fn should_lint(cx: &LateContext<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast
(_, _) => false,
}
}
fn get_const_int_eval(cx: &LateContext<'_>, expr: &Expr<'_>, ty: Ty<'_>) -> Option<i128> {
if let Constant::Int(n) = constant(cx, cx.typeck_results(), expr)?
&& let ty::Int(ity) = *ty.kind()
{
return Some(sext(cx.tcx, n, ity));
}
None
}
enum Sign {
ZeroOrPositive,
Negative,
Uncertain,
}
fn expr_sign(cx: &LateContext<'_>, expr: &Expr<'_>, ty: Ty<'_>) -> Sign {
// Try evaluate this expr first to see if it's positive
if let Some(val) = get_const_int_eval(cx, expr, ty) {
return if val >= 0 { Sign::ZeroOrPositive } else { Sign::Negative };
}
// Calling on methods that always return non-negative values.
if let ExprKind::MethodCall(path, caller, args, ..) = expr.kind {
let mut method_name = path.ident.name.as_str();
if method_name == "unwrap"
&& let Some(arglist) = method_chain_args(expr, &["unwrap"])
&& let ExprKind::MethodCall(inner_path, ..) = &arglist[0].0.kind
{
method_name = inner_path.ident.name.as_str();
}
if method_name == "pow"
&& let [arg] = args
{
return pow_call_result_sign(cx, caller, arg);
} else if METHODS_RET_POSITIVE.iter().any(|&name| method_name == name) {
return Sign::ZeroOrPositive;
}
}
Sign::Uncertain
}
/// Return the sign of the `pow` call's result.
///
/// 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.
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
{
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;
}
Sign::Uncertain
}
/// Peels binary operators such as [`BinOpKind::Mul`], [`BinOpKind::Div`] or [`BinOpKind::Rem`],
/// which the result could always be positive under certain condition.
///
/// 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>>> {
#[inline]
fn collect_operands<'a>(expr: &'a Expr<'a>, operands: &mut Vec<&'a Expr<'a>>) -> Option<()> {
match expr.kind {
ExprKind::Binary(op, lhs, rhs) => {
if matches!(op.node, BinOpKind::Mul | BinOpKind::Div | BinOpKind::Rem) {
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;
}
},
_ => operands.push(expr),
}
Some(())
}
let mut res = vec![];
collect_operands(expr, &mut res)?;
Some(res)
}

49
tests/ui/cast.rs
View File

@ -365,3 +365,52 @@ fn avoid_subtract_overflow(q: u32) {
fn issue11426() {
(&42u8 >> 0xa9008fb6c9d81e42_0e25730562a601c8_u128) as usize;
}
fn issue11642() {
fn square(x: i16) -> u32 {
let x = x as i32;
(x * x) as u32;
x.pow(2) as u32;
(-2_i32).pow(2) as u32
}
let _a = |x: i32| -> u32 { (x * x * x * x) as u32 };
(-2_i32).pow(3) as u32;
//~^ ERROR: casting `i32` to `u32` may lose the sign of the value
let x: i32 = 10;
(x * x) as u32;
(x * x * x) as u32;
//~^ ERROR: casting `i32` to `u32` may lose the sign of the value
let y: i16 = -2;
(y * y * y * y * -2) as u16;
//~^ ERROR: casting `i16` to `u16` may lose the sign of the value
(y * y * y * y * 2) as u16;
(y * y * y * 2) as u16;
//~^ ERROR: casting `i16` to `u16` may lose the sign of the value
(y * y * y * -2) as u16;
//~^ ERROR: casting `i16` to `u16` may lose the sign of the value
fn foo(a: i32, b: i32, c: i32) -> u32 {
(a * a * b * b * c * c) as u32;
(a * b * c) as u32;
//~^ ERROR: casting `i32` to `u32` may lose the sign of the value
(a * -b * c) as u32;
//~^ ERROR: casting `i32` to `u32` may lose the sign of the value
(a * b * c * c) as u32;
(a * -2) as u32;
//~^ ERROR: casting `i32` to `u32` may lose the sign of the value
(a * b * c * -2) as u32;
//~^ ERROR: casting `i32` to `u32` may lose the sign of the value
(a / b) as u32;
(a / b * c) as u32;
//~^ ERROR: casting `i32` to `u32` may lose the sign of the value
(a / b + b * c) as u32;
//~^ ERROR: casting `i32` to `u32` may lose the sign of the value
a.pow(3) as u32;
//~^ ERROR: casting `i32` to `u32` may lose the sign of the value
(a.abs() * b.pow(2) / c.abs()) as u32
}
}

74
tests/ui/cast.stderr
View File

@ -444,5 +444,77 @@ help: ... or use `try_from` and handle the error accordingly
LL | let c = u8::try_from(q / 1000);
| ~~~~~~~~~~~~~~~~~~~~~~
error: aborting due to 51 previous errors
error: casting `i32` to `u32` may lose the sign of the value
--> $DIR/cast.rs:379:5
|
LL | (-2_i32).pow(3) as u32;
| ^^^^^^^^^^^^^^^^^^^^^^
error: casting `i32` to `u32` may lose the sign of the value
--> $DIR/cast.rs:384:5
|
LL | (x * x * x) as u32;
| ^^^^^^^^^^^^^^^^^^
error: casting `i16` to `u16` may lose the sign of the value
--> $DIR/cast.rs:388:5
|
LL | (y * y * y * y * -2) as u16;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: casting `i16` to `u16` may lose the sign of the value
--> $DIR/cast.rs:391:5
|
LL | (y * y * y * 2) as u16;
| ^^^^^^^^^^^^^^^^^^^^^^
error: casting `i16` to `u16` may lose the sign of the value
--> $DIR/cast.rs:393:5
|
LL | (y * y * y * -2) as u16;
| ^^^^^^^^^^^^^^^^^^^^^^^
error: casting `i32` to `u32` may lose the sign of the value
--> $DIR/cast.rs:398:9
|
LL | (a * b * c) as u32;
| ^^^^^^^^^^^^^^^^^^
error: casting `i32` to `u32` may lose the sign of the value
--> $DIR/cast.rs:400:9
|
LL | (a * -b * c) as u32;
| ^^^^^^^^^^^^^^^^^^^
error: casting `i32` to `u32` may lose the sign of the value
--> $DIR/cast.rs:403:9
|
LL | (a * -2) as u32;
| ^^^^^^^^^^^^^^^
error: casting `i32` to `u32` may lose the sign of the value
--> $DIR/cast.rs:405:9
|
LL | (a * b * c * -2) as u32;
| ^^^^^^^^^^^^^^^^^^^^^^^
error: casting `i32` to `u32` may lose the sign of the value
--> $DIR/cast.rs:408:9
|
LL | (a / b * c) as u32;
| ^^^^^^^^^^^^^^^^^^
error: casting `i32` to `u32` may lose the sign of the value
--> $DIR/cast.rs:410:9
|
LL | (a / b + b * c) as u32;
| ^^^^^^^^^^^^^^^^^^^^^^
error: casting `i32` to `u32` may lose the sign of the value
--> $DIR/cast.rs:412:9
|
LL | a.pow(3) as u32;
| ^^^^^^^^^^^^^^^
error: aborting due to 63 previous errors