From 4ab2ed33a07c9b2c76430b8221f78b6efd9fde79 Mon Sep 17 00:00:00 2001
From: teor <teor@riseup.net>
Date: Tue, 16 Jan 2024 18:33:20 +1000
Subject: [PATCH] Put muldiv peeling in its own method

---
 clippy_lints/src/casts/cast_sign_loss.rs | 74 ++++++++++++++----------
 1 file changed, 45 insertions(+), 29 deletions(-)

diff --git a/clippy_lints/src/casts/cast_sign_loss.rs b/clippy_lints/src/casts/cast_sign_loss.rs
index 7e4efbbc0..db7121928 100644
--- a/clippy_lints/src/casts/cast_sign_loss.rs
+++ b/clippy_lints/src/casts/cast_sign_loss.rs
@@ -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) {