Skip to content

Commit 3ed0bcc

Browse files
committed
Auto merge of #6873 - Y-Nak:refactor-casts-lint, r=flip1995
Refactor casts lint Ref: #6724 Changes: 1. Separate the `casts` group from the `types` group. 2. Reorganize the lints of the `casts` group into their own modules. Notes: 1. I didn't `fix` #6874 in order to maintain this PR as small as possible. --- changelog: none
2 parents 727e5f1 + 9e631da commit 3ed0bcc

17 files changed

+1166
-997
lines changed
+87
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
use rustc_errors::Applicability;
2+
use rustc_hir::{Expr, ExprKind};
3+
use rustc_lint::LateContext;
4+
use rustc_middle::ty::{self, FloatTy, Ty};
5+
6+
use crate::utils::{in_constant, is_isize_or_usize, snippet_opt, span_lint_and_sugg};
7+
8+
use super::{utils, CAST_LOSSLESS};
9+
10+
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
11+
if !should_lint(cx, expr, cast_from, cast_to) {
12+
return;
13+
}
14+
15+
// The suggestion is to use a function call, so if the original expression
16+
// has parens on the outside, they are no longer needed.
17+
let mut applicability = Applicability::MachineApplicable;
18+
let opt = snippet_opt(cx, cast_op.span);
19+
let sugg = opt.as_ref().map_or_else(
20+
|| {
21+
applicability = Applicability::HasPlaceholders;
22+
".."
23+
},
24+
|snip| {
25+
if should_strip_parens(cast_op, snip) {
26+
&snip[1..snip.len() - 1]
27+
} else {
28+
snip.as_str()
29+
}
30+
},
31+
);
32+
33+
span_lint_and_sugg(
34+
cx,
35+
CAST_LOSSLESS,
36+
expr.span,
37+
&format!(
38+
"casting `{}` to `{}` may become silently lossy if you later change the type",
39+
cast_from, cast_to
40+
),
41+
"try",
42+
format!("{}::from({})", cast_to, sugg),
43+
applicability,
44+
);
45+
}
46+
47+
fn should_lint(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) -> bool {
48+
// Do not suggest using From in consts/statics until it is valid to do so (see #2267).
49+
if in_constant(cx, expr.hir_id) {
50+
return false;
51+
}
52+
53+
match (cast_from.is_integral(), cast_to.is_integral()) {
54+
(true, true) => {
55+
let cast_signed_to_unsigned = cast_from.is_signed() && !cast_to.is_signed();
56+
let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
57+
let to_nbits = utils::int_ty_to_nbits(cast_to, cx.tcx);
58+
!is_isize_or_usize(cast_from)
59+
&& !is_isize_or_usize(cast_to)
60+
&& from_nbits < to_nbits
61+
&& !cast_signed_to_unsigned
62+
},
63+
64+
(true, false) => {
65+
let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
66+
let to_nbits = if let ty::Float(FloatTy::F32) = cast_to.kind() {
67+
32
68+
} else {
69+
64
70+
};
71+
from_nbits < to_nbits
72+
},
73+
74+
(_, _) => {
75+
matches!(cast_from.kind(), ty::Float(FloatTy::F32)) && matches!(cast_to.kind(), ty::Float(FloatTy::F64))
76+
},
77+
}
78+
}
79+
80+
fn should_strip_parens(cast_expr: &Expr<'_>, snip: &str) -> bool {
81+
if let ExprKind::Binary(_, _, _) = cast_expr.kind {
82+
if snip.starts_with('(') && snip.ends_with(')') {
83+
return true;
84+
}
85+
}
86+
false
87+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
use rustc_hir::Expr;
2+
use rustc_lint::LateContext;
3+
use rustc_middle::ty::{self, FloatTy, Ty};
4+
5+
use crate::utils::{is_isize_or_usize, span_lint};
6+
7+
use super::{utils, CAST_POSSIBLE_TRUNCATION};
8+
9+
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
10+
let msg = match (cast_from.is_integral(), cast_to.is_integral()) {
11+
(true, true) => {
12+
let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
13+
let to_nbits = utils::int_ty_to_nbits(cast_to, cx.tcx);
14+
15+
let (should_lint, suffix) = match (is_isize_or_usize(cast_from), is_isize_or_usize(cast_to)) {
16+
(true, true) | (false, false) => (to_nbits < from_nbits, ""),
17+
(true, false) => (
18+
to_nbits <= 32,
19+
if to_nbits == 32 {
20+
" on targets with 64-bit wide pointers"
21+
} else {
22+
""
23+
},
24+
),
25+
(false, true) => (from_nbits == 64, " on targets with 32-bit wide pointers"),
26+
};
27+
28+
if !should_lint {
29+
return;
30+
}
31+
32+
format!(
33+
"casting `{}` to `{}` may truncate the value{}",
34+
cast_from, cast_to, suffix,
35+
)
36+
},
37+
38+
(false, true) => {
39+
format!("casting `{}` to `{}` may truncate the value", cast_from, cast_to)
40+
},
41+
42+
(_, _) => {
43+
if matches!(cast_from.kind(), &ty::Float(FloatTy::F64))
44+
&& matches!(cast_to.kind(), &ty::Float(FloatTy::F32))
45+
{
46+
"casting `f64` to `f32` may truncate the value".to_string()
47+
} else {
48+
return;
49+
}
50+
},
51+
};
52+
53+
span_lint(cx, CAST_POSSIBLE_TRUNCATION, expr.span, &msg);
54+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
use rustc_hir::Expr;
2+
use rustc_lint::LateContext;
3+
use rustc_middle::ty::Ty;
4+
5+
use crate::utils::{is_isize_or_usize, span_lint};
6+
7+
use super::{utils, CAST_POSSIBLE_WRAP};
8+
9+
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
10+
if !(cast_from.is_integral() && cast_to.is_integral()) {
11+
return;
12+
}
13+
14+
let arch_64_suffix = " on targets with 64-bit wide pointers";
15+
let arch_32_suffix = " on targets with 32-bit wide pointers";
16+
let cast_unsigned_to_signed = !cast_from.is_signed() && cast_to.is_signed();
17+
let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
18+
let to_nbits = utils::int_ty_to_nbits(cast_to, cx.tcx);
19+
20+
let (should_lint, suffix) = match (is_isize_or_usize(cast_from), is_isize_or_usize(cast_to)) {
21+
(true, true) | (false, false) => (to_nbits == from_nbits && cast_unsigned_to_signed, ""),
22+
(true, false) => (to_nbits <= 32 && cast_unsigned_to_signed, arch_32_suffix),
23+
(false, true) => (
24+
cast_unsigned_to_signed,
25+
if from_nbits == 64 {
26+
arch_64_suffix
27+
} else {
28+
arch_32_suffix
29+
},
30+
),
31+
};
32+
33+
if should_lint {
34+
span_lint(
35+
cx,
36+
CAST_POSSIBLE_WRAP,
37+
expr.span,
38+
&format!(
39+
"casting `{}` to `{}` may wrap around the value{}",
40+
cast_from, cast_to, suffix,
41+
),
42+
);
43+
}
44+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
use rustc_hir::Expr;
2+
use rustc_lint::LateContext;
3+
use rustc_middle::ty::{self, FloatTy, Ty};
4+
5+
use crate::utils::{is_isize_or_usize, span_lint};
6+
7+
use super::{utils, CAST_PRECISION_LOSS};
8+
9+
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
10+
if !cast_from.is_integral() || cast_to.is_integral() {
11+
return;
12+
}
13+
14+
let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
15+
let to_nbits = if let ty::Float(FloatTy::F32) = cast_to.kind() {
16+
32
17+
} else {
18+
64
19+
};
20+
21+
if !(is_isize_or_usize(cast_from) || from_nbits >= to_nbits) {
22+
return;
23+
}
24+
25+
let cast_to_f64 = to_nbits == 64;
26+
let mantissa_nbits = if cast_to_f64 { 52 } else { 23 };
27+
let arch_dependent = is_isize_or_usize(cast_from) && cast_to_f64;
28+
let arch_dependent_str = "on targets with 64-bit wide pointers ";
29+
let from_nbits_str = if arch_dependent {
30+
"64".to_owned()
31+
} else if is_isize_or_usize(cast_from) {
32+
"32 or 64".to_owned()
33+
} else {
34+
utils::int_ty_to_nbits(cast_from, cx.tcx).to_string()
35+
};
36+
37+
span_lint(
38+
cx,
39+
CAST_PRECISION_LOSS,
40+
expr.span,
41+
&format!(
42+
"casting `{0}` to `{1}` causes a loss of precision {2}(`{0}` is {3} bits wide, \
43+
but `{1}`'s mantissa is only {4} bits wide)",
44+
cast_from,
45+
if cast_to_f64 { "f64" } else { "f32" },
46+
if arch_dependent { arch_dependent_str } else { "" },
47+
from_nbits_str,
48+
mantissa_nbits
49+
),
50+
);
51+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
use rustc_hir::{Expr, ExprKind, GenericArg};
2+
use rustc_lint::LateContext;
3+
use rustc_middle::ty::{self, Ty};
4+
use rustc_span::symbol::sym;
5+
use rustc_target::abi::LayoutOf;
6+
7+
use if_chain::if_chain;
8+
9+
use crate::utils::{is_hir_ty_cfg_dependant, span_lint};
10+
11+
use super::CAST_PTR_ALIGNMENT;
12+
13+
pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
14+
if let ExprKind::Cast(ref cast_expr, cast_to) = expr.kind {
15+
if is_hir_ty_cfg_dependant(cx, cast_to) {
16+
return;
17+
}
18+
let (cast_from, cast_to) = (
19+
cx.typeck_results().expr_ty(cast_expr),
20+
cx.typeck_results().expr_ty(expr),
21+
);
22+
lint_cast_ptr_alignment(cx, expr, cast_from, cast_to);
23+
} else if let ExprKind::MethodCall(method_path, _, args, _) = expr.kind {
24+
if_chain! {
25+
if method_path.ident.name == sym!(cast);
26+
if let Some(generic_args) = method_path.args;
27+
if let [GenericArg::Type(cast_to)] = generic_args.args;
28+
// There probably is no obvious reason to do this, just to be consistent with `as` cases.
29+
if !is_hir_ty_cfg_dependant(cx, cast_to);
30+
then {
31+
let (cast_from, cast_to) =
32+
(cx.typeck_results().expr_ty(&args[0]), cx.typeck_results().expr_ty(expr));
33+
lint_cast_ptr_alignment(cx, expr, cast_from, cast_to);
34+
}
35+
}
36+
}
37+
}
38+
39+
fn lint_cast_ptr_alignment<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, cast_from: Ty<'tcx>, cast_to: Ty<'tcx>) {
40+
if_chain! {
41+
if let ty::RawPtr(from_ptr_ty) = &cast_from.kind();
42+
if let ty::RawPtr(to_ptr_ty) = &cast_to.kind();
43+
if let Ok(from_layout) = cx.layout_of(from_ptr_ty.ty);
44+
if let Ok(to_layout) = cx.layout_of(to_ptr_ty.ty);
45+
if from_layout.align.abi < to_layout.align.abi;
46+
// with c_void, we inherently need to trust the user
47+
if !is_c_void(cx, from_ptr_ty.ty);
48+
// when casting from a ZST, we don't know enough to properly lint
49+
if !from_layout.is_zst();
50+
then {
51+
span_lint(
52+
cx,
53+
CAST_PTR_ALIGNMENT,
54+
expr.span,
55+
&format!(
56+
"casting from `{}` to a more-strictly-aligned pointer (`{}`) ({} < {} bytes)",
57+
cast_from,
58+
cast_to,
59+
from_layout.align.abi.bytes(),
60+
to_layout.align.abi.bytes(),
61+
),
62+
);
63+
}
64+
}
65+
}
66+
67+
/// Check if the given type is either `core::ffi::c_void` or
68+
/// one of the platform specific `libc::<platform>::c_void` of libc.
69+
fn is_c_void(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
70+
if let ty::Adt(adt, _) = ty.kind() {
71+
let names = cx.get_def_path(adt.did);
72+
73+
if names.is_empty() {
74+
return false;
75+
}
76+
if names[0] == sym::libc || names[0] == sym::core && *names.last().unwrap() == sym!(c_void) {
77+
return true;
78+
}
79+
}
80+
false
81+
}
+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
use rustc_hir::{Expr, ExprKind, MutTy, Mutability, TyKind, UnOp};
2+
use rustc_lint::LateContext;
3+
use rustc_middle::ty;
4+
5+
use if_chain::if_chain;
6+
7+
use crate::utils::span_lint;
8+
9+
use super::CAST_REF_TO_MUT;
10+
11+
pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
12+
if_chain! {
13+
if let ExprKind::Unary(UnOp::Deref, e) = &expr.kind;
14+
if let ExprKind::Cast(e, t) = &e.kind;
15+
if let TyKind::Ptr(MutTy { mutbl: Mutability::Mut, .. }) = t.kind;
16+
if let ExprKind::Cast(e, t) = &e.kind;
17+
if let TyKind::Ptr(MutTy { mutbl: Mutability::Not, .. }) = t.kind;
18+
if let ty::Ref(..) = cx.typeck_results().node_type(e.hir_id).kind();
19+
then {
20+
span_lint(
21+
cx,
22+
CAST_REF_TO_MUT,
23+
expr.span,
24+
"casting `&T` to `&mut T` may cause undefined behavior, consider instead using an `UnsafeCell`",
25+
);
26+
}
27+
}
28+
}

0 commit comments

Comments
 (0)