Skip to content

Commit c5e8487

Browse files
committed
Fix pow() to return more known signs
1 parent 47339d0 commit c5e8487

File tree

1 file changed

+38
-31
lines changed

1 file changed

+38
-31
lines changed

clippy_lints/src/casts/cast_sign_loss.rs

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
use clippy_utils::consts::{constant, Constant};
22
use clippy_utils::diagnostics::span_lint;
3-
use clippy_utils::{clip, method_chain_args, sext};
3+
use clippy_utils::{method_chain_args, sext};
44
use rustc_hir::{BinOpKind, Expr, ExprKind};
55
use rustc_lint::LateContext;
6-
use rustc_middle::ty::{self, Ty, UintTy};
6+
use rustc_middle::ty::{self, Ty};
77

88
use super::CAST_SIGN_LOSS;
99

@@ -22,10 +22,7 @@ const METHODS_RET_POSITIVE: &[&str] = &[
2222
"wrapping_rem_euclid",
2323
];
2424

25-
/// A list of methods that act like `pow()`, and can never return:
26-
/// - a negative value from a non-negative base
27-
/// - a negative value from a negative base and even exponent
28-
/// - a non-negative value from a negative base and odd exponent
25+
/// A list of methods that act like `pow()`. See `pow_call_result_sign()` for details.
2926
///
3027
/// Methods that can overflow and return a negative value must not be included in this list,
3128
/// because casting their return values can still result in sign loss.
@@ -34,7 +31,13 @@ const METHODS_POW: &[&str] = &["pow", "saturating_pow", "checked_pow"];
3431
/// A list of methods that act like `unwrap()`, and don't change the sign of the inner value.
3532
const METHODS_UNWRAP: &[&str] = &["unwrap", "unwrap_unchecked", "expect", "into_ok"];
3633

37-
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
34+
pub(super) fn check<'cx>(
35+
cx: &LateContext<'cx>,
36+
expr: &Expr<'_>,
37+
cast_op: &Expr<'_>,
38+
cast_from: Ty<'cx>,
39+
cast_to: Ty<'_>,
40+
) {
3841
if should_lint(cx, cast_op, cast_from, cast_to) {
3942
span_lint(
4043
cx,
@@ -45,7 +48,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_op: &Expr<'_>, c
4548
}
4649
}
4750

48-
fn should_lint(cx: &LateContext<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) -> bool {
51+
fn should_lint<'cx>(cx: &LateContext<'cx>, cast_op: &Expr<'_>, cast_from: Ty<'cx>, cast_to: Ty<'_>) -> bool {
4952
match (cast_from.is_integral(), cast_to.is_integral()) {
5053
(true, true) => {
5154
if !cast_from.is_signed() || cast_to.is_signed() {
@@ -82,7 +85,9 @@ fn should_lint(cx: &LateContext<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast
8285
}
8386
}
8487

85-
fn get_const_int_eval(cx: &LateContext<'_>, expr: &Expr<'_>, ty: Ty<'_>) -> Option<i128> {
88+
fn get_const_int_eval<'cx>(cx: &LateContext<'cx>, expr: &Expr<'_>, ty: impl Into<Option<Ty<'cx>>>) -> Option<i128> {
89+
let ty = ty.into().unwrap_or_else(|| cx.typeck_results().expr_ty(expr));
90+
8691
if let Constant::Int(n) = constant(cx, cx.typeck_results(), expr)?
8792
&& let ty::Int(ity) = *ty.kind()
8893
{
@@ -97,7 +102,7 @@ enum Sign {
97102
Uncertain,
98103
}
99104

100-
fn expr_sign(cx: &LateContext<'_>, expr: &Expr<'_>, ty: Ty<'_>) -> Sign {
105+
fn expr_sign<'cx>(cx: &LateContext<'cx>, expr: &Expr<'_>, ty: impl Into<Option<Ty<'cx>>>) -> Sign {
101106
// Try evaluate this expr first to see if it's positive
102107
if let Some(val) = get_const_int_eval(cx, expr, ty) {
103108
return if val >= 0 { Sign::ZeroOrPositive } else { Sign::Negative };
@@ -112,6 +117,8 @@ fn expr_sign(cx: &LateContext<'_>, expr: &Expr<'_>, ty: Ty<'_>) -> Sign {
112117
&& let Some(arglist) = method_chain_args(expr, &[found_name])
113118
&& let ExprKind::MethodCall(inner_path, ..) = &arglist[0].0.kind
114119
{
120+
// The original type has changed, but we can't use `ty` here anyway, because it has been
121+
// moved.
115122
method_name = inner_path.ident.name.as_str();
116123
}
117124

@@ -130,31 +137,31 @@ fn expr_sign(cx: &LateContext<'_>, expr: &Expr<'_>, ty: Ty<'_>) -> Sign {
130137
/// Return the sign of the `pow` call's result, ignoring overflow.
131138
///
132139
/// If the base is positive, the result is always positive.
133-
/// If the base is negative, and the exponent is a even number, the result is always positive,
134-
/// otherwise if the exponent is an odd number, the result is always negative.
140+
/// If the exponent is a even number, the result is always positive,
141+
/// Otherwise, if the base is negative, and the exponent is an odd number, the result is always
142+
/// negative.
135143
///
136-
/// If either value can't be evaluated, [`Sign::Uncertain`] will be returned.
144+
/// Otherwise, returns [`Sign::Uncertain`].
137145
fn pow_call_result_sign(cx: &LateContext<'_>, base: &Expr<'_>, exponent: &Expr<'_>) -> Sign {
138-
let base_ty = cx.typeck_results().expr_ty(base);
139-
let Some(base_val) = get_const_int_eval(cx, base, base_ty) else {
140-
return Sign::Uncertain;
141-
};
142-
// Non-negative bases raised to non-negative exponents are always non-negative, ignoring overflow.
143-
// (Rust's integer pow() function takes an unsigned exponent.)
144-
if base_val >= 0 {
145-
return Sign::ZeroOrPositive;
146-
}
146+
let base_sign = expr_sign(cx, base, None);
147+
let exponent_val = get_const_int_eval(cx, exponent, None);
148+
let exponent_is_even = exponent_val.map(|val| val % 2 == 0);
149+
150+
match (base_sign, exponent_is_even) {
151+
// Non-negative bases always return non-negative results, ignoring overflow.
152+
// This is because Rust's integer pow() functions take an unsigned exponent.
153+
(Sign::ZeroOrPositive, _) => Sign::ZeroOrPositive,
154+
155+
// Any base raised to an even exponent is non-negative.
156+
// This is true even if we don't know the value of the base.
157+
(_, Some(true)) => Sign::ZeroOrPositive,
147158

148-
let Some(Constant::Int(n)) = constant(cx, cx.typeck_results(), exponent) else {
149-
return Sign::Uncertain;
150-
};
159+
// A negative base raised to an odd exponent is non-negative.
160+
(Sign::Negative, Some(false)) => Sign::Negative,
151161

152-
// A negative value raised to an even exponent is non-negative, and an odd exponent
153-
// is negative, ignoring overflow.
154-
if clip(cx.tcx, n, UintTy::U32) % 2 == 0 {
155-
return Sign::ZeroOrPositive;
156-
} else {
157-
return Sign::Negative;
162+
// Negative or unknown base to an unknown exponent, or an unknown base to an odd exponent.
163+
(Sign::Negative | Sign::Uncertain, None) => Sign::Uncertain,
164+
(Sign::Uncertain, Some(false)) => Sign::Uncertain,
158165
}
159166
}
160167

0 commit comments

Comments
 (0)