Skip to content

cast_lossless: lint when converting usize, isize, char and float as well #14470

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.

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions clippy_lints/src/booleans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ impl SuggestContext<'_, '_, '_> {
self.output.push(')');
},
Term(n) => {
let terminal = self.terminals[n as usize];
let terminal = self.terminals[usize::from(n)];
if let Some(str) = simplify_not(self.cx, self.msrv, terminal) {
self.output.push_str(&str);
} else {
Expand Down Expand Up @@ -387,7 +387,7 @@ impl SuggestContext<'_, '_, '_> {
},
&Term(n) => {
self.output.push_str(
&self.terminals[n as usize]
&self.terminals[usize::from(n)]
.span
.source_callsite()
.get_source_text(self.cx)?,
Expand Down Expand Up @@ -528,7 +528,7 @@ fn terminal_stats(b: &Bool) -> Stats {
recurse(inner, stats);
}
},
&Term(n) => stats.terminals[n as usize] += 1,
&Term(n) => stats.terminals[usize::from(n)] += 1,
}
}
use quine_mc_cluskey::Bool::{And, False, Not, Or, Term, True};
Expand Down
49 changes: 23 additions & 26 deletions clippy_lints/src/casts/cast_lossless.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@ use clippy_utils::is_in_const_context;
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::source::SpanRangeExt;
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::is_isize_or_usize;
use rustc_errors::Applicability;
use rustc_hir::{Expr, QPath, TyKind};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, FloatTy, Ty};
use rustc_middle::ty::{self, FloatTy, IntTy, Ty, UintTy};
use rustc_span::hygiene;

