Skip to content

make cast_possible_wrap work correctly for 16 bit {u,i}size #10564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 7, 2023
Merged

make cast_possible_wrap work correctly for 16 bit {u,i}size #10564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6681914
make cast_possible_wrap not lint on conversions for sizes that cannot…
asquared31415 Mar 29, 2023
7cd0ec5
add more info link
asquared31415 Jun 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 71 additions & 23 deletions clippy_lints/src/casts/cast_possible_wrap.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
}
});
}
7 changes: 4 additions & 3 deletions clippy_lints/src/casts/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/cast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
118 changes: 104 additions & 14 deletions tests/ui/cast.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -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
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -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;
| ^^^^^^^^^^
Expand All @@ -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;
| ^^^^^^^^^^
Expand All @@ -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;
| ^^^^^^^^^^^
Expand All @@ -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;
| ^^^^^^^^^^^^^
Expand All @@ -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;
| ^^^^^^^^^^^
Expand All @@ -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;
| ^^^^^^^^^^^^^^^
Expand All @@ -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;
| ^^^^^^^^^^^^^^^^
Expand All @@ -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