diff --git a/clippy_lints/src/casts/cast_sign_loss.rs b/clippy_lints/src/casts/cast_sign_loss.rs index 1100625bc4a..9c18989701e 100644 --- a/clippy_lints/src/casts/cast_sign_loss.rs +++ b/clippy_lints/src/casts/cast_sign_loss.rs @@ -229,23 +229,30 @@ fn exprs_with_muldiv_binop_peeled<'e>(expr: &'e Expr<'_>) -> Vec<&'e Expr<'e>> { // For binary operators which both contribute to the sign of the result, // collect all their operands, recursively. This ignores overflow. ControlFlow::Continue(Descend::Yes) - } else if matches!(op.node, BinOpKind::Rem) { + } else if matches!(op.node, BinOpKind::Rem | BinOpKind::Shr) { // 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. // + // Large left shifts turn negatives into zeroes, so we can't use it here. + // // > Given remainder = dividend % divisor, the remainder will have the same sign as the dividend + // > ... + // > Arithmetic right shift on signed integer types // https://doc.rust-lang.org/reference/expressions/operator-expr.html#arithmetic-and-logical-binary-operators + + // We want to descend into the lhs, but skip the rhs. + // That's tricky to do using for_each_expr(), so we just keep the lhs intact. res.push(lhs); - ControlFlow::Break(()) + ControlFlow::Continue(Descend::No) } else { // The sign of the result of other binary operators depends on the values of the operands, // so try to evaluate the expression. - res.push(expr); + res.push(sub_expr); ControlFlow::Continue(Descend::No) } } else { // For other expressions, including unary operators and constants, try to evaluate the expression. - res.push(expr); + res.push(sub_expr); ControlFlow::Continue(Descend::No) } });