Auto merge of #10564 - asquared31415:cast_doesnt_wrap, r=giraffate

bors · bors · commit c8a056547b1b · 2023-06-06T23:54:27.000Z
make cast_possible_wrap work correctly for 16 bit {u,i}size These changes make `cast_possible_wrap` aware of the different pointer widths and fixes the implementation to print the correct pointer widths. Fixes #9337 changelog: `cast_possible_wrap` does not lint on `u8 as isize` or `usize as i8`, since these can never wrap. `cast_possible_wrap` now properly considers 16 bit pointer size and prints the correct bit widths.
diff --git a/clippy_lints/src/casts/cast_possible_wrap.rs b/clippy_lints/src/casts/cast_possible_wrap.rs
@@ -1,41 +1,89 @@
-use clippy_utils::diagnostics::span_lint;
-use clippy_utils::ty::is_isize_or_usize;
 use rustc_hir::Expr;
-use rustc_lint::LateContext;
+use rustc_lint::{LateContext, LintContext};
 use rustc_middle::ty::Ty;
 
 use super::{utils, CAST_POSSIBLE_WRAP};
 
+// this should be kept in sync with the allowed bit widths of `usize` and `isize`
+const ALLOWED_POINTER_SIZES: [u64; 3] = [16, 32, 64];
+
+// whether the lint should be emitted, and the required pointer size, if it matters
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+enum EmitState {
+    NoLint,
+    LintAlways,
+    LintOnPtrSize(u64),
+}
+
 pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
     if !(cast_from.is_integral() && cast_to.is_integral()) {
         return;
     }
 