use super::{CAST_LOSSLESS, utils};
Expand All @@ -21,7 +20,12 @@ pub(super) fn check(
cast_to_hir: &rustc_hir::Ty<'_>,
msrv: Msrv,
) {
if !should_lint(cx, cast_from, cast_to, msrv) {
if !should_lint(cx, cast_from, cast_to, msrv)
// Checking the expression, cast_from_expr and cast_to_hir share the same context.
|| [cast_from_expr.span, cast_to_hir.span]
.iter()
.any(|&span| !expr.span.eq_ctxt(span))
Comment on lines +24 to +27
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we can make an exception here to still lint if !expr.span.from_expansion()? That is, still lint

foo!() as u32

(So basically the new false negative in the tests).

I can see how linting expressions in macro definitions could be a hazard since the macro might be invoked multiple times with different types or expressions which are only sometimes compatible, but I don't think there is a harm if the primary cast expression is in the root context (regardless of what or where cast_from/cast_to comes from), is there?

{
return;
}

Expand All @@ -47,7 +51,8 @@ pub(super) fn check(
);
},
// Don't suggest `A<_>::B::From(x)` or `macro!()::from(x)`
kind if matches!(kind, TyKind::Path(QPath::Resolved(_, path)) if path.segments.iter().any(|s| s.args.is_some()))
kind if matches!(kind, TyKind::Path(QPath::Resolved(_, path))
if path.segments.iter().any(|s| s.args.is_some()))
|| !cast_to_hir.span.eq_ctxt(expr.span) =>
{
diag.span_suggestion_verbose(
Expand Down Expand Up @@ -76,29 +81,21 @@ fn should_lint(cx: &LateContext<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>, msrv: M
return false;
}

match (cast_from.is_integral(), cast_to.is_integral()) {
(true, true) => {
let cast_signed_to_unsigned = cast_from.is_signed() && !cast_to.is_signed();
let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
let to_nbits = utils::int_ty_to_nbits(cast_to, cx.tcx);
!is_isize_or_usize(cast_from)
&& !is_isize_or_usize(cast_to)
&& from_nbits < to_nbits
&& !cast_signed_to_unsigned
match (cast_from.kind(), cast_to.kind()) {
(ty::Bool, ty::Uint(_) | ty::Int(_)) => msrv.meets(cx, msrvs::FROM_BOOL),
(ty::Uint(_), ty::Uint(UintTy::Usize)) | (ty::Uint(UintTy::U8) | ty::Int(_), ty::Int(IntTy::Isize)) => {
utils::int_ty_to_nbits(cast_from, cx.tcx) <= 16
},

(true, false) => {
let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
let to_nbits = if let ty::Float(FloatTy::F32) = cast_to.kind() {
32
} else {
64
};
!is_isize_or_usize(cast_from) && from_nbits < to_nbits
},
(false, true) if matches!(cast_from.kind(), ty::Bool) && msrv.meets(cx, msrvs::FROM_BOOL) => true,
(_, _) => {
matches!(cast_from.kind(), ty::Float(FloatTy::F32)) && matches!(cast_to.kind(), ty::Float(FloatTy::F64))
// No `f16` to `f32`: https://github.com/rust-lang/rust/issues/123831
(ty::Uint(UintTy::Usize) | ty::Int(IntTy::Isize), _)
| (_, ty::Uint(UintTy::Usize) | ty::Int(IntTy::Isize))
| (ty::Float(FloatTy::F16), ty::Float(FloatTy::F32)) => false,
(ty::Uint(_) | ty::Int(_), ty::Int(_)) | (ty::Uint(_), ty::Uint(_)) => {
utils::int_ty_to_nbits(cast_from, cx.tcx) < utils::int_ty_to_nbits(cast_to, cx.tcx)
},
(ty::Uint(_) | ty::Int(_), ty::Float(fl)) => utils::int_ty_to_nbits(cast_from, cx.tcx) < fl.bit_width(),
(ty::Char, ty::Uint(_)) => utils::int_ty_to_nbits(cast_to, cx.tcx) >= 32,
(ty::Float(fl_from), ty::Float(fl_to)) => fl_from.bit_width() < fl_to.bit_width(),
_ => false,
}
}
2 changes: 1 addition & 1 deletion clippy_lints/src/literal_string_with_formatting_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl<'tcx> LateLintPass<'tcx> for LiteralStringWithFormattingArg {
LitKind::Str(symbol, style) => {
let add = match style {
StrStyle::Cooked => 1,
StrStyle::Raw(nb) => nb as usize + 2,
StrStyle::Raw(nb) => usize::from(nb) + 2,
};
(add, symbol)
},
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/regex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ fn lint_syntax_error(cx: &LateContext<'_>, error: &regex_syntax::Error, unescape

if let Some((primary, auxiliary, kind)) = parts
&& let Some(literal_snippet) = base.get_source_text(cx)
&& let Some(inner) = literal_snippet.get(offset as usize..)
&& let Some(inner) = literal_snippet.get(usize::from(offset)..)
// Only convert to native rustc spans if the parsed regex matches the
// source snippet exactly, to ensure the span offsets are correct
&& inner.get(..unescaped.len()) == Some(unescaped)
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/unicode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl LateLintPass<'_> for Unicode {
fn escape<T: Iterator<Item = char>>(s: T) -> String {
let mut result = String::new();
for c in s {
if c as u32 > 0x7F {
if u32::from(c) > 0x7F {
for d in c.escape_unicode() {
result.push(d);
}
Expand Down Expand Up @@ -119,7 +119,7 @@ fn check_str(cx: &LateContext<'_>, span: Span, id: HirId) {
});
}

if string.chars().any(|c| c as u32 > 0x7F) {
if string.chars().any(|c| u32::from(c) > 0x7F) {
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
span_lint_and_then(
cx,
Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,7 @@ pub fn str_literal_to_char_literal(
{
let snip = snippet_with_applicability(sess, expr.span, string, applicability);
let ch = if let StrStyle::Raw(nhash) = style {
let nhash = nhash as usize;
let nhash = usize::from(nhash);
// for raw string: r##"a"##
&snip[(nhash + 2)..(snip.len() - 1 - nhash)]
} else {
Expand Down
20 changes: 20 additions & 0 deletions tests/ui/cast_lossless_char.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#![warn(clippy::cast_lossless)]
#![allow(clippy::char_lit_as_u8)]

type I32 = i32;
type U32 = u32;

fn main() {
let _ = u32::from('a');
//~^ cast_lossless
let _ = 'a' as i32;
let _ = u64::from('a');
//~^ cast_lossless
let _ = 'a' as i64;
let _ = U32::from('a');
//~^ cast_lossless
let _ = 'a' as I32;

let _ = 'a' as u8;
let _ = 'a' as i8;
}
20 changes: 20 additions & 0 deletions tests/ui/cast_lossless_char.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#![warn(clippy::cast_lossless)]
#![allow(clippy::char_lit_as_u8)]

type I32 = i32;
type U32 = u32;

fn main() {
let _ = 'a' as u32;
//~^ cast_lossless
let _ = 'a' as i32;
let _ = 'a' as u64;
//~^ cast_lossless
let _ = 'a' as i64;
let _ = 'a' as U32;
//~^ cast_lossless
let _ = 'a' as I32;

let _ = 'a' as u8;
let _ = 'a' as i8;
}
43 changes: 43 additions & 0 deletions tests/ui/cast_lossless_char.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
error: casts from `char` to `u32` can be expressed infallibly using `From`
--> tests/ui/cast_lossless_char.rs:8:13
|
LL | let _ = 'a' as u32;
| ^^^^^^^^^^
|
= help: an `as` cast can become silently lossy if the types change in the future
= note: `-D clippy::cast-lossless` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::cast_lossless)]`
help: use `u32::from` instead
|
LL - let _ = 'a' as u32;
LL + let _ = u32::from('a');
|

error: casts from `char` to `u64` can be expressed infallibly using `From`
--> tests/ui/cast_lossless_char.rs:11:13
|
LL | let _ = 'a' as u64;
| ^^^^^^^^^^
|
= help: an `as` cast can become silently lossy if the types change in the future
help: use `u64::from` instead
|
LL - let _ = 'a' as u64;
LL + let _ = u64::from('a');
|

error: casts from `char` to `u32` can be expressed infallibly using `From`
--> tests/ui/cast_lossless_char.rs:14:13
|
LL | let _ = 'a' as U32;
| ^^^^^^^^^^
|
= help: an `as` cast can become silently lossy if the types change in the future
help: use `U32::from` instead
|
LL - let _ = 'a' as U32;
LL + let _ = U32::from('a');
|

error: aborting due to 3 previous errors

19 changes: 15 additions & 4 deletions tests/ui/cast_lossless_integer.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,7 @@ fn issue11458() {
};
}
let x = 10_u128;
let _ = i32::from(sign_cast!(x, u8, i8));
//~^ cast_lossless
let _ = sign_cast!(x, u8, i8) as i32;

let _ = i32::from(sign_cast!(x, u8, i8) + 1);
//~^ cast_lossless
Expand All @@ -171,8 +170,20 @@ fn ty_from_macro() {
};
}

let _ = <ty!()>::from(0u8);
//~^ cast_lossless
let _ = 0u8 as ty!();
}

const IN_CONST: u64 = 0u8 as u64;

fn macros() {
macro_rules! mac {
(to_i32 $x:expr) => { $x as u32 };
(zero_as $t:ty) => { 0u8 as $t };
($x:expr => $t:ty) => { $x as $t };
($x:expr, $k:ident, $t:ty) => { $x $k $t };
}
let _ = mac!(to_i32 0u16);
let _ = mac!(zero_as u32);
let _ = mac!(0u8 => u32);
let _ = mac!(0u8, as, u32);
}
15 changes: 13 additions & 2 deletions tests/ui/cast_lossless_integer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ fn issue11458() {
}
let x = 10_u128;
let _ = sign_cast!(x, u8, i8) as i32;
//~^ cast_lossless

let _ = (sign_cast!(x, u8, i8) + 1) as i32;
//~^ cast_lossless
Expand All @@ -172,7 +171,19 @@ fn ty_from_macro() {
}

let _ = 0u8 as ty!();
//~^ cast_lossless
}

const IN_CONST: u64 = 0u8 as u64;

fn macros() {
macro_rules! mac {
(to_i32 $x:expr) => { $x as u32 };
(zero_as $t:ty) => { 0u8 as $t };
($x:expr => $t:ty) => { $x as $t };
($x:expr, $k:ident, $t:ty) => { $x $k $t };
}
let _ = mac!(to_i32 0u16);
let _ = mac!(zero_as u32);
let _ = mac!(0u8 => u32);
let _ = mac!(0u8, as, u32);
}
32 changes: 3 additions & 29 deletions tests/ui/cast_lossless_integer.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -469,20 +469,7 @@ LL + let _: u32 = (1i8 as u16).into();
|

error: casts from `i8` to `i32` can be expressed infallibly using `From`
--> tests/ui/cast_lossless_integer.rs:149:13
|
LL | let _ = sign_cast!(x, u8, i8) as i32;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: an `as` cast can become silently lossy if the types change in the future
help: use `i32::from` instead
|
LL - let _ = sign_cast!(x, u8, i8) as i32;
LL + let _ = i32::from(sign_cast!(x, u8, i8));
|

error: casts from `i8` to `i32` can be expressed infallibly using `From`
--> tests/ui/cast_lossless_integer.rs:152:13
--> tests/ui/cast_lossless_integer.rs:151:13
|
LL | let _ = (sign_cast!(x, u8, i8) + 1) as i32;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -495,7 +482,7 @@ LL + let _ = i32::from(sign_cast!(x, u8, i8) + 1);
|

error: casts from `u8` to `u32` can be expressed infallibly using `From`
--> tests/ui/cast_lossless_integer.rs:159:13
--> tests/ui/cast_lossless_integer.rs:158:13
|
LL | 1u8 as u32
| ^^^^^^^^^^
Expand All @@ -511,18 +498,5 @@ LL - 1u8 as u32
LL + u32::from(1u8)
|

error: casts from `u8` to `u32` can be expressed infallibly using `From`
--> tests/ui/cast_lossless_integer.rs:174:13
|
LL | let _ = 0u8 as ty!();
| ^^^^^^^^^^^^
|
= help: an `as` cast can become silently lossy if the types change in the future
help: use `<ty!()>::from` instead
|
LL - let _ = 0u8 as ty!();
LL + let _ = <ty!()>::from(0u8);
|

error: aborting due to 40 previous errors
error: aborting due to 38 previous errors