about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-11-02 09:44:27 +0000
committerbors <bors@rust-lang.org>2021-11-02 09:44:27 +0000
commit4e355ebb6e8bc7269a37548f51743f897493a129 (patch)
treedf949ed79de05a562c80e7e3fd24ba967954c1ae
parent08b7e87843c711aacba3a6336dd2422d05d44d7d (diff)
parent00ea73e1627841fe3da7ba3699bbe99705063df1 (diff)
downloadrust-4e355ebb6e8bc7269a37548f51743f897493a129.tar.gz
rust-4e355ebb6e8bc7269a37548f51743f897493a129.zip
Auto merge of #7819 - rust-lang:avoid-linting-impossible-truncation, r=flip1995
avoid linting `possible_truncation` on bit-reducing operations

---

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: avoid linting `possible_truncation` on bit-reducing operations
-rw-r--r--clippy_lints/src/casts/cast_possible_truncation.rs81
-rw-r--r--tests/ui/cast.rs15
-rw-r--r--tests/ui/cast.stderr14
3 files changed, 101 insertions, 9 deletions
diff --git a/clippy_lints/src/casts/cast_possible_truncation.rs b/clippy_lints/src/casts/cast_possible_truncation.rs
index 2ae7d16e00b..4af412ccaf3 100644
--- a/clippy_lints/src/casts/cast_possible_truncation.rs
+++ b/clippy_lints/src/casts/cast_possible_truncation.rs
@@ -1,23 +1,88 @@
+use clippy_utils::consts::{constant, Constant};
 use clippy_utils::diagnostics::span_lint;
 use clippy_utils::expr_or_init;
 use clippy_utils::ty::is_isize_or_usize;
-use rustc_hir::{Expr, ExprKind};
+use rustc_hir::{BinOpKind, Expr, ExprKind};
 use rustc_lint::LateContext;
 use rustc_middle::ty::{self, FloatTy, Ty};
 
 use super::{utils, CAST_POSSIBLE_TRUNCATION};
 
-pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
-    // do not lint if cast comes from a `signum` function
-    if let ExprKind::MethodCall(path, ..) = expr_or_init(cx, cast_expr).kind {
-        if path.ident.name.as_str() == "signum" {
-            return;
-        }
+fn constant_int(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<u128> {
+    if let Some((Constant::Int(c), _)) = constant(cx, cx.typeck_results(), expr) {
+        Some(c)
+    } else {
+        None
     }
+}
 
+fn get_constant_bits(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<u64> {
+    constant_int(cx, expr).map(|c| u64::from(128 - c.leading_zeros()))
+}
+
+fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: bool) -> u64 {
+    match expr_or_init(cx, expr).kind {
+        ExprKind::Cast(inner, _) => apply_reductions(cx, nbits, inner, signed),
+        ExprKind::Block(block, _) => block.expr.map_or(nbits, |e| apply_reductions(cx, nbits, e, signed)),
+        ExprKind::Binary(op, left, right) => match op.node {
+            BinOpKind::Div => {
+                apply_reductions(cx, nbits, left, signed)
+                    - (if signed {
+                        0 // let's be conservative here
+                    } else {
+                        // by dividing by 1, we remove 0 bits, etc.
+                        get_constant_bits(cx, right).map_or(0, |b| b.saturating_sub(1))
+                    })
+            },
+            BinOpKind::Rem | BinOpKind::BitAnd => get_constant_bits(cx, right)
+                .unwrap_or(u64::max_value())
+                .min(apply_reductions(cx, nbits, left, signed)),
+            BinOpKind::Shr => {
+                apply_reductions(cx, nbits, left, signed)
+                    - constant_int(cx, right).map_or(0, |s| u64::try_from(s).expect("shift too high"))
+            },
+            _ => nbits,
+        },
+        ExprKind::MethodCall(method, _, [left, right], _) => {
+            if signed {
+                return nbits;
+            }
+            let max_bits = if method.ident.as_str() == "min" {
+                get_constant_bits(cx, right)
+            } else {
+                None
+            };
+            apply_reductions(cx, nbits, left, signed).min(max_bits.unwrap_or(u64::max_value()))
+        },
+        ExprKind::MethodCall(method, _, [_, lo, hi], _) => {
+            if method.ident.as_str() == "clamp" {
+                //FIXME: make this a diagnostic item
+                if let (Some(lo_bits), Some(hi_bits)) = (get_constant_bits(cx, lo), get_constant_bits(cx, hi)) {
+                    return lo_bits.max(hi_bits);
+                }
+            }
+            nbits
+        },
+        ExprKind::MethodCall(method, _, [_value], _) => {
+            if method.ident.name.as_str() == "signum" {
+                0 // do not lint if cast comes from a `signum` function
+            } else {
+                nbits
+            }
+        },
+        _ => nbits,
+    }
+}
+
+pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
     let msg = match (cast_from.is_integral(), cast_to.is_integral()) {
         (true, true) => {
-            let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
+            let from_nbits = apply_reductions(
+                cx,
+                utils::int_ty_to_nbits(cast_from, cx.tcx),
+                cast_expr,
+                cast_from.is_signed(),
+            );
             let to_nbits = utils::int_ty_to_nbits(cast_to, cx.tcx);
 
             let (should_lint, suffix) = match (is_isize_or_usize(cast_from), is_isize_or_usize(cast_to)) {
diff --git a/tests/ui/cast.rs b/tests/ui/cast.rs
index adc95e63ae5..ebc1ed5587f 100644
--- a/tests/ui/cast.rs
+++ b/tests/ui/cast.rs
@@ -100,4 +100,19 @@ fn main() {
 
     let s = x.signum();
     let _ = s as i32;
+
+    // Test for signed min
+    (-99999999999i64).min(1) as i8; // should be linted because signed
+
+    // Test for various operations that remove enough bits for the result to fit
+    (999999u64 & 1) as u8;
+    (999999u64 % 15) as u8;
+    (999999u64 / 0x1_0000_0000_0000) as u16;
+    ({ 999999u64 >> 56 }) as u8;
+    ({
+        let x = 999999u64;
+        x.min(1)
+    }) as u8;
+    999999u64.clamp(0, 255) as u8;
+    999999u64.clamp(0, 256) as u8; // should still be linted
 }
diff --git a/tests/ui/cast.stderr b/tests/ui/cast.stderr
index 4c66d736494..edf8790cf33 100644
--- a/tests/ui/cast.stderr
+++ b/tests/ui/cast.stderr
@@ -138,5 +138,17 @@ error: casting `isize` to `usize` may lose the sign of the value
 LL |     -1isize as usize;
    |     ^^^^^^^^^^^^^^^^
 
-error: aborting due to 22 previous errors
+error: casting `i64` to `i8` may truncate the value
+  --> $DIR/cast.rs:105:5
+   |
+LL |     (-99999999999i64).min(1) as i8; // should be linted because signed
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: casting `u64` to `u8` may truncate the value
+  --> $DIR/cast.rs:117:5
+   |
+LL |     999999u64.clamp(0, 256) as u8; // should still be linted
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 24 previous errors