-    let arch_64_suffix = " on targets with 64-bit wide pointers";
-    let arch_32_suffix = " on targets with 32-bit wide pointers";
-    let cast_unsigned_to_signed = !cast_from.is_signed() && cast_to.is_signed();
+    // emit a lint if a cast is:
+    // 1. unsigned to signed
+    // and
+    // 2. either:
+    //    2a. between two types of constant size that are always the same size
+    //    2b. between one target-dependent size and one constant size integer,
+    //        and the constant integer is in the allowed set of target dependent sizes
+    //        (the ptr size could be chosen to be the same as the constant size)
+
+    if cast_from.is_signed() || !cast_to.is_signed() {
+        return;
+    }
+
     let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
     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)) {
-        (true, true) | (false, false) => (to_nbits == from_nbits && cast_unsigned_to_signed, ""),
-        (true, false) => (to_nbits <= 32 && cast_unsigned_to_signed, arch_32_suffix),
-        (false, true) => (
-            cast_unsigned_to_signed,
-            if from_nbits == 64 {
-                arch_64_suffix
+    let should_lint = match (cast_from.is_ptr_sized_integral(), cast_to.is_ptr_sized_integral()) {
+        (true, true) => {
+            // casts between two ptr sized integers are trivially always the same size
+            // so do not depend on any specific pointer size to be the same
+            EmitState::LintAlways
+        },
+        (true, false) => {
+            // the first type is `usize` and the second is a constant sized signed integer
+            if ALLOWED_POINTER_SIZES.contains(&to_nbits) {
+                EmitState::LintOnPtrSize(to_nbits)
+            } else {
+                EmitState::NoLint
+            }
+        },
+        (false, true) => {
+            // the first type is a constant sized unsigned integer, and the second is `isize`
+            if ALLOWED_POINTER_SIZES.contains(&from_nbits) {
+                EmitState::LintOnPtrSize(from_nbits)
+            } else {
+                EmitState::NoLint
+            }
+        },
+        (false, false) => {
+            // the types are both a constant known size
+            // and do not depend on any specific pointer size to be the same
+            if from_nbits == to_nbits {
+                EmitState::LintAlways
             } else {
-                arch_32_suffix
-            },
+                EmitState::NoLint
+            }
+        },
+    };
+
+    let message = match should_lint {
+        EmitState::NoLint => return,
+        EmitState::LintAlways => format!("casting `{cast_from}` to `{cast_to}` may wrap around the value"),
+        EmitState::LintOnPtrSize(ptr_size) => format!(
+            "casting `{cast_from}` to `{cast_to}` may wrap around the value on targets with {ptr_size}-bit wide pointers",
         ),
     };
 
-    if should_lint {
-        span_lint(
-            cx,
-            CAST_POSSIBLE_WRAP,
-            expr.span,
-            &format!("casting `{cast_from}` to `{cast_to}` may wrap around the value{suffix}",),
-        );
-    }
+    cx.struct_span_lint(CAST_POSSIBLE_WRAP, expr.span, message, |diag| {
+        if let EmitState::LintOnPtrSize(16) = should_lint {
+            diag
+            .note("`usize` and `isize` may be as small as 16 bits on some platforms")
+            .note("for more information see https://doc.rust-lang.org/reference/types/numeric.html#machine-dependent-integer-types")
+        } else {
+            diag
+        }
+    });
 }
diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs
@@ -118,9 +118,10 @@ declare_clippy_lint! {
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for casts from an unsigned type to a signed type of
-    /// the same size. Performing such a cast is a 'no-op' for the compiler,
-    /// i.e., nothing is changed at the bit level, and the binary representation of
-    /// the value is reinterpreted. This can cause wrapping if the value is too big
+    /// the same size, or possibly smaller due to target dependent integers.
+    /// Performing such a cast is a 'no-op' for the compiler, i.e., nothing is
+    /// changed at the bit level, and the binary representation of the value is
+    /// reinterpreted. This can cause wrapping if the value is too big
     /// for the target signed type. However, the cast works as defined, so this lint
     /// is `Allow` by default.
     ///
diff --git a/tests/ui/cast.rs b/tests/ui/cast.rs
@@ -41,6 +41,14 @@ fn main() {
     1u32 as i32;
     1u64 as i64;
     1usize as isize;
+    1usize as i8; // should not wrap, usize is never 8 bits
+    1usize as i16; // wraps on 16 bit ptr size
+    1usize as i32; // wraps on 32 bit ptr size
+    1usize as i64; // wraps on 64 bit ptr size
+    1u8 as isize; // should not wrap, isize is never 8 bits
+    1u16 as isize; // wraps on 16 bit ptr size
+    1u32 as isize; // wraps on 32 bit ptr size
+    1u64 as isize; // wraps on 64 bit ptr size
     // Test clippy::cast_sign_loss
     1i32 as u32;
     -1i32 as u32;
diff --git a/tests/ui/cast.stderr b/tests/ui/cast.stderr
@@ -215,20 +215,110 @@ error: casting `usize` to `isize` may wrap around the value
 LL |     1usize as isize;
    |     ^^^^^^^^^^^^^^^
 
-error: casting `i32` to `u32` may lose the sign of the value
+error: casting `usize` to `i8` may truncate the value
+  --> $DIR/cast.rs:44:5
+   |
+LL |     1usize as i8; // should not wrap, usize is never 8 bits
+   |     ^^^^^^^^^^^^
+   |
+   = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
+help: ... or use `try_from` and handle the error accordingly
+   |
+LL |     i8::try_from(1usize); // should not wrap, usize is never 8 bits
+   |     ~~~~~~~~~~~~~~~~~~~~
+
+error: casting `usize` to `i16` may truncate the value
+  --> $DIR/cast.rs:45:5
+   |
+LL |     1usize as i16; // wraps on 16 bit ptr size
+   |     ^^^^^^^^^^^^^
+   |
+   = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
+help: ... or use `try_from` and handle the error accordingly
+   |
+LL |     i16::try_from(1usize); // wraps on 16 bit ptr size
+   |     ~~~~~~~~~~~~~~~~~~~~~
+
+error: casting `usize` to `i16` may wrap around the value on targets with 16-bit wide pointers
+  --> $DIR/cast.rs:45:5
+   |
+LL |     1usize as i16; // wraps on 16 bit ptr size
+   |     ^^^^^^^^^^^^^
+   |
+   = note: `usize` and `isize` may be as small as 16 bits on some platforms
+   = note: for more information see https://doc.rust-lang.org/reference/types/numeric.html#machine-dependent-integer-types
+
+error: casting `usize` to `i32` may truncate the value on targets with 64-bit wide pointers
   --> $DIR/cast.rs:46:5
    |
+LL |     1usize as i32; // wraps on 32 bit ptr size
+   |     ^^^^^^^^^^^^^
+   |
+   = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
+help: ... or use `try_from` and handle the error accordingly
+   |
+LL |     i32::try_from(1usize); // wraps on 32 bit ptr size
+   |     ~~~~~~~~~~~~~~~~~~~~~
+
+error: casting `usize` to `i32` may wrap around the value on targets with 32-bit wide pointers
+  --> $DIR/cast.rs:46:5
+   |
+LL |     1usize as i32; // wraps on 32 bit ptr size
+   |     ^^^^^^^^^^^^^
+
+error: casting `usize` to `i64` may wrap around the value on targets with 64-bit wide pointers
+  --> $DIR/cast.rs:47:5
+   |
+LL |     1usize as i64; // wraps on 64 bit ptr size
+   |     ^^^^^^^^^^^^^
+
+error: casting `u16` to `isize` may wrap around the value on targets with 16-bit wide pointers
+  --> $DIR/cast.rs:49:5
+   |
+LL |     1u16 as isize; // wraps on 16 bit ptr size
+   |     ^^^^^^^^^^^^^
+   |
+   = note: `usize` and `isize` may be as small as 16 bits on some platforms
+   = note: for more information see https://doc.rust-lang.org/reference/types/numeric.html#machine-dependent-integer-types
+
+error: casting `u32` to `isize` may wrap around the value on targets with 32-bit wide pointers
+  --> $DIR/cast.rs:50:5
+   |
+LL |     1u32 as isize; // wraps on 32 bit ptr size
+   |     ^^^^^^^^^^^^^
+
+error: casting `u64` to `isize` may truncate the value on targets with 32-bit wide pointers
+  --> $DIR/cast.rs:51:5
+   |
+LL |     1u64 as isize; // wraps on 64 bit ptr size
+   |     ^^^^^^^^^^^^^
+   |
+   = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
+help: ... or use `try_from` and handle the error accordingly
+   |
+LL |     isize::try_from(1u64); // wraps on 64 bit ptr size
+   |     ~~~~~~~~~~~~~~~~~~~~~
+
+error: casting `u64` to `isize` may wrap around the value on targets with 64-bit wide pointers
+  --> $DIR/cast.rs:51:5
+   |
+LL |     1u64 as isize; // wraps on 64 bit ptr size
+   |     ^^^^^^^^^^^^^
+
+error: casting `i32` to `u32` may lose the sign of the value
+  --> $DIR/cast.rs:54:5
+   |
 LL |     -1i32 as u32;
    |     ^^^^^^^^^^^^
 
 error: casting `isize` to `usize` may lose the sign of the value
-  --> $DIR/cast.rs:48:5
+  --> $DIR/cast.rs:56:5
    |
 LL |     -1isize as usize;
    |     ^^^^^^^^^^^^^^^^
 
 error: casting `i64` to `i8` may truncate the value
-  --> $DIR/cast.rs:115:5
+  --> $DIR/cast.rs:123:5
    |
 LL |     (-99999999999i64).min(1) as i8; // should be linted because signed
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -240,7 +330,7 @@ LL |     i8::try_from((-99999999999i64).min(1)); // should be linted because sig
    |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 error: casting `u64` to `u8` may truncate the value
-  --> $DIR/cast.rs:127:5
+  --> $DIR/cast.rs:135:5
    |
 LL |     999999u64.clamp(0, 256) as u8; // should still be linted
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -252,7 +342,7 @@ LL |     u8::try_from(999999u64.clamp(0, 256)); // should still be linted
    |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 error: casting `main::E2` to `u8` may truncate the value
-  --> $DIR/cast.rs:148:21
+  --> $DIR/cast.rs:156:21
    |
 LL |             let _ = self as u8;
    |                     ^^^^^^^^^^
@@ -264,15 +354,15 @@ LL |             let _ = u8::try_from(self);
    |                     ~~~~~~~~~~~~~~~~~~
 
 error: casting `main::E2::B` to `u8` will truncate the value
-  --> $DIR/cast.rs:149:21
+  --> $DIR/cast.rs:157:21
    |
 LL |             let _ = Self::B as u8;
    |                     ^^^^^^^^^^^^^
    |
    = note: `-D clippy::cast-enum-truncation` implied by `-D warnings`
 
 error: casting `main::E5` to `i8` may truncate the value
-  --> $DIR/cast.rs:185:21
+  --> $DIR/cast.rs:193:21
    |
 LL |             let _ = self as i8;
    |                     ^^^^^^^^^^
@@ -284,13 +374,13 @@ LL |             let _ = i8::try_from(self);
    |                     ~~~~~~~~~~~~~~~~~~
 
 error: casting `main::E5::A` to `i8` will truncate the value
-  --> $DIR/cast.rs:186:21
+  --> $DIR/cast.rs:194:21
    |
 LL |             let _ = Self::A as i8;
    |                     ^^^^^^^^^^^^^
 
 error: casting `main::E6` to `i16` may truncate the value
-  --> $DIR/cast.rs:200:21
+  --> $DIR/cast.rs:208:21
    |
 LL |             let _ = self as i16;
    |                     ^^^^^^^^^^^
@@ -302,7 +392,7 @@ LL |             let _ = i16::try_from(self);
    |                     ~~~~~~~~~~~~~~~~~~~
 
 error: casting `main::E7` to `usize` may truncate the value on targets with 32-bit wide pointers
-  --> $DIR/cast.rs:215:21
+  --> $DIR/cast.rs:223:21
    |
 LL |             let _ = self as usize;
    |                     ^^^^^^^^^^^^^
@@ -314,7 +404,7 @@ LL |             let _ = usize::try_from(self);
    |                     ~~~~~~~~~~~~~~~~~~~~~
 
 error: casting `main::E10` to `u16` may truncate the value
-  --> $DIR/cast.rs:256:21
+  --> $DIR/cast.rs:264:21
    |
 LL |             let _ = self as u16;
    |                     ^^^^^^^^^^^
@@ -326,7 +416,7 @@ LL |             let _ = u16::try_from(self);
    |                     ~~~~~~~~~~~~~~~~~~~
 
 error: casting `u32` to `u8` may truncate the value
-  --> $DIR/cast.rs:264:13
+  --> $DIR/cast.rs:272:13
    |
 LL |     let c = (q >> 16) as u8;
    |             ^^^^^^^^^^^^^^^
@@ -338,7 +428,7 @@ LL |     let c = u8::try_from(q >> 16);
    |             ~~~~~~~~~~~~~~~~~~~~~
 
 error: casting `u32` to `u8` may truncate the value
-  --> $DIR/cast.rs:267:13
+  --> $DIR/cast.rs:275:13
    |
 LL |     let c = (q / 1000) as u8;
    |             ^^^^^^^^^^^^^^^^
@@ -349,5 +439,5 @@ help: ... or use `try_from` and handle the error accordingly
 LL |     let c = u8::try_from(q / 1000);
    |             ~~~~~~~~~~~~~~~~~~~~~~
 
-error: aborting due to 41 previous errors
+error: aborting due to 51 previous errors
 
