From f098007caaebe660b06ee7c43cd30006807a2a8c Mon Sep 17 00:00:00 2001 From: Yoshitomo Nakanishi Date: Tue, 9 Mar 2021 10:57:25 +0900 Subject: [PATCH 01/15] Separate lints of casts group from types group --- clippy_lints/src/casts/mod.rs | 973 ++++++++++++++++++++++++++++++++++ clippy_lints/src/lib.rs | 67 +-- clippy_lints/src/types/mod.rs | 969 +-------------------------------- 3 files changed, 1015 insertions(+), 994 deletions(-) create mode 100644 clippy_lints/src/casts/mod.rs diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs new file mode 100644 index 000000000000..d8245a3b88f7 --- /dev/null +++ b/clippy_lints/src/casts/mod.rs @@ -0,0 +1,973 @@ +use std::borrow::Cow; + +use if_chain::if_chain; +use rustc_ast::{LitFloatType, LitIntType, LitKind}; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, GenericArg, Lit, MutTy, Mutability, TyKind, UnOp}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_middle::ty::{self, FloatTy, InferTy, IntTy, Ty, TyCtxt, TypeAndMut, UintTy}; +use rustc_semver::RustcVersion; +use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; +use rustc_span::symbol::sym; +use rustc_target::abi::LayoutOf; + +use crate::consts::{constant, Constant}; +use crate::utils::sugg::Sugg; +use crate::utils::{ + in_constant, is_hir_ty_cfg_dependant, meets_msrv, method_chain_args, numeric_literal::NumericLiteral, sext, + snippet_opt, snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then, +}; + +declare_clippy_lint! { + /// **What it does:** Checks for casts from any numerical to a float type where + /// the receiving type cannot store all values from the original type without + /// rounding errors. This possible rounding is to be expected, so this lint is + /// `Allow` by default. + /// + /// Basically, this warns on casting any integer with 32 or more bits to `f32` + /// or any 64-bit integer to `f64`. + /// + /// **Why is this bad?** It's not bad at all. But in some applications it can be + /// helpful to know where precision loss can take place. This lint can help find + /// those places in the code. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust + /// let x = u64::MAX; + /// x as f64; + /// ``` + pub CAST_PRECISION_LOSS, + pedantic, + "casts that cause loss of precision, e.g., `x as f32` where `x: u64`" +} + +declare_clippy_lint! { + /// **What it does:** Checks for casts from a signed to an unsigned numerical + /// type. In this case, negative values wrap around to large positive values, + /// which can be quite surprising in practice. However, as the cast works as + /// defined, this lint is `Allow` by default. + /// + /// **Why is this bad?** Possibly surprising results. You can activate this lint + /// as a one-time check to see where numerical wrapping can arise. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust + /// let y: i8 = -1; + /// y as u128; // will return 18446744073709551615 + /// ``` + pub CAST_SIGN_LOSS, + pedantic, + "casts from signed types to unsigned types, e.g., `x as u32` where `x: i32`" +} + +declare_clippy_lint! { + /// **What it does:** Checks for casts between numerical types that may + /// truncate large values. This is expected behavior, so the cast is `Allow` by + /// default. + /// + /// **Why is this bad?** In some problem domains, it is good practice to avoid + /// truncation. This lint can be activated to help assess where additional + /// checks could be beneficial. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust + /// fn as_u8(x: u64) -> u8 { + /// x as u8 + /// } + /// ``` + pub CAST_POSSIBLE_TRUNCATION, + pedantic, + "casts that may cause truncation of the value, e.g., `x as u8` where `x: u32`, or `x as i32` where `x: f32`" +} + +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 + /// for the target signed type. However, the cast works as defined, so this lint + /// is `Allow` by default. + /// + /// **Why is this bad?** While such a cast is not bad in itself, the results can + /// be surprising when this is not the intended behavior, as demonstrated by the + /// example below. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust + /// u32::MAX as i32; // will yield a value of `-1` + /// ``` + pub CAST_POSSIBLE_WRAP, + pedantic, + "casts that may cause wrapping around the value, e.g., `x as i32` where `x: u32` and `x > i32::MAX`" +} + +declare_clippy_lint! { + /// **What it does:** Checks for casts between numerical types that may + /// be replaced by safe conversion functions. + /// + /// **Why is this bad?** Rust's `as` keyword will perform many kinds of + /// conversions, including silently lossy conversions. Conversion functions such + /// as `i32::from` will only perform lossless conversions. Using the conversion + /// functions prevents conversions from turning into silent lossy conversions if + /// the types of the input expressions ever change, and make it easier for + /// people reading the code to know that the conversion is lossless. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust + /// fn as_u64(x: u8) -> u64 { + /// x as u64 + /// } + /// ``` + /// + /// Using `::from` would look like this: + /// + /// ```rust + /// fn as_u64(x: u8) -> u64 { + /// u64::from(x) + /// } + /// ``` + pub CAST_LOSSLESS, + pedantic, + "casts using `as` that are known to be lossless, e.g., `x as u64` where `x: u8`" +} + +declare_clippy_lint! { + /// **What it does:** Checks for casts to the same type, casts of int literals to integer types + /// and casts of float literals to float types. + /// + /// **Why is this bad?** It's just unnecessary. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust + /// let _ = 2i32 as i32; + /// let _ = 0.5 as f32; + /// ``` + /// + /// Better: + /// + /// ```rust + /// let _ = 2_i32; + /// let _ = 0.5_f32; + /// ``` + pub UNNECESSARY_CAST, + complexity, + "cast to the same type, e.g., `x as i32` where `x: i32`" +} + +declare_clippy_lint! { + /// **What it does:** Checks for casts, using `as` or `pointer::cast`, + /// from a less-strictly-aligned pointer to a more-strictly-aligned pointer + /// + /// **Why is this bad?** Dereferencing the resulting pointer may be undefined + /// behavior. + /// + /// **Known problems:** Using `std::ptr::read_unaligned` and `std::ptr::write_unaligned` or similar + /// on the resulting pointer is fine. Is over-zealous: Casts with manual alignment checks or casts like + /// u64-> u8 -> u16 can be fine. Miri is able to do a more in-depth analysis. + /// + /// **Example:** + /// ```rust + /// let _ = (&1u8 as *const u8) as *const u16; + /// let _ = (&mut 1u8 as *mut u8) as *mut u16; + /// + /// (&1u8 as *const u8).cast::(); + /// (&mut 1u8 as *mut u8).cast::(); + /// ``` + pub CAST_PTR_ALIGNMENT, + pedantic, + "cast from a pointer to a more-strictly-aligned pointer" +} + +declare_clippy_lint! { + /// **What it does:** Checks for casts of function pointers to something other than usize + /// + /// **Why is this bad?** + /// Casting a function pointer to anything other than usize/isize is not portable across + /// architectures, because you end up losing bits if the target type is too small or end up with a + /// bunch of extra bits that waste space and add more instructions to the final binary than + /// strictly necessary for the problem + /// + /// Casting to isize also doesn't make sense since there are no signed addresses. + /// + /// **Example** + /// + /// ```rust + /// // Bad + /// fn fun() -> i32 { 1 } + /// let a = fun as i64; + /// + /// // Good + /// fn fun2() -> i32 { 1 } + /// let a = fun2 as usize; + /// ``` + pub FN_TO_NUMERIC_CAST, + style, + "casting a function pointer to a numeric type other than usize" +} + +declare_clippy_lint! { + /// **What it does:** Checks for casts of a function pointer to a numeric type not wide enough to + /// store address. + /// + /// **Why is this bad?** + /// Such a cast discards some bits of the function's address. If this is intended, it would be more + /// clearly expressed by casting to usize first, then casting the usize to the intended type (with + /// a comment) to perform the truncation. + /// + /// **Example** + /// + /// ```rust + /// // Bad + /// fn fn1() -> i16 { + /// 1 + /// }; + /// let _ = fn1 as i32; + /// + /// // Better: Cast to usize first, then comment with the reason for the truncation + /// fn fn2() -> i16 { + /// 1 + /// }; + /// let fn_ptr = fn2 as usize; + /// let fn_ptr_truncated = fn_ptr as i32; + /// ``` + pub FN_TO_NUMERIC_CAST_WITH_TRUNCATION, + style, + "casting a function pointer to a numeric type not wide enough to store the address" +} + +/// Returns the size in bits of an integral type. +/// Will return 0 if the type is not an int or uint variant +fn int_ty_to_nbits(typ: Ty<'_>, tcx: TyCtxt<'_>) -> u64 { + match typ.kind() { + ty::Int(i) => match i { + IntTy::Isize => tcx.data_layout.pointer_size.bits(), + IntTy::I8 => 8, + IntTy::I16 => 16, + IntTy::I32 => 32, + IntTy::I64 => 64, + IntTy::I128 => 128, + }, + ty::Uint(i) => match i { + UintTy::Usize => tcx.data_layout.pointer_size.bits(), + UintTy::U8 => 8, + UintTy::U16 => 16, + UintTy::U32 => 32, + UintTy::U64 => 64, + UintTy::U128 => 128, + }, + _ => 0, + } +} + +fn span_precision_loss_lint(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to_f64: bool) { + let mantissa_nbits = if cast_to_f64 { 52 } else { 23 }; + let arch_dependent = is_isize_or_usize(cast_from) && cast_to_f64; + let arch_dependent_str = "on targets with 64-bit wide pointers "; + let from_nbits_str = if arch_dependent { + "64".to_owned() + } else if is_isize_or_usize(cast_from) { + "32 or 64".to_owned() + } else { + int_ty_to_nbits(cast_from, cx.tcx).to_string() + }; + span_lint( + cx, + CAST_PRECISION_LOSS, + expr.span, + &format!( + "casting `{0}` to `{1}` causes a loss of precision {2}(`{0}` is {3} bits wide, \ + but `{1}`'s mantissa is only {4} bits wide)", + cast_from, + if cast_to_f64 { "f64" } else { "f32" }, + if arch_dependent { arch_dependent_str } else { "" }, + from_nbits_str, + mantissa_nbits + ), + ); +} + +fn should_strip_parens(op: &Expr<'_>, snip: &str) -> bool { + if let ExprKind::Binary(_, _, _) = op.kind { + if snip.starts_with('(') && snip.ends_with(')') { + return true; + } + } + false +} + +fn span_lossless_lint(cx: &LateContext<'_>, expr: &Expr<'_>, op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { + // Do not suggest using From in consts/statics until it is valid to do so (see #2267). + if in_constant(cx, expr.hir_id) { + return; + } + // The suggestion is to use a function call, so if the original expression + // has parens on the outside, they are no longer needed. + let mut applicability = Applicability::MachineApplicable; + let opt = snippet_opt(cx, op.span); + let sugg = opt.as_ref().map_or_else( + || { + applicability = Applicability::HasPlaceholders; + ".." + }, + |snip| { + if should_strip_parens(op, snip) { + &snip[1..snip.len() - 1] + } else { + snip.as_str() + } + }, + ); + + span_lint_and_sugg( + cx, + CAST_LOSSLESS, + expr.span, + &format!( + "casting `{}` to `{}` may become silently lossy if you later change the type", + cast_from, cast_to + ), + "try", + format!("{}::from({})", cast_to, sugg), + applicability, + ); +} + +enum ArchSuffix { + _32, + _64, + None, +} + +fn check_loss_of_sign(cx: &LateContext<'_>, expr: &Expr<'_>, op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { + if !cast_from.is_signed() || cast_to.is_signed() { + return; + } + + // don't lint for positive constants + let const_val = constant(cx, &cx.typeck_results(), op); + if_chain! { + if let Some((Constant::Int(n), _)) = const_val; + if let ty::Int(ity) = *cast_from.kind(); + if sext(cx.tcx, n, ity) >= 0; + then { + return + } + } + + // don't lint for the result of methods that always return non-negative values + if let ExprKind::MethodCall(ref path, _, _, _) = op.kind { + let mut method_name = path.ident.name.as_str(); + let allowed_methods = ["abs", "checked_abs", "rem_euclid", "checked_rem_euclid"]; + + if_chain! { + if method_name == "unwrap"; + if let Some(arglist) = method_chain_args(op, &["unwrap"]); + if let ExprKind::MethodCall(ref inner_path, _, _, _) = &arglist[0][0].kind; + then { + method_name = inner_path.ident.name.as_str(); + } + } + + if allowed_methods.iter().any(|&name| method_name == name) { + return; + } + } + + span_lint( + cx, + CAST_SIGN_LOSS, + expr.span, + &format!( + "casting `{}` to `{}` may lose the sign of the value", + cast_from, cast_to + ), + ); +} + +fn check_truncation_and_wrapping(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { + 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(); + let from_nbits = int_ty_to_nbits(cast_from, cx.tcx); + let to_nbits = int_ty_to_nbits(cast_to, cx.tcx); + let (span_truncation, suffix_truncation, span_wrap, suffix_wrap) = + match (is_isize_or_usize(cast_from), is_isize_or_usize(cast_to)) { + (true, true) | (false, false) => ( + to_nbits < from_nbits, + ArchSuffix::None, + to_nbits == from_nbits && cast_unsigned_to_signed, + ArchSuffix::None, + ), + (true, false) => ( + to_nbits <= 32, + if to_nbits == 32 { + ArchSuffix::_64 + } else { + ArchSuffix::None + }, + to_nbits <= 32 && cast_unsigned_to_signed, + ArchSuffix::_32, + ), + (false, true) => ( + from_nbits == 64, + ArchSuffix::_32, + cast_unsigned_to_signed, + if from_nbits == 64 { + ArchSuffix::_64 + } else { + ArchSuffix::_32 + }, + ), + }; + if span_truncation { + span_lint( + cx, + CAST_POSSIBLE_TRUNCATION, + expr.span, + &format!( + "casting `{}` to `{}` may truncate the value{}", + cast_from, + cast_to, + match suffix_truncation { + ArchSuffix::_32 => arch_32_suffix, + ArchSuffix::_64 => arch_64_suffix, + ArchSuffix::None => "", + } + ), + ); + } + if span_wrap { + span_lint( + cx, + CAST_POSSIBLE_WRAP, + expr.span, + &format!( + "casting `{}` to `{}` may wrap around the value{}", + cast_from, + cast_to, + match suffix_wrap { + ArchSuffix::_32 => arch_32_suffix, + ArchSuffix::_64 => arch_64_suffix, + ArchSuffix::None => "", + } + ), + ); + } +} + +fn check_lossless(cx: &LateContext<'_>, expr: &Expr<'_>, op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { + let cast_signed_to_unsigned = cast_from.is_signed() && !cast_to.is_signed(); + let from_nbits = int_ty_to_nbits(cast_from, cx.tcx); + let to_nbits = int_ty_to_nbits(cast_to, cx.tcx); + if !is_isize_or_usize(cast_from) && !is_isize_or_usize(cast_to) && from_nbits < to_nbits && !cast_signed_to_unsigned + { + span_lossless_lint(cx, expr, op, cast_from, cast_to); + } +} + +declare_lint_pass!(Casts => [ + CAST_PRECISION_LOSS, + CAST_SIGN_LOSS, + CAST_POSSIBLE_TRUNCATION, + CAST_POSSIBLE_WRAP, + CAST_LOSSLESS, + UNNECESSARY_CAST, + CAST_PTR_ALIGNMENT, + FN_TO_NUMERIC_CAST, + FN_TO_NUMERIC_CAST_WITH_TRUNCATION, +]); + +/// Check if the given type is either `core::ffi::c_void` or +/// one of the platform specific `libc::::c_void` of libc. +fn is_c_void(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { + if let ty::Adt(adt, _) = ty.kind() { + let names = cx.get_def_path(adt.did); + + if names.is_empty() { + return false; + } + if names[0] == sym::libc || names[0] == sym::core && *names.last().unwrap() == sym!(c_void) { + return true; + } + } + false +} + +/// Returns the mantissa bits wide of a fp type. +/// Will return 0 if the type is not a fp +fn fp_ty_mantissa_nbits(typ: Ty<'_>) -> u32 { + match typ.kind() { + ty::Float(FloatTy::F32) => 23, + ty::Float(FloatTy::F64) | ty::Infer(InferTy::FloatVar(_)) => 52, + _ => 0, + } +} + +impl<'tcx> LateLintPass<'tcx> for Casts { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if expr.span.from_expansion() { + return; + } + if let ExprKind::Cast(ref ex, cast_to) = expr.kind { + if is_hir_ty_cfg_dependant(cx, cast_to) { + return; + } + let (cast_from, cast_to) = (cx.typeck_results().expr_ty(ex), cx.typeck_results().expr_ty(expr)); + lint_fn_to_numeric_cast(cx, expr, ex, cast_from, cast_to); + if let Some(lit) = get_numeric_literal(ex) { + let literal_str = snippet_opt(cx, ex.span).unwrap_or_default(); + + if_chain! { + if let LitKind::Int(n, _) = lit.node; + if let Some(src) = snippet_opt(cx, lit.span); + if cast_to.is_floating_point(); + if let Some(num_lit) = NumericLiteral::from_lit_kind(&src, &lit.node); + let from_nbits = 128 - n.leading_zeros(); + let to_nbits = fp_ty_mantissa_nbits(cast_to); + if from_nbits != 0 && to_nbits != 0 && from_nbits <= to_nbits && num_lit.is_decimal(); + then { + let literal_str = if is_unary_neg(ex) { format!("-{}", num_lit.integer) } else { num_lit.integer.into() }; + show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); + return; + } + } + + match lit.node { + LitKind::Int(_, LitIntType::Unsuffixed) if cast_to.is_integral() => { + show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); + }, + LitKind::Float(_, LitFloatType::Unsuffixed) if cast_to.is_floating_point() => { + show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); + }, + LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed) => {}, + _ => { + if cast_from.kind() == cast_to.kind() && !in_external_macro(cx.sess(), expr.span) { + span_lint( + cx, + UNNECESSARY_CAST, + expr.span, + &format!( + "casting to the same type is unnecessary (`{}` -> `{}`)", + cast_from, cast_to + ), + ); + } + }, + } + } + if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) { + lint_numeric_casts(cx, expr, ex, cast_from, cast_to); + } + + lint_cast_ptr_alignment(cx, expr, cast_from, cast_to); + } else if let ExprKind::MethodCall(method_path, _, args, _) = expr.kind { + if_chain! { + if method_path.ident.name == sym!(cast); + if let Some(generic_args) = method_path.args; + if let [GenericArg::Type(cast_to)] = generic_args.args; + // There probably is no obvious reason to do this, just to be consistent with `as` cases. + if !is_hir_ty_cfg_dependant(cx, cast_to); + then { + let (cast_from, cast_to) = + (cx.typeck_results().expr_ty(&args[0]), cx.typeck_results().expr_ty(expr)); + lint_cast_ptr_alignment(cx, expr, cast_from, cast_to); + } + } + } + } +} + +fn is_unary_neg(expr: &Expr<'_>) -> bool { + matches!(expr.kind, ExprKind::Unary(UnOp::Neg, _)) +} + +fn get_numeric_literal<'e>(expr: &'e Expr<'e>) -> Option<&'e Lit> { + match expr.kind { + ExprKind::Lit(ref lit) => Some(lit), + ExprKind::Unary(UnOp::Neg, e) => { + if let ExprKind::Lit(ref lit) = e.kind { + Some(lit) + } else { + None + } + }, + _ => None, + } +} + +fn show_unnecessary_cast(cx: &LateContext<'_>, expr: &Expr<'_>, literal_str: &str, cast_from: Ty<'_>, cast_to: Ty<'_>) { + let literal_kind_name = if cast_from.is_integral() { "integer" } else { "float" }; + span_lint_and_sugg( + cx, + UNNECESSARY_CAST, + expr.span, + &format!("casting {} literal to `{}` is unnecessary", literal_kind_name, cast_to), + "try", + format!("{}_{}", literal_str.trim_end_matches('.'), cast_to), + Applicability::MachineApplicable, + ); +} + +fn lint_numeric_casts<'tcx>( + cx: &LateContext<'tcx>, + expr: &Expr<'tcx>, + cast_expr: &Expr<'_>, + cast_from: Ty<'tcx>, + cast_to: Ty<'tcx>, +) { + match (cast_from.is_integral(), cast_to.is_integral()) { + (true, false) => { + let from_nbits = int_ty_to_nbits(cast_from, cx.tcx); + let to_nbits = if let ty::Float(FloatTy::F32) = cast_to.kind() { + 32 + } else { + 64 + }; + if is_isize_or_usize(cast_from) || from_nbits >= to_nbits { + span_precision_loss_lint(cx, expr, cast_from, to_nbits == 64); + } + if from_nbits < to_nbits { + span_lossless_lint(cx, expr, cast_expr, cast_from, cast_to); + } + }, + (false, true) => { + span_lint( + cx, + CAST_POSSIBLE_TRUNCATION, + expr.span, + &format!("casting `{}` to `{}` may truncate the value", cast_from, cast_to), + ); + if !cast_to.is_signed() { + span_lint( + cx, + CAST_SIGN_LOSS, + expr.span, + &format!( + "casting `{}` to `{}` may lose the sign of the value", + cast_from, cast_to + ), + ); + } + }, + (true, true) => { + check_loss_of_sign(cx, expr, cast_expr, cast_from, cast_to); + check_truncation_and_wrapping(cx, expr, cast_from, cast_to); + check_lossless(cx, expr, cast_expr, cast_from, cast_to); + }, + (false, false) => { + if let (&ty::Float(FloatTy::F64), &ty::Float(FloatTy::F32)) = (&cast_from.kind(), &cast_to.kind()) { + span_lint( + cx, + CAST_POSSIBLE_TRUNCATION, + expr.span, + "casting `f64` to `f32` may truncate the value", + ); + } + if let (&ty::Float(FloatTy::F32), &ty::Float(FloatTy::F64)) = (&cast_from.kind(), &cast_to.kind()) { + span_lossless_lint(cx, expr, cast_expr, cast_from, cast_to); + } + }, + } +} + +fn lint_cast_ptr_alignment<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, cast_from: Ty<'tcx>, cast_to: Ty<'tcx>) { + if_chain! { + if let ty::RawPtr(from_ptr_ty) = &cast_from.kind(); + if let ty::RawPtr(to_ptr_ty) = &cast_to.kind(); + if let Ok(from_layout) = cx.layout_of(from_ptr_ty.ty); + if let Ok(to_layout) = cx.layout_of(to_ptr_ty.ty); + if from_layout.align.abi < to_layout.align.abi; + // with c_void, we inherently need to trust the user + if !is_c_void(cx, from_ptr_ty.ty); + // when casting from a ZST, we don't know enough to properly lint + if !from_layout.is_zst(); + then { + span_lint( + cx, + CAST_PTR_ALIGNMENT, + expr.span, + &format!( + "casting from `{}` to a more-strictly-aligned pointer (`{}`) ({} < {} bytes)", + cast_from, + cast_to, + from_layout.align.abi.bytes(), + to_layout.align.abi.bytes(), + ), + ); + } + } +} + +fn lint_fn_to_numeric_cast( + cx: &LateContext<'_>, + expr: &Expr<'_>, + cast_expr: &Expr<'_>, + cast_from: Ty<'_>, + cast_to: Ty<'_>, +) { + // We only want to check casts to `ty::Uint` or `ty::Int` + match cast_to.kind() { + ty::Uint(_) | ty::Int(..) => { /* continue on */ }, + _ => return, + } + match cast_from.kind() { + ty::FnDef(..) | ty::FnPtr(_) => { + let mut applicability = Applicability::MaybeIncorrect; + let from_snippet = snippet_with_applicability(cx, cast_expr.span, "x", &mut applicability); + + let to_nbits = int_ty_to_nbits(cast_to, cx.tcx); + if to_nbits < cx.tcx.data_layout.pointer_size.bits() { + span_lint_and_sugg( + cx, + FN_TO_NUMERIC_CAST_WITH_TRUNCATION, + expr.span, + &format!( + "casting function pointer `{}` to `{}`, which truncates the value", + from_snippet, cast_to + ), + "try", + format!("{} as usize", from_snippet), + applicability, + ); + } else if *cast_to.kind() != ty::Uint(UintTy::Usize) { + span_lint_and_sugg( + cx, + FN_TO_NUMERIC_CAST, + expr.span, + &format!("casting function pointer `{}` to `{}`", from_snippet, cast_to), + "try", + format!("{} as usize", from_snippet), + applicability, + ); + } + }, + _ => {}, + } +} + +declare_clippy_lint! { + /// **What it does:** Checks for casts of `&T` to `&mut T` anywhere in the code. + /// + /// **Why is this bad?** It’s basically guaranteed to be undefined behaviour. + /// `UnsafeCell` is the only way to obtain aliasable data that is considered + /// mutable. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust,ignore + /// fn x(r: &i32) { + /// unsafe { + /// *(r as *const _ as *mut _) += 1; + /// } + /// } + /// ``` + /// + /// Instead consider using interior mutability types. + /// + /// ```rust + /// use std::cell::UnsafeCell; + /// + /// fn x(r: &UnsafeCell) { + /// unsafe { + /// *r.get() += 1; + /// } + /// } + /// ``` + pub CAST_REF_TO_MUT, + correctness, + "a cast of reference to a mutable pointer" +} + +declare_lint_pass!(RefToMut => [CAST_REF_TO_MUT]); + +impl<'tcx> LateLintPass<'tcx> for RefToMut { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if_chain! { + if let ExprKind::Unary(UnOp::Deref, e) = &expr.kind; + if let ExprKind::Cast(e, t) = &e.kind; + if let TyKind::Ptr(MutTy { mutbl: Mutability::Mut, .. }) = t.kind; + if let ExprKind::Cast(e, t) = &e.kind; + if let TyKind::Ptr(MutTy { mutbl: Mutability::Not, .. }) = t.kind; + if let ty::Ref(..) = cx.typeck_results().node_type(e.hir_id).kind(); + then { + span_lint( + cx, + CAST_REF_TO_MUT, + expr.span, + "casting `&T` to `&mut T` may cause undefined behavior, consider instead using an `UnsafeCell`", + ); + } + } + } +} + +const PTR_AS_PTR_MSRV: RustcVersion = RustcVersion::new(1, 38, 0); + +declare_clippy_lint! { + /// **What it does:** Checks for expressions where a character literal is cast + /// to `u8` and suggests using a byte literal instead. + /// + /// **Why is this bad?** In general, casting values to smaller types is + /// error-prone and should be avoided where possible. In the particular case of + /// converting a character literal to u8, it is easy to avoid by just using a + /// byte literal instead. As an added bonus, `b'a'` is even slightly shorter + /// than `'a' as u8`. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust,ignore + /// 'x' as u8 + /// ``` + /// + /// A better version, using the byte literal: + /// + /// ```rust,ignore + /// b'x' + /// ``` + pub CHAR_LIT_AS_U8, + complexity, + "casting a character literal to `u8` truncates" +} + +declare_lint_pass!(CharLitAsU8 => [CHAR_LIT_AS_U8]); + +impl<'tcx> LateLintPass<'tcx> for CharLitAsU8 { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if_chain! { + if !expr.span.from_expansion(); + if let ExprKind::Cast(e, _) = &expr.kind; + if let ExprKind::Lit(l) = &e.kind; + if let LitKind::Char(c) = l.node; + if ty::Uint(UintTy::U8) == *cx.typeck_results().expr_ty(expr).kind(); + then { + let mut applicability = Applicability::MachineApplicable; + let snippet = snippet_with_applicability(cx, e.span, "'x'", &mut applicability); + + span_lint_and_then( + cx, + CHAR_LIT_AS_U8, + expr.span, + "casting a character literal to `u8` truncates", + |diag| { + diag.note("`char` is four bytes wide, but `u8` is a single byte"); + + if c.is_ascii() { + diag.span_suggestion( + expr.span, + "use a byte literal instead", + format!("b{}", snippet), + applicability, + ); + } + }); + } + } + } +} + +declare_clippy_lint! { + /// **What it does:** + /// Checks for `as` casts between raw pointers without changing its mutability, + /// namely `*const T` to `*const U` and `*mut T` to `*mut U`. + /// + /// **Why is this bad?** + /// Though `as` casts between raw pointers is not terrible, `pointer::cast` is safer because + /// it cannot accidentally change the pointer's mutability nor cast the pointer to other types like `usize`. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// let ptr: *const u32 = &42_u32; + /// let mut_ptr: *mut u32 = &mut 42_u32; + /// let _ = ptr as *const i32; + /// let _ = mut_ptr as *mut i32; + /// ``` + /// Use instead: + /// ```rust + /// let ptr: *const u32 = &42_u32; + /// let mut_ptr: *mut u32 = &mut 42_u32; + /// let _ = ptr.cast::(); + /// let _ = mut_ptr.cast::(); + /// ``` + pub PTR_AS_PTR, + pedantic, + "casting using `as` from and to raw pointers that doesn't change its mutability, where `pointer::cast` could take the place of `as`" +} + +pub struct PtrAsPtr { + msrv: Option, +} + +impl PtrAsPtr { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} + +impl_lint_pass!(PtrAsPtr => [PTR_AS_PTR]); + +impl<'tcx> LateLintPass<'tcx> for PtrAsPtr { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if !meets_msrv(self.msrv.as_ref(), &PTR_AS_PTR_MSRV) { + return; + } + + if expr.span.from_expansion() { + return; + } + + if_chain! { + if let ExprKind::Cast(cast_expr, cast_to_hir_ty) = expr.kind; + let (cast_from, cast_to) = (cx.typeck_results().expr_ty(cast_expr), cx.typeck_results().expr_ty(expr)); + if let ty::RawPtr(TypeAndMut { mutbl: from_mutbl, .. }) = cast_from.kind(); + if let ty::RawPtr(TypeAndMut { ty: to_pointee_ty, mutbl: to_mutbl }) = cast_to.kind(); + if matches!((from_mutbl, to_mutbl), + (Mutability::Not, Mutability::Not) | (Mutability::Mut, Mutability::Mut)); + // The `U` in `pointer::cast` have to be `Sized` + // as explained here: https://github.com/rust-lang/rust/issues/60602. + if to_pointee_ty.is_sized(cx.tcx.at(expr.span), cx.param_env); + then { + let mut applicability = Applicability::MachineApplicable; + let cast_expr_sugg = Sugg::hir_with_applicability(cx, cast_expr, "_", &mut applicability); + let turbofish = match &cast_to_hir_ty.kind { + TyKind::Infer => Cow::Borrowed(""), + TyKind::Ptr(mut_ty) if matches!(mut_ty.ty.kind, TyKind::Infer) => Cow::Borrowed(""), + _ => Cow::Owned(format!("::<{}>", to_pointee_ty)), + }; + span_lint_and_sugg( + cx, + PTR_AS_PTR, + expr.span, + "`as` casting between raw pointers without changing its mutability", + "try `pointer::cast`, a safer alternative", + format!("{}.cast{}()", cast_expr_sugg.maybe_par(), turbofish), + applicability, + ); + } + } + } + + extract_msrv_attr!(LateContext); +} + +fn is_isize_or_usize(typ: Ty<'_>) -> bool { + matches!(typ.kind(), ty::Int(IntTy::Isize) | ty::Uint(UintTy::Usize)) +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 8259fd3c320b..a46c56a01d5d 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -182,6 +182,7 @@ mod booleans; mod bytecount; mod cargo_common_metadata; mod case_sensitive_file_extension_comparisons; +mod casts; mod checked_conversions; mod cognitive_complexity; mod collapsible_if; @@ -586,6 +587,18 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &bytecount::NAIVE_BYTECOUNT, &cargo_common_metadata::CARGO_COMMON_METADATA, &case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS, + &casts::CAST_LOSSLESS, + &casts::CAST_POSSIBLE_TRUNCATION, + &casts::CAST_POSSIBLE_WRAP, + &casts::CAST_PRECISION_LOSS, + &casts::CAST_PTR_ALIGNMENT, + &casts::CAST_REF_TO_MUT, + &casts::CAST_SIGN_LOSS, + &casts::CHAR_LIT_AS_U8, + &casts::FN_TO_NUMERIC_CAST, + &casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION, + &casts::PTR_AS_PTR, + &casts::UNNECESSARY_CAST, &checked_conversions::CHECKED_CONVERSIONS, &cognitive_complexity::COGNITIVE_COMPLEXITY, &collapsible_if::COLLAPSIBLE_ELSE_IF, @@ -943,28 +956,16 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &types::ABSURD_EXTREME_COMPARISONS, &types::BORROWED_BOX, &types::BOX_VEC, - &types::CAST_LOSSLESS, - &types::CAST_POSSIBLE_TRUNCATION, - &types::CAST_POSSIBLE_WRAP, - &types::CAST_PRECISION_LOSS, - &types::CAST_PTR_ALIGNMENT, - &types::CAST_REF_TO_MUT, - &types::CAST_SIGN_LOSS, - &types::CHAR_LIT_AS_U8, - &types::FN_TO_NUMERIC_CAST, - &types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION, &types::IMPLICIT_HASHER, &types::INVALID_UPCAST_COMPARISONS, &types::LET_UNIT_VALUE, &types::LINKEDLIST, &types::OPTION_OPTION, - &types::PTR_AS_PTR, &types::RC_BUFFER, &types::REDUNDANT_ALLOCATION, &types::TYPE_COMPLEXITY, &types::UNIT_ARG, &types::UNIT_CMP, - &types::UNNECESSARY_CAST, &types::VEC_BOX, &undropped_manually_drops::UNDROPPED_MANUALLY_DROPS, &unicode::INVISIBLE_CHARACTERS, @@ -1086,7 +1087,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box main_recursion::MainRecursion::default()); store.register_late_pass(|| box lifetimes::Lifetimes); store.register_late_pass(|| box entry::HashMapPass); - store.register_late_pass(|| box types::Casts); + store.register_late_pass(|| box casts::Casts); let type_complexity_threshold = conf.type_complexity_threshold; store.register_late_pass(move || box types::TypeComplexity::new(type_complexity_threshold)); store.register_late_pass(|| box minmax::MinMaxPass); @@ -1107,7 +1108,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box panic_unimplemented::PanicUnimplemented); store.register_late_pass(|| box strings::StringLitAsBytes); store.register_late_pass(|| box derive::Derive); - store.register_late_pass(|| box types::CharLitAsU8); + store.register_late_pass(|| box casts::CharLitAsU8); store.register_late_pass(|| box get_last_with_len::GetLastWithLen); store.register_late_pass(|| box drop_forget_ref::DropForgetRef); store.register_late_pass(|| box empty_enum::EmptyEnum); @@ -1175,7 +1176,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box slow_vector_initialization::SlowVectorInit); store.register_late_pass(|| box unnecessary_sort_by::UnnecessarySortBy); store.register_late_pass(|| box unnecessary_wraps::UnnecessaryWraps); - store.register_late_pass(|| box types::RefToMut); + store.register_late_pass(|| box casts::RefToMut); store.register_late_pass(|| box assertions_on_constants::AssertionsOnConstants); store.register_late_pass(|| box transmuting_null::TransmutingNull); store.register_late_pass(|| box path_buf_push_overwrite::PathBufPushOverwrite); @@ -1277,7 +1278,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box strings::StringToString); store.register_late_pass(|| box zero_sized_map_values::ZeroSizedMapValues); store.register_late_pass(|| box vec_init_then_push::VecInitThenPush::default()); - store.register_late_pass(move || box types::PtrAsPtr::new(msrv)); + store.register_late_pass(move || box casts::PtrAsPtr::new(msrv)); store.register_late_pass(|| box case_sensitive_file_extension_comparisons::CaseSensitiveFileExtensionComparisons); store.register_late_pass(|| box redundant_slicing::RedundantSlicing); store.register_late_pass(|| box from_str_radix_10::FromStrRadix10); @@ -1345,6 +1346,13 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&bit_mask::VERBOSE_BIT_MASK), LintId::of(&bytecount::NAIVE_BYTECOUNT), LintId::of(&case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS), + LintId::of(&casts::CAST_LOSSLESS), + LintId::of(&casts::CAST_POSSIBLE_TRUNCATION), + LintId::of(&casts::CAST_POSSIBLE_WRAP), + LintId::of(&casts::CAST_PRECISION_LOSS), + LintId::of(&casts::CAST_PTR_ALIGNMENT), + LintId::of(&casts::CAST_SIGN_LOSS), + LintId::of(&casts::PTR_AS_PTR), LintId::of(&checked_conversions::CHECKED_CONVERSIONS), LintId::of(&copies::SAME_FUNCTIONS_IN_IF_CONDITION), LintId::of(©_iterator::COPY_ITERATOR), @@ -1404,18 +1412,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&strings::STRING_ADD_ASSIGN), LintId::of(&trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS), LintId::of(&trait_bounds::TYPE_REPETITION_IN_BOUNDS), - LintId::of(&types::CAST_LOSSLESS), - LintId::of(&types::CAST_POSSIBLE_TRUNCATION), - LintId::of(&types::CAST_POSSIBLE_WRAP), - LintId::of(&types::CAST_PRECISION_LOSS), - LintId::of(&types::CAST_PTR_ALIGNMENT), - LintId::of(&types::CAST_SIGN_LOSS), LintId::of(&types::IMPLICIT_HASHER), LintId::of(&types::INVALID_UPCAST_COMPARISONS), LintId::of(&types::LET_UNIT_VALUE), LintId::of(&types::LINKEDLIST), LintId::of(&types::OPTION_OPTION), - LintId::of(&types::PTR_AS_PTR), LintId::of(&unicode::NON_ASCII_LITERAL), LintId::of(&unicode::UNICODE_NOT_NFC), LintId::of(&unnecessary_wraps::UNNECESSARY_WRAPS), @@ -1459,6 +1460,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS), LintId::of(&booleans::LOGIC_BUG), LintId::of(&booleans::NONMINIMAL_BOOL), + LintId::of(&casts::CAST_REF_TO_MUT), + LintId::of(&casts::CHAR_LIT_AS_U8), + LintId::of(&casts::FN_TO_NUMERIC_CAST), + LintId::of(&casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION), + LintId::of(&casts::UNNECESSARY_CAST), LintId::of(&collapsible_if::COLLAPSIBLE_ELSE_IF), LintId::of(&collapsible_if::COLLAPSIBLE_IF), LintId::of(&collapsible_match::COLLAPSIBLE_MATCH), @@ -1699,15 +1705,10 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::ABSURD_EXTREME_COMPARISONS), LintId::of(&types::BORROWED_BOX), LintId::of(&types::BOX_VEC), - LintId::of(&types::CAST_REF_TO_MUT), - LintId::of(&types::CHAR_LIT_AS_U8), - LintId::of(&types::FN_TO_NUMERIC_CAST), - LintId::of(&types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION), LintId::of(&types::REDUNDANT_ALLOCATION), LintId::of(&types::TYPE_COMPLEXITY), LintId::of(&types::UNIT_ARG), LintId::of(&types::UNIT_CMP), - LintId::of(&types::UNNECESSARY_CAST), LintId::of(&types::VEC_BOX), LintId::of(&undropped_manually_drops::UNDROPPED_MANUALLY_DROPS), LintId::of(&unicode::INVISIBLE_CHARACTERS), @@ -1740,6 +1741,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&attrs::BLANKET_CLIPPY_RESTRICTION_LINTS), LintId::of(&blacklisted_name::BLACKLISTED_NAME), LintId::of(&blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS), + LintId::of(&casts::FN_TO_NUMERIC_CAST), + LintId::of(&casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION), LintId::of(&collapsible_if::COLLAPSIBLE_ELSE_IF), LintId::of(&collapsible_if::COLLAPSIBLE_IF), LintId::of(&collapsible_match::COLLAPSIBLE_MATCH), @@ -1836,8 +1839,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&tabs_in_doc_comments::TABS_IN_DOC_COMMENTS), LintId::of(&to_digit_is_some::TO_DIGIT_IS_SOME), LintId::of(&try_err::TRY_ERR), - LintId::of(&types::FN_TO_NUMERIC_CAST), - LintId::of(&types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION), LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME), LintId::of(&unused_unit::UNUSED_UNIT), LintId::of(&upper_case_acronyms::UPPER_CASE_ACRONYMS), @@ -1853,6 +1854,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&assign_ops::MISREFACTORED_ASSIGN_OP), LintId::of(&attrs::DEPRECATED_CFG_ATTR), LintId::of(&booleans::NONMINIMAL_BOOL), + LintId::of(&casts::CHAR_LIT_AS_U8), + LintId::of(&casts::UNNECESSARY_CAST), LintId::of(&double_comparison::DOUBLE_COMPARISONS), LintId::of(&double_parens::DOUBLE_PARENS), LintId::of(&duration_subsec::DURATION_SUBSEC), @@ -1929,10 +1932,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&transmute::TRANSMUTE_PTR_TO_PTR), LintId::of(&transmute::TRANSMUTE_PTR_TO_REF), LintId::of(&types::BORROWED_BOX), - LintId::of(&types::CHAR_LIT_AS_U8), LintId::of(&types::TYPE_COMPLEXITY), LintId::of(&types::UNIT_ARG), - LintId::of(&types::UNNECESSARY_CAST), LintId::of(&types::VEC_BOX), LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY), LintId::of(&unwrap::UNNECESSARY_UNWRAP), @@ -1950,6 +1951,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&bit_mask::BAD_BIT_MASK), LintId::of(&bit_mask::INEFFECTIVE_BIT_MASK), LintId::of(&booleans::LOGIC_BUG), + LintId::of(&casts::CAST_REF_TO_MUT), LintId::of(&copies::IFS_SAME_COND), LintId::of(&copies::IF_SAME_THEN_ELSE), LintId::of(&derive::DERIVE_HASH_XOR_EQ), @@ -2002,7 +2004,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&transmute::WRONG_TRANSMUTE), LintId::of(&transmuting_null::TRANSMUTING_NULL), LintId::of(&types::ABSURD_EXTREME_COMPARISONS), - LintId::of(&types::CAST_REF_TO_MUT), LintId::of(&types::UNIT_CMP), LintId::of(&undropped_manually_drops::UNDROPPED_MANUALLY_DROPS), LintId::of(&unicode::INVISIBLE_CHARACTERS), diff --git a/clippy_lints/src/types/mod.rs b/clippy_lints/src/types/mod.rs index 25cc40917c30..3e7aa53d5bc9 100644 --- a/clippy_lints/src/types/mod.rs +++ b/clippy_lints/src/types/mod.rs @@ -14,20 +14,18 @@ use std::cmp::Ordering; use std::collections::BTreeMap; use if_chain::if_chain; -use rustc_ast::{LitFloatType, LitIntType, LitKind}; use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_hir as hir; use rustc_hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisitorMap, Visitor}; use rustc_hir::{ BinOpKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, FnSig, GenericArg, GenericParamKind, HirId, ImplItem, - ImplItemKind, Item, ItemKind, Lit, Local, MatchSource, MutTy, Mutability, Node, QPath, Stmt, StmtKind, TraitFn, - TraitItem, TraitItemKind, TyKind, UnOp, + ImplItemKind, Item, ItemKind, Local, MatchSource, MutTy, Node, QPath, Stmt, StmtKind, TraitFn, TraitItem, + TraitItemKind, TyKind, }; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::hir::map::Map; use rustc_middle::lint::in_external_macro; -use rustc_middle::ty::{self, FloatTy, InferTy, IntTy, Ty, TyCtxt, TyS, TypeAndMut, TypeckResults, UintTy}; -use rustc_semver::RustcVersion; +use rustc_middle::ty::{self, IntTy, Ty, TyS, TypeckResults, UintTy}; use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; use rustc_span::hygiene::{ExpnKind, MacroKind}; use rustc_span::source_map::Span; @@ -38,12 +36,10 @@ use rustc_typeck::hir_ty_to_ty; use crate::consts::{constant, Constant}; use crate::utils::paths; -use crate::utils::sugg::Sugg; use crate::utils::{ - clip, comparisons, differing_macro_contexts, higher, in_constant, indent_of, int_bits, is_hir_ty_cfg_dependant, - is_type_diagnostic_item, match_path, meets_msrv, method_chain_args, multispan_sugg, - numeric_literal::NumericLiteral, reindent_multiline, sext, snippet, snippet_opt, snippet_with_applicability, - snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, unsext, + clip, comparisons, differing_macro_contexts, higher, indent_of, int_bits, is_type_diagnostic_item, match_path, + multispan_sugg, reindent_multiline, sext, snippet, snippet_opt, snippet_with_macro_callsite, span_lint, + span_lint_and_help, span_lint_and_then, unsext, }; declare_clippy_lint! { @@ -778,749 +774,6 @@ fn is_unit_literal(expr: &Expr<'_>) -> bool { matches!(expr.kind, ExprKind::Tup(ref slice) if slice.is_empty()) } -declare_clippy_lint! { - /// **What it does:** Checks for casts from any numerical to a float type where - /// the receiving type cannot store all values from the original type without - /// rounding errors. This possible rounding is to be expected, so this lint is - /// `Allow` by default. - /// - /// Basically, this warns on casting any integer with 32 or more bits to `f32` - /// or any 64-bit integer to `f64`. - /// - /// **Why is this bad?** It's not bad at all. But in some applications it can be - /// helpful to know where precision loss can take place. This lint can help find - /// those places in the code. - /// - /// **Known problems:** None. - /// - /// **Example:** - /// ```rust - /// let x = u64::MAX; - /// x as f64; - /// ``` - pub CAST_PRECISION_LOSS, - pedantic, - "casts that cause loss of precision, e.g., `x as f32` where `x: u64`" -} - -declare_clippy_lint! { - /// **What it does:** Checks for casts from a signed to an unsigned numerical - /// type. In this case, negative values wrap around to large positive values, - /// which can be quite surprising in practice. However, as the cast works as - /// defined, this lint is `Allow` by default. - /// - /// **Why is this bad?** Possibly surprising results. You can activate this lint - /// as a one-time check to see where numerical wrapping can arise. - /// - /// **Known problems:** None. - /// - /// **Example:** - /// ```rust - /// let y: i8 = -1; - /// y as u128; // will return 18446744073709551615 - /// ``` - pub CAST_SIGN_LOSS, - pedantic, - "casts from signed types to unsigned types, e.g., `x as u32` where `x: i32`" -} - -declare_clippy_lint! { - /// **What it does:** Checks for casts between numerical types that may - /// truncate large values. This is expected behavior, so the cast is `Allow` by - /// default. - /// - /// **Why is this bad?** In some problem domains, it is good practice to avoid - /// truncation. This lint can be activated to help assess where additional - /// checks could be beneficial. - /// - /// **Known problems:** None. - /// - /// **Example:** - /// ```rust - /// fn as_u8(x: u64) -> u8 { - /// x as u8 - /// } - /// ``` - pub CAST_POSSIBLE_TRUNCATION, - pedantic, - "casts that may cause truncation of the value, e.g., `x as u8` where `x: u32`, or `x as i32` where `x: f32`" -} - -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 - /// for the target signed type. However, the cast works as defined, so this lint - /// is `Allow` by default. - /// - /// **Why is this bad?** While such a cast is not bad in itself, the results can - /// be surprising when this is not the intended behavior, as demonstrated by the - /// example below. - /// - /// **Known problems:** None. - /// - /// **Example:** - /// ```rust - /// u32::MAX as i32; // will yield a value of `-1` - /// ``` - pub CAST_POSSIBLE_WRAP, - pedantic, - "casts that may cause wrapping around the value, e.g., `x as i32` where `x: u32` and `x > i32::MAX`" -} - -declare_clippy_lint! { - /// **What it does:** Checks for casts between numerical types that may - /// be replaced by safe conversion functions. - /// - /// **Why is this bad?** Rust's `as` keyword will perform many kinds of - /// conversions, including silently lossy conversions. Conversion functions such - /// as `i32::from` will only perform lossless conversions. Using the conversion - /// functions prevents conversions from turning into silent lossy conversions if - /// the types of the input expressions ever change, and make it easier for - /// people reading the code to know that the conversion is lossless. - /// - /// **Known problems:** None. - /// - /// **Example:** - /// ```rust - /// fn as_u64(x: u8) -> u64 { - /// x as u64 - /// } - /// ``` - /// - /// Using `::from` would look like this: - /// - /// ```rust - /// fn as_u64(x: u8) -> u64 { - /// u64::from(x) - /// } - /// ``` - pub CAST_LOSSLESS, - pedantic, - "casts using `as` that are known to be lossless, e.g., `x as u64` where `x: u8`" -} - -declare_clippy_lint! { - /// **What it does:** Checks for casts to the same type, casts of int literals to integer types - /// and casts of float literals to float types. - /// - /// **Why is this bad?** It's just unnecessary. - /// - /// **Known problems:** None. - /// - /// **Example:** - /// ```rust - /// let _ = 2i32 as i32; - /// let _ = 0.5 as f32; - /// ``` - /// - /// Better: - /// - /// ```rust - /// let _ = 2_i32; - /// let _ = 0.5_f32; - /// ``` - pub UNNECESSARY_CAST, - complexity, - "cast to the same type, e.g., `x as i32` where `x: i32`" -} - -declare_clippy_lint! { - /// **What it does:** Checks for casts, using `as` or `pointer::cast`, - /// from a less-strictly-aligned pointer to a more-strictly-aligned pointer - /// - /// **Why is this bad?** Dereferencing the resulting pointer may be undefined - /// behavior. - /// - /// **Known problems:** Using `std::ptr::read_unaligned` and `std::ptr::write_unaligned` or similar - /// on the resulting pointer is fine. Is over-zealous: Casts with manual alignment checks or casts like - /// u64-> u8 -> u16 can be fine. Miri is able to do a more in-depth analysis. - /// - /// **Example:** - /// ```rust - /// let _ = (&1u8 as *const u8) as *const u16; - /// let _ = (&mut 1u8 as *mut u8) as *mut u16; - /// - /// (&1u8 as *const u8).cast::(); - /// (&mut 1u8 as *mut u8).cast::(); - /// ``` - pub CAST_PTR_ALIGNMENT, - pedantic, - "cast from a pointer to a more-strictly-aligned pointer" -} - -declare_clippy_lint! { - /// **What it does:** Checks for casts of function pointers to something other than usize - /// - /// **Why is this bad?** - /// Casting a function pointer to anything other than usize/isize is not portable across - /// architectures, because you end up losing bits if the target type is too small or end up with a - /// bunch of extra bits that waste space and add more instructions to the final binary than - /// strictly necessary for the problem - /// - /// Casting to isize also doesn't make sense since there are no signed addresses. - /// - /// **Example** - /// - /// ```rust - /// // Bad - /// fn fun() -> i32 { 1 } - /// let a = fun as i64; - /// - /// // Good - /// fn fun2() -> i32 { 1 } - /// let a = fun2 as usize; - /// ``` - pub FN_TO_NUMERIC_CAST, - style, - "casting a function pointer to a numeric type other than usize" -} - -declare_clippy_lint! { - /// **What it does:** Checks for casts of a function pointer to a numeric type not wide enough to - /// store address. - /// - /// **Why is this bad?** - /// Such a cast discards some bits of the function's address. If this is intended, it would be more - /// clearly expressed by casting to usize first, then casting the usize to the intended type (with - /// a comment) to perform the truncation. - /// - /// **Example** - /// - /// ```rust - /// // Bad - /// fn fn1() -> i16 { - /// 1 - /// }; - /// let _ = fn1 as i32; - /// - /// // Better: Cast to usize first, then comment with the reason for the truncation - /// fn fn2() -> i16 { - /// 1 - /// }; - /// let fn_ptr = fn2 as usize; - /// let fn_ptr_truncated = fn_ptr as i32; - /// ``` - pub FN_TO_NUMERIC_CAST_WITH_TRUNCATION, - style, - "casting a function pointer to a numeric type not wide enough to store the address" -} - -/// Returns the size in bits of an integral type. -/// Will return 0 if the type is not an int or uint variant -fn int_ty_to_nbits(typ: Ty<'_>, tcx: TyCtxt<'_>) -> u64 { - match typ.kind() { - ty::Int(i) => match i { - IntTy::Isize => tcx.data_layout.pointer_size.bits(), - IntTy::I8 => 8, - IntTy::I16 => 16, - IntTy::I32 => 32, - IntTy::I64 => 64, - IntTy::I128 => 128, - }, - ty::Uint(i) => match i { - UintTy::Usize => tcx.data_layout.pointer_size.bits(), - UintTy::U8 => 8, - UintTy::U16 => 16, - UintTy::U32 => 32, - UintTy::U64 => 64, - UintTy::U128 => 128, - }, - _ => 0, - } -} - -fn is_isize_or_usize(typ: Ty<'_>) -> bool { - matches!(typ.kind(), ty::Int(IntTy::Isize) | ty::Uint(UintTy::Usize)) -} - -fn span_precision_loss_lint(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to_f64: bool) { - let mantissa_nbits = if cast_to_f64 { 52 } else { 23 }; - let arch_dependent = is_isize_or_usize(cast_from) && cast_to_f64; - let arch_dependent_str = "on targets with 64-bit wide pointers "; - let from_nbits_str = if arch_dependent { - "64".to_owned() - } else if is_isize_or_usize(cast_from) { - "32 or 64".to_owned() - } else { - int_ty_to_nbits(cast_from, cx.tcx).to_string() - }; - span_lint( - cx, - CAST_PRECISION_LOSS, - expr.span, - &format!( - "casting `{0}` to `{1}` causes a loss of precision {2}(`{0}` is {3} bits wide, \ - but `{1}`'s mantissa is only {4} bits wide)", - cast_from, - if cast_to_f64 { "f64" } else { "f32" }, - if arch_dependent { arch_dependent_str } else { "" }, - from_nbits_str, - mantissa_nbits - ), - ); -} - -fn should_strip_parens(op: &Expr<'_>, snip: &str) -> bool { - if let ExprKind::Binary(_, _, _) = op.kind { - if snip.starts_with('(') && snip.ends_with(')') { - return true; - } - } - false -} - -fn span_lossless_lint(cx: &LateContext<'_>, expr: &Expr<'_>, op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { - // Do not suggest using From in consts/statics until it is valid to do so (see #2267). - if in_constant(cx, expr.hir_id) { - return; - } - // The suggestion is to use a function call, so if the original expression - // has parens on the outside, they are no longer needed. - let mut applicability = Applicability::MachineApplicable; - let opt = snippet_opt(cx, op.span); - let sugg = opt.as_ref().map_or_else( - || { - applicability = Applicability::HasPlaceholders; - ".." - }, - |snip| { - if should_strip_parens(op, snip) { - &snip[1..snip.len() - 1] - } else { - snip.as_str() - } - }, - ); - - span_lint_and_sugg( - cx, - CAST_LOSSLESS, - expr.span, - &format!( - "casting `{}` to `{}` may become silently lossy if you later change the type", - cast_from, cast_to - ), - "try", - format!("{}::from({})", cast_to, sugg), - applicability, - ); -} - -enum ArchSuffix { - _32, - _64, - None, -} - -fn check_loss_of_sign(cx: &LateContext<'_>, expr: &Expr<'_>, op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { - if !cast_from.is_signed() || cast_to.is_signed() { - return; - } - - // don't lint for positive constants - let const_val = constant(cx, &cx.typeck_results(), op); - if_chain! { - if let Some((Constant::Int(n), _)) = const_val; - if let ty::Int(ity) = *cast_from.kind(); - if sext(cx.tcx, n, ity) >= 0; - then { - return - } - } - - // don't lint for the result of methods that always return non-negative values - if let ExprKind::MethodCall(ref path, _, _, _) = op.kind { - let mut method_name = path.ident.name.as_str(); - let allowed_methods = ["abs", "checked_abs", "rem_euclid", "checked_rem_euclid"]; - - if_chain! { - if method_name == "unwrap"; - if let Some(arglist) = method_chain_args(op, &["unwrap"]); - if let ExprKind::MethodCall(ref inner_path, _, _, _) = &arglist[0][0].kind; - then { - method_name = inner_path.ident.name.as_str(); - } - } - - if allowed_methods.iter().any(|&name| method_name == name) { - return; - } - } - - span_lint( - cx, - CAST_SIGN_LOSS, - expr.span, - &format!( - "casting `{}` to `{}` may lose the sign of the value", - cast_from, cast_to - ), - ); -} - -fn check_truncation_and_wrapping(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { - 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(); - let from_nbits = int_ty_to_nbits(cast_from, cx.tcx); - let to_nbits = int_ty_to_nbits(cast_to, cx.tcx); - let (span_truncation, suffix_truncation, span_wrap, suffix_wrap) = - match (is_isize_or_usize(cast_from), is_isize_or_usize(cast_to)) { - (true, true) | (false, false) => ( - to_nbits < from_nbits, - ArchSuffix::None, - to_nbits == from_nbits && cast_unsigned_to_signed, - ArchSuffix::None, - ), - (true, false) => ( - to_nbits <= 32, - if to_nbits == 32 { - ArchSuffix::_64 - } else { - ArchSuffix::None - }, - to_nbits <= 32 && cast_unsigned_to_signed, - ArchSuffix::_32, - ), - (false, true) => ( - from_nbits == 64, - ArchSuffix::_32, - cast_unsigned_to_signed, - if from_nbits == 64 { - ArchSuffix::_64 - } else { - ArchSuffix::_32 - }, - ), - }; - if span_truncation { - span_lint( - cx, - CAST_POSSIBLE_TRUNCATION, - expr.span, - &format!( - "casting `{}` to `{}` may truncate the value{}", - cast_from, - cast_to, - match suffix_truncation { - ArchSuffix::_32 => arch_32_suffix, - ArchSuffix::_64 => arch_64_suffix, - ArchSuffix::None => "", - } - ), - ); - } - if span_wrap { - span_lint( - cx, - CAST_POSSIBLE_WRAP, - expr.span, - &format!( - "casting `{}` to `{}` may wrap around the value{}", - cast_from, - cast_to, - match suffix_wrap { - ArchSuffix::_32 => arch_32_suffix, - ArchSuffix::_64 => arch_64_suffix, - ArchSuffix::None => "", - } - ), - ); - } -} - -fn check_lossless(cx: &LateContext<'_>, expr: &Expr<'_>, op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { - let cast_signed_to_unsigned = cast_from.is_signed() && !cast_to.is_signed(); - let from_nbits = int_ty_to_nbits(cast_from, cx.tcx); - let to_nbits = int_ty_to_nbits(cast_to, cx.tcx); - if !is_isize_or_usize(cast_from) && !is_isize_or_usize(cast_to) && from_nbits < to_nbits && !cast_signed_to_unsigned - { - span_lossless_lint(cx, expr, op, cast_from, cast_to); - } -} - -declare_lint_pass!(Casts => [ - CAST_PRECISION_LOSS, - CAST_SIGN_LOSS, - CAST_POSSIBLE_TRUNCATION, - CAST_POSSIBLE_WRAP, - CAST_LOSSLESS, - UNNECESSARY_CAST, - CAST_PTR_ALIGNMENT, - FN_TO_NUMERIC_CAST, - FN_TO_NUMERIC_CAST_WITH_TRUNCATION, -]); - -// Check if the given type is either `core::ffi::c_void` or -// one of the platform specific `libc::::c_void` of libc. -fn is_c_void(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { - if let ty::Adt(adt, _) = ty.kind() { - let names = cx.get_def_path(adt.did); - - if names.is_empty() { - return false; - } - if names[0] == sym::libc || names[0] == sym::core && *names.last().unwrap() == sym!(c_void) { - return true; - } - } - false -} - -/// Returns the mantissa bits wide of a fp type. -/// Will return 0 if the type is not a fp -fn fp_ty_mantissa_nbits(typ: Ty<'_>) -> u32 { - match typ.kind() { - ty::Float(FloatTy::F32) => 23, - ty::Float(FloatTy::F64) | ty::Infer(InferTy::FloatVar(_)) => 52, - _ => 0, - } -} - -impl<'tcx> LateLintPass<'tcx> for Casts { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if expr.span.from_expansion() { - return; - } - if let ExprKind::Cast(ref ex, cast_to) = expr.kind { - if is_hir_ty_cfg_dependant(cx, cast_to) { - return; - } - let (cast_from, cast_to) = (cx.typeck_results().expr_ty(ex), cx.typeck_results().expr_ty(expr)); - lint_fn_to_numeric_cast(cx, expr, ex, cast_from, cast_to); - if let Some(lit) = get_numeric_literal(ex) { - let literal_str = snippet_opt(cx, ex.span).unwrap_or_default(); - - if_chain! { - if let LitKind::Int(n, _) = lit.node; - if let Some(src) = snippet_opt(cx, lit.span); - if cast_to.is_floating_point(); - if let Some(num_lit) = NumericLiteral::from_lit_kind(&src, &lit.node); - let from_nbits = 128 - n.leading_zeros(); - let to_nbits = fp_ty_mantissa_nbits(cast_to); - if from_nbits != 0 && to_nbits != 0 && from_nbits <= to_nbits && num_lit.is_decimal(); - then { - let literal_str = if is_unary_neg(ex) { format!("-{}", num_lit.integer) } else { num_lit.integer.into() }; - show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); - return; - } - } - - match lit.node { - LitKind::Int(_, LitIntType::Unsuffixed) if cast_to.is_integral() => { - show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); - }, - LitKind::Float(_, LitFloatType::Unsuffixed) if cast_to.is_floating_point() => { - show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); - }, - LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed) => {}, - _ => { - if cast_from.kind() == cast_to.kind() && !in_external_macro(cx.sess(), expr.span) { - span_lint( - cx, - UNNECESSARY_CAST, - expr.span, - &format!( - "casting to the same type is unnecessary (`{}` -> `{}`)", - cast_from, cast_to - ), - ); - } - }, - } - } - if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) { - lint_numeric_casts(cx, expr, ex, cast_from, cast_to); - } - - lint_cast_ptr_alignment(cx, expr, cast_from, cast_to); - } else if let ExprKind::MethodCall(method_path, _, args, _) = expr.kind { - if_chain! { - if method_path.ident.name == sym!(cast); - if let Some(generic_args) = method_path.args; - if let [GenericArg::Type(cast_to)] = generic_args.args; - // There probably is no obvious reason to do this, just to be consistent with `as` cases. - if !is_hir_ty_cfg_dependant(cx, cast_to); - then { - let (cast_from, cast_to) = - (cx.typeck_results().expr_ty(&args[0]), cx.typeck_results().expr_ty(expr)); - lint_cast_ptr_alignment(cx, expr, cast_from, cast_to); - } - } - } - } -} - -fn is_unary_neg(expr: &Expr<'_>) -> bool { - matches!(expr.kind, ExprKind::Unary(UnOp::Neg, _)) -} - -fn get_numeric_literal<'e>(expr: &'e Expr<'e>) -> Option<&'e Lit> { - match expr.kind { - ExprKind::Lit(ref lit) => Some(lit), - ExprKind::Unary(UnOp::Neg, e) => { - if let ExprKind::Lit(ref lit) = e.kind { - Some(lit) - } else { - None - } - }, - _ => None, - } -} - -fn show_unnecessary_cast(cx: &LateContext<'_>, expr: &Expr<'_>, literal_str: &str, cast_from: Ty<'_>, cast_to: Ty<'_>) { - let literal_kind_name = if cast_from.is_integral() { "integer" } else { "float" }; - span_lint_and_sugg( - cx, - UNNECESSARY_CAST, - expr.span, - &format!("casting {} literal to `{}` is unnecessary", literal_kind_name, cast_to), - "try", - format!("{}_{}", literal_str.trim_end_matches('.'), cast_to), - Applicability::MachineApplicable, - ); -} - -fn lint_numeric_casts<'tcx>( - cx: &LateContext<'tcx>, - expr: &Expr<'tcx>, - cast_expr: &Expr<'_>, - cast_from: Ty<'tcx>, - cast_to: Ty<'tcx>, -) { - match (cast_from.is_integral(), cast_to.is_integral()) { - (true, false) => { - let from_nbits = int_ty_to_nbits(cast_from, cx.tcx); - let to_nbits = if let ty::Float(FloatTy::F32) = cast_to.kind() { - 32 - } else { - 64 - }; - if is_isize_or_usize(cast_from) || from_nbits >= to_nbits { - span_precision_loss_lint(cx, expr, cast_from, to_nbits == 64); - } - if from_nbits < to_nbits { - span_lossless_lint(cx, expr, cast_expr, cast_from, cast_to); - } - }, - (false, true) => { - span_lint( - cx, - CAST_POSSIBLE_TRUNCATION, - expr.span, - &format!("casting `{}` to `{}` may truncate the value", cast_from, cast_to), - ); - if !cast_to.is_signed() { - span_lint( - cx, - CAST_SIGN_LOSS, - expr.span, - &format!( - "casting `{}` to `{}` may lose the sign of the value", - cast_from, cast_to - ), - ); - } - }, - (true, true) => { - check_loss_of_sign(cx, expr, cast_expr, cast_from, cast_to); - check_truncation_and_wrapping(cx, expr, cast_from, cast_to); - check_lossless(cx, expr, cast_expr, cast_from, cast_to); - }, - (false, false) => { - if let (&ty::Float(FloatTy::F64), &ty::Float(FloatTy::F32)) = (&cast_from.kind(), &cast_to.kind()) { - span_lint( - cx, - CAST_POSSIBLE_TRUNCATION, - expr.span, - "casting `f64` to `f32` may truncate the value", - ); - } - if let (&ty::Float(FloatTy::F32), &ty::Float(FloatTy::F64)) = (&cast_from.kind(), &cast_to.kind()) { - span_lossless_lint(cx, expr, cast_expr, cast_from, cast_to); - } - }, - } -} - -fn lint_cast_ptr_alignment<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, cast_from: Ty<'tcx>, cast_to: Ty<'tcx>) { - if_chain! { - if let ty::RawPtr(from_ptr_ty) = &cast_from.kind(); - if let ty::RawPtr(to_ptr_ty) = &cast_to.kind(); - if let Ok(from_layout) = cx.layout_of(from_ptr_ty.ty); - if let Ok(to_layout) = cx.layout_of(to_ptr_ty.ty); - if from_layout.align.abi < to_layout.align.abi; - // with c_void, we inherently need to trust the user - if !is_c_void(cx, from_ptr_ty.ty); - // when casting from a ZST, we don't know enough to properly lint - if !from_layout.is_zst(); - then { - span_lint( - cx, - CAST_PTR_ALIGNMENT, - expr.span, - &format!( - "casting from `{}` to a more-strictly-aligned pointer (`{}`) ({} < {} bytes)", - cast_from, - cast_to, - from_layout.align.abi.bytes(), - to_layout.align.abi.bytes(), - ), - ); - } - } -} - -fn lint_fn_to_numeric_cast( - cx: &LateContext<'_>, - expr: &Expr<'_>, - cast_expr: &Expr<'_>, - cast_from: Ty<'_>, - cast_to: Ty<'_>, -) { - // We only want to check casts to `ty::Uint` or `ty::Int` - match cast_to.kind() { - ty::Uint(_) | ty::Int(..) => { /* continue on */ }, - _ => return, - } - match cast_from.kind() { - ty::FnDef(..) | ty::FnPtr(_) => { - let mut applicability = Applicability::MaybeIncorrect; - let from_snippet = snippet_with_applicability(cx, cast_expr.span, "x", &mut applicability); - - let to_nbits = int_ty_to_nbits(cast_to, cx.tcx); - if to_nbits < cx.tcx.data_layout.pointer_size.bits() { - span_lint_and_sugg( - cx, - FN_TO_NUMERIC_CAST_WITH_TRUNCATION, - expr.span, - &format!( - "casting function pointer `{}` to `{}`, which truncates the value", - from_snippet, cast_to - ), - "try", - format!("{} as usize", from_snippet), - applicability, - ); - } else if *cast_to.kind() != ty::Uint(UintTy::Usize) { - span_lint_and_sugg( - cx, - FN_TO_NUMERIC_CAST, - expr.span, - &format!("casting function pointer `{}` to `{}`", from_snippet, cast_to), - "try", - format!("{} as usize", from_snippet), - applicability, - ); - } - }, - _ => {}, - } -} - declare_clippy_lint! { /// **What it does:** Checks for types used in structs, parameters and `let` /// declarations above a certain complexity threshold. @@ -1686,69 +939,6 @@ impl<'tcx> Visitor<'tcx> for TypeComplexityVisitor { } } -declare_clippy_lint! { - /// **What it does:** Checks for expressions where a character literal is cast - /// to `u8` and suggests using a byte literal instead. - /// - /// **Why is this bad?** In general, casting values to smaller types is - /// error-prone and should be avoided where possible. In the particular case of - /// converting a character literal to u8, it is easy to avoid by just using a - /// byte literal instead. As an added bonus, `b'a'` is even slightly shorter - /// than `'a' as u8`. - /// - /// **Known problems:** None. - /// - /// **Example:** - /// ```rust,ignore - /// 'x' as u8 - /// ``` - /// - /// A better version, using the byte literal: - /// - /// ```rust,ignore - /// b'x' - /// ``` - pub CHAR_LIT_AS_U8, - complexity, - "casting a character literal to `u8` truncates" -} - -declare_lint_pass!(CharLitAsU8 => [CHAR_LIT_AS_U8]); - -impl<'tcx> LateLintPass<'tcx> for CharLitAsU8 { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if_chain! { - if !expr.span.from_expansion(); - if let ExprKind::Cast(e, _) = &expr.kind; - if let ExprKind::Lit(l) = &e.kind; - if let LitKind::Char(c) = l.node; - if ty::Uint(UintTy::U8) == *cx.typeck_results().expr_ty(expr).kind(); - then { - let mut applicability = Applicability::MachineApplicable; - let snippet = snippet_with_applicability(cx, e.span, "'x'", &mut applicability); - - span_lint_and_then( - cx, - CHAR_LIT_AS_U8, - expr.span, - "casting a character literal to `u8` truncates", - |diag| { - diag.note("`char` is four bytes wide, but `u8` is a single byte"); - - if c.is_ascii() { - diag.span_suggestion( - expr.span, - "use a byte literal instead", - format!("b{}", snippet), - applicability, - ); - } - }); - } - } - } -} - declare_clippy_lint! { /// **What it does:** Checks for comparisons where one side of the relation is /// either the minimum or maximum value for its type and warns if it involves a @@ -2477,149 +1667,6 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for ImplicitHasherConstructorVisitor<'a, 'b, 't } } -declare_clippy_lint! { - /// **What it does:** Checks for casts of `&T` to `&mut T` anywhere in the code. - /// - /// **Why is this bad?** It’s basically guaranteed to be undefined behaviour. - /// `UnsafeCell` is the only way to obtain aliasable data that is considered - /// mutable. - /// - /// **Known problems:** None. - /// - /// **Example:** - /// ```rust,ignore - /// fn x(r: &i32) { - /// unsafe { - /// *(r as *const _ as *mut _) += 1; - /// } - /// } - /// ``` - /// - /// Instead consider using interior mutability types. - /// - /// ```rust - /// use std::cell::UnsafeCell; - /// - /// fn x(r: &UnsafeCell) { - /// unsafe { - /// *r.get() += 1; - /// } - /// } - /// ``` - pub CAST_REF_TO_MUT, - correctness, - "a cast of reference to a mutable pointer" -} - -declare_lint_pass!(RefToMut => [CAST_REF_TO_MUT]); - -impl<'tcx> LateLintPass<'tcx> for RefToMut { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if_chain! { - if let ExprKind::Unary(UnOp::Deref, e) = &expr.kind; - if let ExprKind::Cast(e, t) = &e.kind; - if let TyKind::Ptr(MutTy { mutbl: Mutability::Mut, .. }) = t.kind; - if let ExprKind::Cast(e, t) = &e.kind; - if let TyKind::Ptr(MutTy { mutbl: Mutability::Not, .. }) = t.kind; - if let ty::Ref(..) = cx.typeck_results().node_type(e.hir_id).kind(); - then { - span_lint( - cx, - CAST_REF_TO_MUT, - expr.span, - "casting `&T` to `&mut T` may cause undefined behavior, consider instead using an `UnsafeCell`", - ); - } - } - } -} - -const PTR_AS_PTR_MSRV: RustcVersion = RustcVersion::new(1, 38, 0); - -declare_clippy_lint! { - /// **What it does:** - /// Checks for `as` casts between raw pointers without changing its mutability, - /// namely `*const T` to `*const U` and `*mut T` to `*mut U`. - /// - /// **Why is this bad?** - /// Though `as` casts between raw pointers is not terrible, `pointer::cast` is safer because - /// it cannot accidentally change the pointer's mutability nor cast the pointer to other types like `usize`. - /// - /// **Known problems:** None. - /// - /// **Example:** - /// - /// ```rust - /// let ptr: *const u32 = &42_u32; - /// let mut_ptr: *mut u32 = &mut 42_u32; - /// let _ = ptr as *const i32; - /// let _ = mut_ptr as *mut i32; - /// ``` - /// Use instead: - /// ```rust - /// let ptr: *const u32 = &42_u32; - /// let mut_ptr: *mut u32 = &mut 42_u32; - /// let _ = ptr.cast::(); - /// let _ = mut_ptr.cast::(); - /// ``` - pub PTR_AS_PTR, - pedantic, - "casting using `as` from and to raw pointers that doesn't change its mutability, where `pointer::cast` could take the place of `as`" -} - -pub struct PtrAsPtr { - msrv: Option, -} - -impl PtrAsPtr { - #[must_use] - pub fn new(msrv: Option) -> Self { - Self { msrv } - } -} - -impl_lint_pass!(PtrAsPtr => [PTR_AS_PTR]); - -impl<'tcx> LateLintPass<'tcx> for PtrAsPtr { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if !meets_msrv(self.msrv.as_ref(), &PTR_AS_PTR_MSRV) { - return; - } - - if expr.span.from_expansion() { - return; - } - - if_chain! { - if let ExprKind::Cast(cast_expr, cast_to_hir_ty) = expr.kind; - let (cast_from, cast_to) = (cx.typeck_results().expr_ty(cast_expr), cx.typeck_results().expr_ty(expr)); - if let ty::RawPtr(TypeAndMut { mutbl: from_mutbl, .. }) = cast_from.kind(); - if let ty::RawPtr(TypeAndMut { ty: to_pointee_ty, mutbl: to_mutbl }) = cast_to.kind(); - if matches!((from_mutbl, to_mutbl), - (Mutability::Not, Mutability::Not) | (Mutability::Mut, Mutability::Mut)); - // The `U` in `pointer::cast` have to be `Sized` - // as explained here: https://github.com/rust-lang/rust/issues/60602. - if to_pointee_ty.is_sized(cx.tcx.at(expr.span), cx.param_env); - then { - let mut applicability = Applicability::MachineApplicable; - let cast_expr_sugg = Sugg::hir_with_applicability(cx, cast_expr, "_", &mut applicability); - let turbofish = match &cast_to_hir_ty.kind { - TyKind::Infer => Cow::Borrowed(""), - TyKind::Ptr(mut_ty) if matches!(mut_ty.ty.kind, TyKind::Infer) => Cow::Borrowed(""), - _ => Cow::Owned(format!("::<{}>", to_pointee_ty)), - }; - span_lint_and_sugg( - cx, - PTR_AS_PTR, - expr.span, - "`as` casting between raw pointers without changing its mutability", - "try `pointer::cast`, a safer alternative", - format!("{}.cast{}()", cast_expr_sugg.maybe_par(), turbofish), - applicability, - ); - } - } - } - - extract_msrv_attr!(LateContext); +fn is_isize_or_usize(typ: Ty<'_>) -> bool { + matches!(typ.kind(), ty::Int(IntTy::Isize) | ty::Uint(UintTy::Usize)) } From 360f0654047ef45212440e0cc9dcfe77c970b1eb Mon Sep 17 00:00:00 2001 From: Yoshitomo Nakanishi Date: Tue, 9 Mar 2021 11:08:26 +0900 Subject: [PATCH 02/15] Move 'is_isize_or_usize' to clippy_utils --- clippy_lints/src/casts/mod.rs | 9 +++------ clippy_lints/src/types/mod.rs | 10 +++------- clippy_utils/src/lib.rs | 7 ++++++- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index d8245a3b88f7..95e563fc3bab 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -15,8 +15,9 @@ use rustc_target::abi::LayoutOf; use crate::consts::{constant, Constant}; use crate::utils::sugg::Sugg; use crate::utils::{ - in_constant, is_hir_ty_cfg_dependant, meets_msrv, method_chain_args, numeric_literal::NumericLiteral, sext, - snippet_opt, snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then, + in_constant, is_hir_ty_cfg_dependant, is_isize_or_usize, meets_msrv, method_chain_args, + numeric_literal::NumericLiteral, sext, snippet_opt, snippet_with_applicability, span_lint, span_lint_and_sugg, + span_lint_and_then, }; declare_clippy_lint! { @@ -967,7 +968,3 @@ impl<'tcx> LateLintPass<'tcx> for PtrAsPtr { extract_msrv_attr!(LateContext); } - -fn is_isize_or_usize(typ: Ty<'_>) -> bool { - matches!(typ.kind(), ty::Int(IntTy::Isize) | ty::Uint(UintTy::Usize)) -} diff --git a/clippy_lints/src/types/mod.rs b/clippy_lints/src/types/mod.rs index 3e7aa53d5bc9..7be4ea037931 100644 --- a/clippy_lints/src/types/mod.rs +++ b/clippy_lints/src/types/mod.rs @@ -37,9 +37,9 @@ use rustc_typeck::hir_ty_to_ty; use crate::consts::{constant, Constant}; use crate::utils::paths; use crate::utils::{ - clip, comparisons, differing_macro_contexts, higher, indent_of, int_bits, is_type_diagnostic_item, match_path, - multispan_sugg, reindent_multiline, sext, snippet, snippet_opt, snippet_with_macro_callsite, span_lint, - span_lint_and_help, span_lint_and_then, unsext, + clip, comparisons, differing_macro_contexts, higher, indent_of, int_bits, is_isize_or_usize, + is_type_diagnostic_item, match_path, multispan_sugg, reindent_multiline, sext, snippet, snippet_opt, + snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_then, unsext, }; declare_clippy_lint! { @@ -1666,7 +1666,3 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for ImplicitHasherConstructorVisitor<'a, 'b, 't NestedVisitorMap::OnlyBodies(self.cx.tcx.hir()) } } - -fn is_isize_or_usize(typ: Ty<'_>) -> bool { - matches!(typ.kind(), ty::Int(IntTy::Isize) | ty::Uint(UintTy::Usize)) -} diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 3845667802d8..74429982759b 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -72,7 +72,7 @@ use rustc_lint::{LateContext, Level, Lint, LintContext}; use rustc_middle::hir::exports::Export; use rustc_middle::hir::map::Map; use rustc_middle::ty::subst::{GenericArg, GenericArgKind}; -use rustc_middle::ty::{self, layout::IntegerExt, DefIdTree, Ty, TyCtxt, TypeFoldable}; +use rustc_middle::ty::{self, layout::IntegerExt, DefIdTree, IntTy, Ty, TyCtxt, TypeFoldable, UintTy}; use rustc_semver::RustcVersion; use rustc_session::Session; use rustc_span::hygiene::{self, ExpnKind, MacroKind}; @@ -262,6 +262,11 @@ pub fn is_ty_param_diagnostic_item( } } +/// Return `true` if the passed `typ` is `isize` or `usize`. +pub fn is_isize_or_usize(typ: Ty<'_>) -> bool { + matches!(typ.kind(), ty::Int(IntTy::Isize) | ty::Uint(UintTy::Usize)) +} + /// Checks if the method call given in `expr` belongs to the given trait. pub fn match_trait_method(cx: &LateContext<'_>, expr: &Expr<'_>, path: &[&str]) -> bool { let def_id = cx.typeck_results().type_dependent_def_id(expr.hir_id).unwrap(); From c2cbcd32299270487388442e3dc684619279ae6b Mon Sep 17 00:00:00 2001 From: Yoshitomo Nakanishi Date: Tue, 9 Mar 2021 11:22:17 +0900 Subject: [PATCH 03/15] Move cast_precision_loss to its own module --- clippy_lints/src/casts/cast_precision_loss.rs | 51 +++++++++++++++ clippy_lints/src/casts/mod.rs | 62 +++---------------- clippy_lints/src/casts/utils.rs | 25 ++++++++ 3 files changed, 83 insertions(+), 55 deletions(-) create mode 100644 clippy_lints/src/casts/cast_precision_loss.rs create mode 100644 clippy_lints/src/casts/utils.rs diff --git a/clippy_lints/src/casts/cast_precision_loss.rs b/clippy_lints/src/casts/cast_precision_loss.rs new file mode 100644 index 000000000000..a1c3900ce1f6 --- /dev/null +++ b/clippy_lints/src/casts/cast_precision_loss.rs @@ -0,0 +1,51 @@ +use rustc_hir::Expr; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, FloatTy, Ty}; + +use crate::utils::{is_isize_or_usize, span_lint}; + +use super::{utils, CAST_PRECISION_LOSS}; + +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 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 + }; + + if !(is_isize_or_usize(cast_from) || from_nbits >= to_nbits) { + return; + } + + let cast_to_f64 = to_nbits == 64; + let mantissa_nbits = if cast_to_f64 { 52 } else { 23 }; + let arch_dependent = is_isize_or_usize(cast_from) && cast_to_f64; + let arch_dependent_str = "on targets with 64-bit wide pointers "; + let from_nbits_str = if arch_dependent { + "64".to_owned() + } else if is_isize_or_usize(cast_from) { + "32 or 64".to_owned() + } else { + utils::int_ty_to_nbits(cast_from, cx.tcx).to_string() + }; + + span_lint( + cx, + CAST_PRECISION_LOSS, + expr.span, + &format!( + "casting `{0}` to `{1}` causes a loss of precision {2}(`{0}` is {3} bits wide, \ + but `{1}`'s mantissa is only {4} bits wide)", + cast_from, + if cast_to_f64 { "f64" } else { "f32" }, + if arch_dependent { arch_dependent_str } else { "" }, + from_nbits_str, + mantissa_nbits + ), + ); +} diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index 95e563fc3bab..7eb35aa82908 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -1,3 +1,6 @@ +mod cast_precision_loss; +mod utils; + use std::borrow::Cow; use if_chain::if_chain; @@ -6,7 +9,7 @@ use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, GenericArg, Lit, MutTy, Mutability, TyKind, UnOp}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; -use rustc_middle::ty::{self, FloatTy, InferTy, IntTy, Ty, TyCtxt, TypeAndMut, UintTy}; +use rustc_middle::ty::{self, FloatTy, InferTy, Ty, TypeAndMut, UintTy}; use rustc_semver::RustcVersion; use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; use rustc_span::symbol::sym; @@ -20,6 +23,8 @@ use crate::utils::{ span_lint_and_then, }; +use utils::int_ty_to_nbits; + declare_clippy_lint! { /// **What it does:** Checks for casts from any numerical to a float type where /// the receiving type cannot store all values from the original type without @@ -249,57 +254,6 @@ declare_clippy_lint! { "casting a function pointer to a numeric type not wide enough to store the address" } -/// Returns the size in bits of an integral type. -/// Will return 0 if the type is not an int or uint variant -fn int_ty_to_nbits(typ: Ty<'_>, tcx: TyCtxt<'_>) -> u64 { - match typ.kind() { - ty::Int(i) => match i { - IntTy::Isize => tcx.data_layout.pointer_size.bits(), - IntTy::I8 => 8, - IntTy::I16 => 16, - IntTy::I32 => 32, - IntTy::I64 => 64, - IntTy::I128 => 128, - }, - ty::Uint(i) => match i { - UintTy::Usize => tcx.data_layout.pointer_size.bits(), - UintTy::U8 => 8, - UintTy::U16 => 16, - UintTy::U32 => 32, - UintTy::U64 => 64, - UintTy::U128 => 128, - }, - _ => 0, - } -} - -fn span_precision_loss_lint(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to_f64: bool) { - let mantissa_nbits = if cast_to_f64 { 52 } else { 23 }; - let arch_dependent = is_isize_or_usize(cast_from) && cast_to_f64; - let arch_dependent_str = "on targets with 64-bit wide pointers "; - let from_nbits_str = if arch_dependent { - "64".to_owned() - } else if is_isize_or_usize(cast_from) { - "32 or 64".to_owned() - } else { - int_ty_to_nbits(cast_from, cx.tcx).to_string() - }; - span_lint( - cx, - CAST_PRECISION_LOSS, - expr.span, - &format!( - "casting `{0}` to `{1}` causes a loss of precision {2}(`{0}` is {3} bits wide, \ - but `{1}`'s mantissa is only {4} bits wide)", - cast_from, - if cast_to_f64 { "f64" } else { "f32" }, - if arch_dependent { arch_dependent_str } else { "" }, - from_nbits_str, - mantissa_nbits - ), - ); -} - fn should_strip_parens(op: &Expr<'_>, snip: &str) -> bool { if let ExprKind::Binary(_, _, _) = op.kind { if snip.starts_with('(') && snip.ends_with(')') { @@ -629,6 +583,7 @@ fn lint_numeric_casts<'tcx>( cast_from: Ty<'tcx>, cast_to: Ty<'tcx>, ) { + cast_precision_loss::check(cx, expr, cast_from, cast_to); match (cast_from.is_integral(), cast_to.is_integral()) { (true, false) => { let from_nbits = int_ty_to_nbits(cast_from, cx.tcx); @@ -637,9 +592,6 @@ fn lint_numeric_casts<'tcx>( } else { 64 }; - if is_isize_or_usize(cast_from) || from_nbits >= to_nbits { - span_precision_loss_lint(cx, expr, cast_from, to_nbits == 64); - } if from_nbits < to_nbits { span_lossless_lint(cx, expr, cast_expr, cast_from, cast_to); } diff --git a/clippy_lints/src/casts/utils.rs b/clippy_lints/src/casts/utils.rs new file mode 100644 index 000000000000..00fd0b3473b4 --- /dev/null +++ b/clippy_lints/src/casts/utils.rs @@ -0,0 +1,25 @@ +use rustc_middle::ty::{self, IntTy, Ty, TyCtxt, UintTy}; + +/// Returns the size in bits of an integral type. +/// Will return 0 if the type is not an int or uint variant +pub(super) fn int_ty_to_nbits(typ: Ty<'_>, tcx: TyCtxt<'_>) -> u64 { + match typ.kind() { + ty::Int(i) => match i { + IntTy::Isize => tcx.data_layout.pointer_size.bits(), + IntTy::I8 => 8, + IntTy::I16 => 16, + IntTy::I32 => 32, + IntTy::I64 => 64, + IntTy::I128 => 128, + }, + ty::Uint(i) => match i { + UintTy::Usize => tcx.data_layout.pointer_size.bits(), + UintTy::U8 => 8, + UintTy::U16 => 16, + UintTy::U32 => 32, + UintTy::U64 => 64, + UintTy::U128 => 128, + }, + _ => 0, + } +} From b12d7515b1ee867b0ef23b489f5b3166bed31ae3 Mon Sep 17 00:00:00 2001 From: Yoshitomo Nakanishi Date: Tue, 9 Mar 2021 11:44:52 +0900 Subject: [PATCH 04/15] Move cast_lossless to its own module --- clippy_lints/src/casts/cast_lossless.rs | 87 +++++++++++++++++++++++++ clippy_lints/src/casts/mod.rs | 79 ++-------------------- 2 files changed, 92 insertions(+), 74 deletions(-) create mode 100644 clippy_lints/src/casts/cast_lossless.rs diff --git a/clippy_lints/src/casts/cast_lossless.rs b/clippy_lints/src/casts/cast_lossless.rs new file mode 100644 index 000000000000..478832a5164a --- /dev/null +++ b/clippy_lints/src/casts/cast_lossless.rs @@ -0,0 +1,87 @@ +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, FloatTy, Ty}; + +use crate::utils::{in_constant, is_isize_or_usize, snippet_opt, span_lint_and_sugg}; + +use super::{utils, CAST_LOSSLESS}; + +pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { + if !should_lint(cx, expr, cast_from, cast_to) { + return; + } + + // The suggestion is to use a function call, so if the original expression + // has parens on the outside, they are no longer needed. + let mut applicability = Applicability::MachineApplicable; + let opt = snippet_opt(cx, cast_op.span); + let sugg = opt.as_ref().map_or_else( + || { + applicability = Applicability::HasPlaceholders; + ".." + }, + |snip| { + if should_strip_parens(cast_op, snip) { + &snip[1..snip.len() - 1] + } else { + snip.as_str() + } + }, + ); + + span_lint_and_sugg( + cx, + CAST_LOSSLESS, + expr.span, + &format!( + "casting `{}` to `{}` may become silently lossy if you later change the type", + cast_from, cast_to + ), + "try", + format!("{}::from({})", cast_to, sugg), + applicability, + ); +} + +fn should_lint(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) -> bool { + // Do not suggest using From in consts/statics until it is valid to do so (see #2267). + if in_constant(cx, expr.hir_id) { + 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 + }, + + (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 + }; + from_nbits < to_nbits + }, + + (_, _) => { + matches!(cast_from.kind(), ty::Float(FloatTy::F32)) && matches!(cast_to.kind(), ty::Float(FloatTy::F64)) + }, + } +} + +fn should_strip_parens(cast_expr: &Expr<'_>, snip: &str) -> bool { + if let ExprKind::Binary(_, _, _) = cast_expr.kind { + if snip.starts_with('(') && snip.ends_with(')') { + return true; + } + } + false +} diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index 7eb35aa82908..6c2464e0090b 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -1,3 +1,4 @@ +mod cast_lossless; mod cast_precision_loss; mod utils; @@ -18,9 +19,8 @@ use rustc_target::abi::LayoutOf; use crate::consts::{constant, Constant}; use crate::utils::sugg::Sugg; use crate::utils::{ - in_constant, is_hir_ty_cfg_dependant, is_isize_or_usize, meets_msrv, method_chain_args, - numeric_literal::NumericLiteral, sext, snippet_opt, snippet_with_applicability, span_lint, span_lint_and_sugg, - span_lint_and_then, + is_hir_ty_cfg_dependant, is_isize_or_usize, meets_msrv, method_chain_args, numeric_literal::NumericLiteral, sext, + snippet_opt, snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then, }; use utils::int_ty_to_nbits; @@ -254,52 +254,6 @@ declare_clippy_lint! { "casting a function pointer to a numeric type not wide enough to store the address" } -fn should_strip_parens(op: &Expr<'_>, snip: &str) -> bool { - if let ExprKind::Binary(_, _, _) = op.kind { - if snip.starts_with('(') && snip.ends_with(')') { - return true; - } - } - false -} - -fn span_lossless_lint(cx: &LateContext<'_>, expr: &Expr<'_>, op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { - // Do not suggest using From in consts/statics until it is valid to do so (see #2267). - if in_constant(cx, expr.hir_id) { - return; - } - // The suggestion is to use a function call, so if the original expression - // has parens on the outside, they are no longer needed. - let mut applicability = Applicability::MachineApplicable; - let opt = snippet_opt(cx, op.span); - let sugg = opt.as_ref().map_or_else( - || { - applicability = Applicability::HasPlaceholders; - ".." - }, - |snip| { - if should_strip_parens(op, snip) { - &snip[1..snip.len() - 1] - } else { - snip.as_str() - } - }, - ); - - span_lint_and_sugg( - cx, - CAST_LOSSLESS, - expr.span, - &format!( - "casting `{}` to `{}` may become silently lossy if you later change the type", - cast_from, cast_to - ), - "try", - format!("{}::from({})", cast_to, sugg), - applicability, - ); -} - enum ArchSuffix { _32, _64, @@ -423,16 +377,6 @@ fn check_truncation_and_wrapping(cx: &LateContext<'_>, expr: &Expr<'_>, cast_fro } } -fn check_lossless(cx: &LateContext<'_>, expr: &Expr<'_>, op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { - let cast_signed_to_unsigned = cast_from.is_signed() && !cast_to.is_signed(); - let from_nbits = int_ty_to_nbits(cast_from, cx.tcx); - let to_nbits = int_ty_to_nbits(cast_to, cx.tcx); - if !is_isize_or_usize(cast_from) && !is_isize_or_usize(cast_to) && from_nbits < to_nbits && !cast_signed_to_unsigned - { - span_lossless_lint(cx, expr, op, cast_from, cast_to); - } -} - declare_lint_pass!(Casts => [ CAST_PRECISION_LOSS, CAST_SIGN_LOSS, @@ -584,18 +528,8 @@ fn lint_numeric_casts<'tcx>( cast_to: Ty<'tcx>, ) { cast_precision_loss::check(cx, expr, cast_from, cast_to); + cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to); match (cast_from.is_integral(), cast_to.is_integral()) { - (true, false) => { - let from_nbits = int_ty_to_nbits(cast_from, cx.tcx); - let to_nbits = if let ty::Float(FloatTy::F32) = cast_to.kind() { - 32 - } else { - 64 - }; - if from_nbits < to_nbits { - span_lossless_lint(cx, expr, cast_expr, cast_from, cast_to); - } - }, (false, true) => { span_lint( cx, @@ -618,7 +552,6 @@ fn lint_numeric_casts<'tcx>( (true, true) => { check_loss_of_sign(cx, expr, cast_expr, cast_from, cast_to); check_truncation_and_wrapping(cx, expr, cast_from, cast_to); - check_lossless(cx, expr, cast_expr, cast_from, cast_to); }, (false, false) => { if let (&ty::Float(FloatTy::F64), &ty::Float(FloatTy::F32)) = (&cast_from.kind(), &cast_to.kind()) { @@ -629,10 +562,8 @@ fn lint_numeric_casts<'tcx>( "casting `f64` to `f32` may truncate the value", ); } - if let (&ty::Float(FloatTy::F32), &ty::Float(FloatTy::F64)) = (&cast_from.kind(), &cast_to.kind()) { - span_lossless_lint(cx, expr, cast_expr, cast_from, cast_to); - } }, + (_, _) => {}, } } From 0534bf4698acc7b302664bb80d9c85be2648fcd8 Mon Sep 17 00:00:00 2001 From: Yoshitomo Nakanishi Date: Tue, 9 Mar 2021 12:10:59 +0900 Subject: [PATCH 05/15] Move cast_possible_truncation to its own module --- .../src/casts/cast_possible_truncation.rs | 54 +++++++++++++ clippy_lints/src/casts/mod.rs | 76 ++++--------------- 2 files changed, 68 insertions(+), 62 deletions(-) create mode 100644 clippy_lints/src/casts/cast_possible_truncation.rs diff --git a/clippy_lints/src/casts/cast_possible_truncation.rs b/clippy_lints/src/casts/cast_possible_truncation.rs new file mode 100644 index 000000000000..33b06b8fe7ca --- /dev/null +++ b/clippy_lints/src/casts/cast_possible_truncation.rs @@ -0,0 +1,54 @@ +use rustc_hir::Expr; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, FloatTy, Ty}; + +use crate::utils::{is_isize_or_usize, span_lint}; + +use super::{utils, CAST_POSSIBLE_TRUNCATION}; + +pub(super) fn check(cx: &LateContext<'_>, 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 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, ""), + (true, false) => ( + to_nbits <= 32, + if to_nbits == 32 { + " on targets with 64-bit wide pointers" + } else { + "" + }, + ), + (false, true) => (from_nbits == 64, " on targets with 32-bit wide pointers"), + }; + + if !should_lint { + return; + } + + format!( + "casting `{}` to `{}` may truncate the value{}", + cast_from, cast_to, suffix, + ) + }, + + (false, true) => { + format!("casting `{}` to `{}` may truncate the value", cast_from, cast_to) + }, + + (_, _) => { + if matches!(cast_from.kind(), &ty::Float(FloatTy::F64)) + && matches!(cast_to.kind(), &ty::Float(FloatTy::F32)) + { + "casting `f64` to `f32` may truncate the value".to_string() + } else { + return; + } + }, + }; + + span_lint(cx, CAST_POSSIBLE_TRUNCATION, expr.span, &msg); +} diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index 6c2464e0090b..9b89b076d906 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -1,4 +1,5 @@ mod cast_lossless; +mod cast_possible_truncation; mod cast_precision_loss; mod utils; @@ -312,52 +313,18 @@ fn check_truncation_and_wrapping(cx: &LateContext<'_>, expr: &Expr<'_>, cast_fro let cast_unsigned_to_signed = !cast_from.is_signed() && cast_to.is_signed(); let from_nbits = int_ty_to_nbits(cast_from, cx.tcx); let to_nbits = int_ty_to_nbits(cast_to, cx.tcx); - let (span_truncation, suffix_truncation, span_wrap, suffix_wrap) = - match (is_isize_or_usize(cast_from), is_isize_or_usize(cast_to)) { - (true, true) | (false, false) => ( - to_nbits < from_nbits, - ArchSuffix::None, - to_nbits == from_nbits && cast_unsigned_to_signed, - ArchSuffix::None, - ), - (true, false) => ( - to_nbits <= 32, - if to_nbits == 32 { - ArchSuffix::_64 - } else { - ArchSuffix::None - }, - to_nbits <= 32 && cast_unsigned_to_signed, - ArchSuffix::_32, - ), - (false, true) => ( - from_nbits == 64, - ArchSuffix::_32, - cast_unsigned_to_signed, - if from_nbits == 64 { - ArchSuffix::_64 - } else { - ArchSuffix::_32 - }, - ), - }; - if span_truncation { - span_lint( - cx, - CAST_POSSIBLE_TRUNCATION, - expr.span, - &format!( - "casting `{}` to `{}` may truncate the value{}", - cast_from, - cast_to, - match suffix_truncation { - ArchSuffix::_32 => arch_32_suffix, - ArchSuffix::_64 => arch_64_suffix, - ArchSuffix::None => "", - } - ), - ); - } + let (span_wrap, suffix_wrap) = 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, ArchSuffix::None), + (true, false) => (to_nbits <= 32 && cast_unsigned_to_signed, ArchSuffix::_32), + (false, true) => ( + cast_unsigned_to_signed, + if from_nbits == 64 { + ArchSuffix::_64 + } else { + ArchSuffix::_32 + }, + ), + }; if span_wrap { span_lint( cx, @@ -529,14 +496,9 @@ fn lint_numeric_casts<'tcx>( ) { cast_precision_loss::check(cx, expr, cast_from, cast_to); cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to); + cast_possible_truncation::check(cx, expr, cast_from, cast_to); match (cast_from.is_integral(), cast_to.is_integral()) { (false, true) => { - span_lint( - cx, - CAST_POSSIBLE_TRUNCATION, - expr.span, - &format!("casting `{}` to `{}` may truncate the value", cast_from, cast_to), - ); if !cast_to.is_signed() { span_lint( cx, @@ -553,16 +515,6 @@ fn lint_numeric_casts<'tcx>( check_loss_of_sign(cx, expr, cast_expr, cast_from, cast_to); check_truncation_and_wrapping(cx, expr, cast_from, cast_to); }, - (false, false) => { - if let (&ty::Float(FloatTy::F64), &ty::Float(FloatTy::F32)) = (&cast_from.kind(), &cast_to.kind()) { - span_lint( - cx, - CAST_POSSIBLE_TRUNCATION, - expr.span, - "casting `f64` to `f32` may truncate the value", - ); - } - }, (_, _) => {}, } } From 0975031117fc1de35220f0e16d2294acaab154f8 Mon Sep 17 00:00:00 2001 From: Yoshitomo Nakanishi Date: Tue, 9 Mar 2021 12:37:46 +0900 Subject: [PATCH 06/15] Move cast_sign_loss to its own module --- clippy_lints/src/casts/cast_sign_loss.rs | 70 ++++++++++++++++++++++ clippy_lints/src/casts/mod.rs | 74 +++--------------------- 2 files changed, 78 insertions(+), 66 deletions(-) create mode 100644 clippy_lints/src/casts/cast_sign_loss.rs diff --git a/clippy_lints/src/casts/cast_sign_loss.rs b/clippy_lints/src/casts/cast_sign_loss.rs new file mode 100644 index 000000000000..9656fbebd772 --- /dev/null +++ b/clippy_lints/src/casts/cast_sign_loss.rs @@ -0,0 +1,70 @@ +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, Ty}; + +use if_chain::if_chain; + +use crate::consts::{constant, Constant}; +use crate::utils::{method_chain_args, sext, span_lint}; + +use super::CAST_SIGN_LOSS; + +pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { + if should_lint(cx, cast_op, cast_from, cast_to) { + span_lint( + cx, + CAST_SIGN_LOSS, + expr.span, + &format!( + "casting `{}` to `{}` may lose the sign of the value", + cast_from, cast_to + ), + ); + } +} + +fn should_lint(cx: &LateContext<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) -> bool { + match (cast_from.is_integral(), cast_to.is_integral()) { + (true, true) => { + if !cast_from.is_signed() || cast_to.is_signed() { + return false; + } + + // Don't lint for positive constants. + let const_val = constant(cx, &cx.typeck_results(), cast_op); + if_chain! { + if let Some((Constant::Int(n), _)) = const_val; + if let ty::Int(ity) = *cast_from.kind(); + if sext(cx.tcx, n, ity) >= 0; + then { + return false; + } + } + + // Don't lint for the result of methods that always return non-negative values. + if let ExprKind::MethodCall(ref path, _, _, _) = cast_op.kind { + let mut method_name = path.ident.name.as_str(); + let allowed_methods = ["abs", "checked_abs", "rem_euclid", "checked_rem_euclid"]; + + if_chain! { + if method_name == "unwrap"; + if let Some(arglist) = method_chain_args(cast_op, &["unwrap"]); + if let ExprKind::MethodCall(ref inner_path, _, _, _) = &arglist[0][0].kind; + then { + method_name = inner_path.ident.name.as_str(); + } + } + + if allowed_methods.iter().any(|&name| method_name == name) { + return false; + } + } + + true + }, + + (false, true) => !cast_to.is_signed(), + + (_, _) => false, + } +} diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index 9b89b076d906..f390dfdfab76 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -1,6 +1,7 @@ mod cast_lossless; mod cast_possible_truncation; mod cast_precision_loss; +mod cast_sign_loss; mod utils; use std::borrow::Cow; @@ -17,11 +18,10 @@ use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; use rustc_span::symbol::sym; use rustc_target::abi::LayoutOf; -use crate::consts::{constant, Constant}; use crate::utils::sugg::Sugg; use crate::utils::{ - is_hir_ty_cfg_dependant, is_isize_or_usize, meets_msrv, method_chain_args, numeric_literal::NumericLiteral, sext, - snippet_opt, snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then, + is_hir_ty_cfg_dependant, is_isize_or_usize, meets_msrv, numeric_literal::NumericLiteral, snippet_opt, + snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then, }; use utils::int_ty_to_nbits; @@ -261,52 +261,6 @@ enum ArchSuffix { None, } -fn check_loss_of_sign(cx: &LateContext<'_>, expr: &Expr<'_>, op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { - if !cast_from.is_signed() || cast_to.is_signed() { - return; - } - - // don't lint for positive constants - let const_val = constant(cx, &cx.typeck_results(), op); - if_chain! { - if let Some((Constant::Int(n), _)) = const_val; - if let ty::Int(ity) = *cast_from.kind(); - if sext(cx.tcx, n, ity) >= 0; - then { - return - } - } - - // don't lint for the result of methods that always return non-negative values - if let ExprKind::MethodCall(ref path, _, _, _) = op.kind { - let mut method_name = path.ident.name.as_str(); - let allowed_methods = ["abs", "checked_abs", "rem_euclid", "checked_rem_euclid"]; - - if_chain! { - if method_name == "unwrap"; - if let Some(arglist) = method_chain_args(op, &["unwrap"]); - if let ExprKind::MethodCall(ref inner_path, _, _, _) = &arglist[0][0].kind; - then { - method_name = inner_path.ident.name.as_str(); - } - } - - if allowed_methods.iter().any(|&name| method_name == name) { - return; - } - } - - span_lint( - cx, - CAST_SIGN_LOSS, - expr.span, - &format!( - "casting `{}` to `{}` may lose the sign of the value", - cast_from, cast_to - ), - ); -} - fn check_truncation_and_wrapping(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { let arch_64_suffix = " on targets with 64-bit wide pointers"; let arch_32_suffix = " on targets with 32-bit wide pointers"; @@ -490,29 +444,17 @@ fn show_unnecessary_cast(cx: &LateContext<'_>, expr: &Expr<'_>, literal_str: &st fn lint_numeric_casts<'tcx>( cx: &LateContext<'tcx>, expr: &Expr<'tcx>, - cast_expr: &Expr<'_>, + cast_op: &Expr<'_>, cast_from: Ty<'tcx>, cast_to: Ty<'tcx>, ) { - cast_precision_loss::check(cx, expr, cast_from, cast_to); - cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to); cast_possible_truncation::check(cx, expr, cast_from, cast_to); + cast_precision_loss::check(cx, expr, cast_from, cast_to); + cast_lossless::check(cx, expr, cast_op, cast_from, cast_to); + cast_sign_loss::check(cx, expr, cast_op, cast_from, cast_to); + match (cast_from.is_integral(), cast_to.is_integral()) { - (false, true) => { - if !cast_to.is_signed() { - span_lint( - cx, - CAST_SIGN_LOSS, - expr.span, - &format!( - "casting `{}` to `{}` may lose the sign of the value", - cast_from, cast_to - ), - ); - } - }, (true, true) => { - check_loss_of_sign(cx, expr, cast_expr, cast_from, cast_to); check_truncation_and_wrapping(cx, expr, cast_from, cast_to); }, (_, _) => {}, From a383e034dc2f3d9cb53bd7164447fe7f3bfbf08c Mon Sep 17 00:00:00 2001 From: Yoshitomo Nakanishi Date: Tue, 9 Mar 2021 16:11:46 +0900 Subject: [PATCH 07/15] Move cast_possible_wrap to its own module --- clippy_lints/src/casts/cast_possible_wrap.rs | 44 +++++++++++++++ clippy_lints/src/casts/mod.rs | 56 ++------------------ 2 files changed, 48 insertions(+), 52 deletions(-) create mode 100644 clippy_lints/src/casts/cast_possible_wrap.rs diff --git a/clippy_lints/src/casts/cast_possible_wrap.rs b/clippy_lints/src/casts/cast_possible_wrap.rs new file mode 100644 index 000000000000..56d301ed3e1c --- /dev/null +++ b/clippy_lints/src/casts/cast_possible_wrap.rs @@ -0,0 +1,44 @@ +use rustc_hir::Expr; +use rustc_lint::LateContext; +use rustc_middle::ty::Ty; + +use crate::utils::{is_isize_or_usize, span_lint}; + +use super::{utils, CAST_POSSIBLE_WRAP}; + +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(); + 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 + } else { + arch_32_suffix + }, + ), + }; + + if should_lint { + span_lint( + cx, + CAST_POSSIBLE_WRAP, + expr.span, + &format!( + "casting `{}` to `{}` may wrap around the value{}", + cast_from, cast_to, suffix, + ), + ); + } +} diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index f390dfdfab76..01528a5a16a0 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -1,5 +1,6 @@ mod cast_lossless; mod cast_possible_truncation; +mod cast_possible_wrap; mod cast_precision_loss; mod cast_sign_loss; mod utils; @@ -20,8 +21,8 @@ use rustc_target::abi::LayoutOf; use crate::utils::sugg::Sugg; use crate::utils::{ - is_hir_ty_cfg_dependant, is_isize_or_usize, meets_msrv, numeric_literal::NumericLiteral, snippet_opt, - snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then, + is_hir_ty_cfg_dependant, meets_msrv, numeric_literal::NumericLiteral, snippet_opt, snippet_with_applicability, + span_lint, span_lint_and_sugg, span_lint_and_then, }; use utils::int_ty_to_nbits; @@ -255,49 +256,6 @@ declare_clippy_lint! { "casting a function pointer to a numeric type not wide enough to store the address" } -enum ArchSuffix { - _32, - _64, - None, -} - -fn check_truncation_and_wrapping(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { - 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(); - let from_nbits = int_ty_to_nbits(cast_from, cx.tcx); - let to_nbits = int_ty_to_nbits(cast_to, cx.tcx); - let (span_wrap, suffix_wrap) = 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, ArchSuffix::None), - (true, false) => (to_nbits <= 32 && cast_unsigned_to_signed, ArchSuffix::_32), - (false, true) => ( - cast_unsigned_to_signed, - if from_nbits == 64 { - ArchSuffix::_64 - } else { - ArchSuffix::_32 - }, - ), - }; - if span_wrap { - span_lint( - cx, - CAST_POSSIBLE_WRAP, - expr.span, - &format!( - "casting `{}` to `{}` may wrap around the value{}", - cast_from, - cast_to, - match suffix_wrap { - ArchSuffix::_32 => arch_32_suffix, - ArchSuffix::_64 => arch_64_suffix, - ArchSuffix::None => "", - } - ), - ); - } -} - declare_lint_pass!(Casts => [ CAST_PRECISION_LOSS, CAST_SIGN_LOSS, @@ -449,16 +407,10 @@ fn lint_numeric_casts<'tcx>( cast_to: Ty<'tcx>, ) { cast_possible_truncation::check(cx, expr, cast_from, cast_to); + cast_possible_wrap::check(cx, expr, cast_from, cast_to); cast_precision_loss::check(cx, expr, cast_from, cast_to); cast_lossless::check(cx, expr, cast_op, cast_from, cast_to); cast_sign_loss::check(cx, expr, cast_op, cast_from, cast_to); - - match (cast_from.is_integral(), cast_to.is_integral()) { - (true, true) => { - check_truncation_and_wrapping(cx, expr, cast_from, cast_to); - }, - (_, _) => {}, - } } fn lint_cast_ptr_alignment<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, cast_from: Ty<'tcx>, cast_to: Ty<'tcx>) { From 9709993e412dd3400aec27d4fefae0cbf1f702aa Mon Sep 17 00:00:00 2001 From: Yoshitomo Nakanishi Date: Tue, 9 Mar 2021 16:43:10 +0900 Subject: [PATCH 08/15] Move unnecessary_cast to its own module --- clippy_lints/src/casts/mod.rs | 120 ++++----------------- clippy_lints/src/casts/unnecessary_cast.rs | 106 ++++++++++++++++++ 2 files changed, 128 insertions(+), 98 deletions(-) create mode 100644 clippy_lints/src/casts/unnecessary_cast.rs diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index 01528a5a16a0..d8c744365014 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -3,17 +3,18 @@ mod cast_possible_truncation; mod cast_possible_wrap; mod cast_precision_loss; mod cast_sign_loss; +mod unnecessary_cast; mod utils; use std::borrow::Cow; use if_chain::if_chain; -use rustc_ast::{LitFloatType, LitIntType, LitKind}; +use rustc_ast::LitKind; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind, GenericArg, Lit, MutTy, Mutability, TyKind, UnOp}; +use rustc_hir::{Expr, ExprKind, GenericArg, MutTy, Mutability, TyKind, UnOp}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; -use rustc_middle::ty::{self, FloatTy, InferTy, Ty, TypeAndMut, UintTy}; +use rustc_middle::ty::{self, Ty, TypeAndMut, UintTy}; use rustc_semver::RustcVersion; use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; use rustc_span::symbol::sym; @@ -21,8 +22,7 @@ use rustc_target::abi::LayoutOf; use crate::utils::sugg::Sugg; use crate::utils::{ - is_hir_ty_cfg_dependant, meets_msrv, numeric_literal::NumericLiteral, snippet_opt, snippet_with_applicability, - span_lint, span_lint_and_sugg, span_lint_and_then, + is_hir_ty_cfg_dependant, meets_msrv, snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then, }; use utils::int_ty_to_nbits; @@ -284,72 +284,25 @@ fn is_c_void(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { false } -/// Returns the mantissa bits wide of a fp type. -/// Will return 0 if the type is not a fp -fn fp_ty_mantissa_nbits(typ: Ty<'_>) -> u32 { - match typ.kind() { - ty::Float(FloatTy::F32) => 23, - ty::Float(FloatTy::F64) | ty::Infer(InferTy::FloatVar(_)) => 52, - _ => 0, - } -} - impl<'tcx> LateLintPass<'tcx> for Casts { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if expr.span.from_expansion() { return; } - if let ExprKind::Cast(ref ex, cast_to) = expr.kind { + if let ExprKind::Cast(ref cast_expr, cast_to) = expr.kind { if is_hir_ty_cfg_dependant(cx, cast_to) { return; } - let (cast_from, cast_to) = (cx.typeck_results().expr_ty(ex), cx.typeck_results().expr_ty(expr)); - lint_fn_to_numeric_cast(cx, expr, ex, cast_from, cast_to); - if let Some(lit) = get_numeric_literal(ex) { - let literal_str = snippet_opt(cx, ex.span).unwrap_or_default(); - - if_chain! { - if let LitKind::Int(n, _) = lit.node; - if let Some(src) = snippet_opt(cx, lit.span); - if cast_to.is_floating_point(); - if let Some(num_lit) = NumericLiteral::from_lit_kind(&src, &lit.node); - let from_nbits = 128 - n.leading_zeros(); - let to_nbits = fp_ty_mantissa_nbits(cast_to); - if from_nbits != 0 && to_nbits != 0 && from_nbits <= to_nbits && num_lit.is_decimal(); - then { - let literal_str = if is_unary_neg(ex) { format!("-{}", num_lit.integer) } else { num_lit.integer.into() }; - show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); - return; - } - } - - match lit.node { - LitKind::Int(_, LitIntType::Unsuffixed) if cast_to.is_integral() => { - show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); - }, - LitKind::Float(_, LitFloatType::Unsuffixed) if cast_to.is_floating_point() => { - show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); - }, - LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed) => {}, - _ => { - if cast_from.kind() == cast_to.kind() && !in_external_macro(cx.sess(), expr.span) { - span_lint( - cx, - UNNECESSARY_CAST, - expr.span, - &format!( - "casting to the same type is unnecessary (`{}` -> `{}`)", - cast_from, cast_to - ), - ); - } - }, - } - } - if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) { - lint_numeric_casts(cx, expr, ex, cast_from, cast_to); - } + let (cast_from, cast_to) = ( + cx.typeck_results().expr_ty(cast_expr), + cx.typeck_results().expr_ty(expr), + ); + if unnecessary_cast::check(cx, expr, cast_expr, cast_from, cast_to) { + return; + } + lint_fn_to_numeric_cast(cx, expr, cast_expr, cast_from, cast_to); + lint_numeric_casts(cx, expr, cast_expr, cast_from, cast_to); lint_cast_ptr_alignment(cx, expr, cast_from, cast_to); } else if let ExprKind::MethodCall(method_path, _, args, _) = expr.kind { if_chain! { @@ -368,37 +321,6 @@ impl<'tcx> LateLintPass<'tcx> for Casts { } } -fn is_unary_neg(expr: &Expr<'_>) -> bool { - matches!(expr.kind, ExprKind::Unary(UnOp::Neg, _)) -} - -fn get_numeric_literal<'e>(expr: &'e Expr<'e>) -> Option<&'e Lit> { - match expr.kind { - ExprKind::Lit(ref lit) => Some(lit), - ExprKind::Unary(UnOp::Neg, e) => { - if let ExprKind::Lit(ref lit) = e.kind { - Some(lit) - } else { - None - } - }, - _ => None, - } -} - -fn show_unnecessary_cast(cx: &LateContext<'_>, expr: &Expr<'_>, literal_str: &str, cast_from: Ty<'_>, cast_to: Ty<'_>) { - let literal_kind_name = if cast_from.is_integral() { "integer" } else { "float" }; - span_lint_and_sugg( - cx, - UNNECESSARY_CAST, - expr.span, - &format!("casting {} literal to `{}` is unnecessary", literal_kind_name, cast_to), - "try", - format!("{}_{}", literal_str.trim_end_matches('.'), cast_to), - Applicability::MachineApplicable, - ); -} - fn lint_numeric_casts<'tcx>( cx: &LateContext<'tcx>, expr: &Expr<'tcx>, @@ -406,11 +328,13 @@ fn lint_numeric_casts<'tcx>( cast_from: Ty<'tcx>, cast_to: Ty<'tcx>, ) { - cast_possible_truncation::check(cx, expr, cast_from, cast_to); - cast_possible_wrap::check(cx, expr, cast_from, cast_to); - cast_precision_loss::check(cx, expr, cast_from, cast_to); - cast_lossless::check(cx, expr, cast_op, cast_from, cast_to); - cast_sign_loss::check(cx, expr, cast_op, cast_from, cast_to); + if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) { + cast_possible_truncation::check(cx, expr, cast_from, cast_to); + cast_possible_wrap::check(cx, expr, cast_from, cast_to); + cast_precision_loss::check(cx, expr, cast_from, cast_to); + cast_lossless::check(cx, expr, cast_op, cast_from, cast_to); + cast_sign_loss::check(cx, expr, cast_op, cast_from, cast_to); + } } fn lint_cast_ptr_alignment<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, cast_from: Ty<'tcx>, cast_to: Ty<'tcx>) { diff --git a/clippy_lints/src/casts/unnecessary_cast.rs b/clippy_lints/src/casts/unnecessary_cast.rs new file mode 100644 index 000000000000..fa2a07ef1da0 --- /dev/null +++ b/clippy_lints/src/casts/unnecessary_cast.rs @@ -0,0 +1,106 @@ +use rustc_ast::{LitFloatType, LitIntType, LitKind}; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, Lit, UnOp}; +use rustc_lint::{LateContext, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_middle::ty::{self, FloatTy, InferTy, Ty}; + +use if_chain::if_chain; + +use crate::utils::{numeric_literal::NumericLiteral, snippet_opt, span_lint, span_lint_and_sugg}; + +use super::UNNECESSARY_CAST; + +pub(super) fn check( + cx: &LateContext<'_>, + expr: &Expr<'_>, + cast_expr: &Expr<'_>, + cast_from: Ty<'_>, + cast_to: Ty<'_>, +) -> bool { + if let Some(lit) = get_numeric_literal(cast_expr) { + let literal_str = snippet_opt(cx, cast_expr.span).unwrap_or_default(); + + if_chain! { + if let LitKind::Int(n, _) = lit.node; + if let Some(src) = snippet_opt(cx, lit.span); + if cast_to.is_floating_point(); + if let Some(num_lit) = NumericLiteral::from_lit_kind(&src, &lit.node); + let from_nbits = 128 - n.leading_zeros(); + let to_nbits = fp_ty_mantissa_nbits(cast_to); + if from_nbits != 0 && to_nbits != 0 && from_nbits <= to_nbits && num_lit.is_decimal(); + then { + let literal_str = if is_unary_neg(cast_expr) { format!("-{}", num_lit.integer) } else { num_lit.integer.into() }; + lint_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); + return true + } + } + + match lit.node { + LitKind::Int(_, LitIntType::Unsuffixed) if cast_to.is_integral() => { + lint_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); + }, + LitKind::Float(_, LitFloatType::Unsuffixed) if cast_to.is_floating_point() => { + lint_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); + }, + LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed) => {}, + _ => { + if cast_from.kind() == cast_to.kind() && !in_external_macro(cx.sess(), expr.span) { + span_lint( + cx, + UNNECESSARY_CAST, + expr.span, + &format!( + "casting to the same type is unnecessary (`{}` -> `{}`)", + cast_from, cast_to + ), + ); + return true; + } + }, + } + } + + false +} + +fn lint_unnecessary_cast(cx: &LateContext<'_>, expr: &Expr<'_>, literal_str: &str, cast_from: Ty<'_>, cast_to: Ty<'_>) { + let literal_kind_name = if cast_from.is_integral() { "integer" } else { "float" }; + span_lint_and_sugg( + cx, + UNNECESSARY_CAST, + expr.span, + &format!("casting {} literal to `{}` is unnecessary", literal_kind_name, cast_to), + "try", + format!("{}_{}", literal_str.trim_end_matches('.'), cast_to), + Applicability::MachineApplicable, + ); +} + +fn get_numeric_literal<'e>(expr: &'e Expr<'e>) -> Option<&'e Lit> { + match expr.kind { + ExprKind::Lit(ref lit) => Some(lit), + ExprKind::Unary(UnOp::Neg, e) => { + if let ExprKind::Lit(ref lit) = e.kind { + Some(lit) + } else { + None + } + }, + _ => None, + } +} + +/// Returns the mantissa bits wide of a fp type. +/// Will return 0 if the type is not a fp +fn fp_ty_mantissa_nbits(typ: Ty<'_>) -> u32 { + match typ.kind() { + ty::Float(FloatTy::F32) => 23, + ty::Float(FloatTy::F64) | ty::Infer(InferTy::FloatVar(_)) => 52, + _ => 0, + } +} + +fn is_unary_neg(expr: &Expr<'_>) -> bool { + matches!(expr.kind, ExprKind::Unary(UnOp::Neg, _)) +} From 0dce9921fd6a1e55430c56e5ccf0faf0f1e23491 Mon Sep 17 00:00:00 2001 From: Yoshitomo Nakanishi Date: Tue, 9 Mar 2021 16:47:46 +0900 Subject: [PATCH 09/15] Remove 'lint_numeric_casts' --- clippy_lints/src/casts/mod.rs | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index d8c744365014..c7e9066f3eaa 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -302,8 +302,14 @@ impl<'tcx> LateLintPass<'tcx> for Casts { return; } lint_fn_to_numeric_cast(cx, expr, cast_expr, cast_from, cast_to); - lint_numeric_casts(cx, expr, cast_expr, cast_from, cast_to); lint_cast_ptr_alignment(cx, expr, cast_from, cast_to); + if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) { + cast_possible_truncation::check(cx, expr, cast_from, cast_to); + cast_possible_wrap::check(cx, expr, cast_from, cast_to); + cast_precision_loss::check(cx, expr, cast_from, cast_to); + cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to); + cast_sign_loss::check(cx, expr, cast_expr, cast_from, cast_to); + } } else if let ExprKind::MethodCall(method_path, _, args, _) = expr.kind { if_chain! { if method_path.ident.name == sym!(cast); @@ -321,22 +327,6 @@ impl<'tcx> LateLintPass<'tcx> for Casts { } } -fn lint_numeric_casts<'tcx>( - cx: &LateContext<'tcx>, - expr: &Expr<'tcx>, - cast_op: &Expr<'_>, - cast_from: Ty<'tcx>, - cast_to: Ty<'tcx>, -) { - if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) { - cast_possible_truncation::check(cx, expr, cast_from, cast_to); - cast_possible_wrap::check(cx, expr, cast_from, cast_to); - cast_precision_loss::check(cx, expr, cast_from, cast_to); - cast_lossless::check(cx, expr, cast_op, cast_from, cast_to); - cast_sign_loss::check(cx, expr, cast_op, cast_from, cast_to); - } -} - fn lint_cast_ptr_alignment<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, cast_from: Ty<'tcx>, cast_to: Ty<'tcx>) { if_chain! { if let ty::RawPtr(from_ptr_ty) = &cast_from.kind(); From 458f6d19d0c51ee97b7ac90ce60994f627983824 Mon Sep 17 00:00:00 2001 From: Yoshitomo Nakanishi Date: Tue, 9 Mar 2021 17:03:07 +0900 Subject: [PATCH 10/15] Move fn_to_numeric_cast to its own module --- clippy_lints/src/casts/fn_to_numeric_cast.rs | 37 ++++++++++++++++++++ clippy_lints/src/casts/mod.rs | 13 ++----- 2 files changed, 40 insertions(+), 10 deletions(-) create mode 100644 clippy_lints/src/casts/fn_to_numeric_cast.rs diff --git a/clippy_lints/src/casts/fn_to_numeric_cast.rs b/clippy_lints/src/casts/fn_to_numeric_cast.rs new file mode 100644 index 000000000000..a8d508585b5d --- /dev/null +++ b/clippy_lints/src/casts/fn_to_numeric_cast.rs @@ -0,0 +1,37 @@ +use rustc_errors::Applicability; +use rustc_hir::Expr; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, Ty, UintTy}; + +use crate::utils::{snippet_with_applicability, span_lint_and_sugg}; + +use super::{utils, FN_TO_NUMERIC_CAST}; + +pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { + // We only want to check casts to `ty::Uint` or `ty::Int` + match cast_to.kind() { + ty::Uint(_) | ty::Int(..) => { /* continue on */ }, + _ => return, + } + + match cast_from.kind() { + ty::FnDef(..) | ty::FnPtr(_) => { + let mut applicability = Applicability::MaybeIncorrect; + let from_snippet = snippet_with_applicability(cx, cast_expr.span, "x", &mut applicability); + let to_nbits = utils::int_ty_to_nbits(cast_to, cx.tcx); + + if (to_nbits >= cx.tcx.data_layout.pointer_size.bits()) && (*cast_to.kind() != ty::Uint(UintTy::Usize)) { + span_lint_and_sugg( + cx, + FN_TO_NUMERIC_CAST, + expr.span, + &format!("casting function pointer `{}` to `{}`", from_snippet, cast_to), + "try", + format!("{} as usize", from_snippet), + applicability, + ); + } + }, + _ => {}, + } +} diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index c7e9066f3eaa..bbe492be72ec 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -3,6 +3,7 @@ mod cast_possible_truncation; mod cast_possible_wrap; mod cast_precision_loss; mod cast_sign_loss; +mod fn_to_numeric_cast; mod unnecessary_cast; mod utils; @@ -301,6 +302,8 @@ impl<'tcx> LateLintPass<'tcx> for Casts { if unnecessary_cast::check(cx, expr, cast_expr, cast_from, cast_to) { return; } + + fn_to_numeric_cast::check(cx, expr, cast_expr, cast_from, cast_to); lint_fn_to_numeric_cast(cx, expr, cast_expr, cast_from, cast_to); lint_cast_ptr_alignment(cx, expr, cast_from, cast_to); if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) { @@ -386,16 +389,6 @@ fn lint_fn_to_numeric_cast( format!("{} as usize", from_snippet), applicability, ); - } else if *cast_to.kind() != ty::Uint(UintTy::Usize) { - span_lint_and_sugg( - cx, - FN_TO_NUMERIC_CAST, - expr.span, - &format!("casting function pointer `{}` to `{}`", from_snippet, cast_to), - "try", - format!("{} as usize", from_snippet), - applicability, - ); } }, _ => {}, From 9a24877e2cd68f22edd3de4a44f59c9899953f91 Mon Sep 17 00:00:00 2001 From: Yoshitomo Nakanishi Date: Tue, 9 Mar 2021 17:06:25 +0900 Subject: [PATCH 11/15] Move fn_to_numeric_cast_with_truncation to its own module --- .../fn_to_numeric_cast_with_truncation.rs | 39 ++++++++++++++++++ clippy_lints/src/casts/mod.rs | 40 +------------------ 2 files changed, 41 insertions(+), 38 deletions(-) create mode 100644 clippy_lints/src/casts/fn_to_numeric_cast_with_truncation.rs diff --git a/clippy_lints/src/casts/fn_to_numeric_cast_with_truncation.rs b/clippy_lints/src/casts/fn_to_numeric_cast_with_truncation.rs new file mode 100644 index 000000000000..0085c7b27b29 --- /dev/null +++ b/clippy_lints/src/casts/fn_to_numeric_cast_with_truncation.rs @@ -0,0 +1,39 @@ +use rustc_errors::Applicability; +use rustc_hir::Expr; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, Ty}; + +use crate::utils::{snippet_with_applicability, span_lint_and_sugg}; + +use super::{utils, FN_TO_NUMERIC_CAST_WITH_TRUNCATION}; + +pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { + // We only want to check casts to `ty::Uint` or `ty::Int` + match cast_to.kind() { + ty::Uint(_) | ty::Int(..) => { /* continue on */ }, + _ => return, + } + match cast_from.kind() { + ty::FnDef(..) | ty::FnPtr(_) => { + let mut applicability = Applicability::MaybeIncorrect; + let from_snippet = snippet_with_applicability(cx, cast_expr.span, "x", &mut applicability); + + let to_nbits = utils::int_ty_to_nbits(cast_to, cx.tcx); + if to_nbits < cx.tcx.data_layout.pointer_size.bits() { + span_lint_and_sugg( + cx, + FN_TO_NUMERIC_CAST_WITH_TRUNCATION, + expr.span, + &format!( + "casting function pointer `{}` to `{}`, which truncates the value", + from_snippet, cast_to + ), + "try", + format!("{} as usize", from_snippet), + applicability, + ); + } + }, + _ => {}, + } +} diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index bbe492be72ec..a80de01cfaf3 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -4,6 +4,7 @@ mod cast_possible_wrap; mod cast_precision_loss; mod cast_sign_loss; mod fn_to_numeric_cast; +mod fn_to_numeric_cast_with_truncation; mod unnecessary_cast; mod utils; @@ -304,7 +305,7 @@ impl<'tcx> LateLintPass<'tcx> for Casts { } fn_to_numeric_cast::check(cx, expr, cast_expr, cast_from, cast_to); - lint_fn_to_numeric_cast(cx, expr, cast_expr, cast_from, cast_to); + fn_to_numeric_cast_with_truncation::check(cx, expr, cast_expr, cast_from, cast_to); lint_cast_ptr_alignment(cx, expr, cast_from, cast_to); if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) { cast_possible_truncation::check(cx, expr, cast_from, cast_to); @@ -358,43 +359,6 @@ fn lint_cast_ptr_alignment<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, cast_f } } -fn lint_fn_to_numeric_cast( - cx: &LateContext<'_>, - expr: &Expr<'_>, - cast_expr: &Expr<'_>, - cast_from: Ty<'_>, - cast_to: Ty<'_>, -) { - // We only want to check casts to `ty::Uint` or `ty::Int` - match cast_to.kind() { - ty::Uint(_) | ty::Int(..) => { /* continue on */ }, - _ => return, - } - match cast_from.kind() { - ty::FnDef(..) | ty::FnPtr(_) => { - let mut applicability = Applicability::MaybeIncorrect; - let from_snippet = snippet_with_applicability(cx, cast_expr.span, "x", &mut applicability); - - let to_nbits = int_ty_to_nbits(cast_to, cx.tcx); - if to_nbits < cx.tcx.data_layout.pointer_size.bits() { - span_lint_and_sugg( - cx, - FN_TO_NUMERIC_CAST_WITH_TRUNCATION, - expr.span, - &format!( - "casting function pointer `{}` to `{}`, which truncates the value", - from_snippet, cast_to - ), - "try", - format!("{} as usize", from_snippet), - applicability, - ); - } - }, - _ => {}, - } -} - declare_clippy_lint! { /// **What it does:** Checks for casts of `&T` to `&mut T` anywhere in the code. /// From ebd1ec0e40739d8b968c2b9bfd249485cec866ab Mon Sep 17 00:00:00 2001 From: Yoshitomo Nakanishi Date: Tue, 9 Mar 2021 17:15:18 +0900 Subject: [PATCH 12/15] Move cast_ptr_alignment to its own module --- clippy_lints/src/casts/cast_ptr_alignment.rs | 81 ++++++++++++++++++++ clippy_lints/src/casts/mod.rs | 67 +--------------- 2 files changed, 85 insertions(+), 63 deletions(-) create mode 100644 clippy_lints/src/casts/cast_ptr_alignment.rs diff --git a/clippy_lints/src/casts/cast_ptr_alignment.rs b/clippy_lints/src/casts/cast_ptr_alignment.rs new file mode 100644 index 000000000000..87fb5557be06 --- /dev/null +++ b/clippy_lints/src/casts/cast_ptr_alignment.rs @@ -0,0 +1,81 @@ +use rustc_hir::{Expr, ExprKind, GenericArg}; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, Ty}; +use rustc_span::symbol::sym; +use rustc_target::abi::LayoutOf; + +use if_chain::if_chain; + +use crate::utils::{is_hir_ty_cfg_dependant, span_lint}; + +use super::CAST_PTR_ALIGNMENT; + +pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if let ExprKind::Cast(ref cast_expr, cast_to) = expr.kind { + if is_hir_ty_cfg_dependant(cx, cast_to) { + return; + } + let (cast_from, cast_to) = ( + cx.typeck_results().expr_ty(cast_expr), + cx.typeck_results().expr_ty(expr), + ); + lint_cast_ptr_alignment(cx, expr, cast_from, cast_to); + } else if let ExprKind::MethodCall(method_path, _, args, _) = expr.kind { + if_chain! { + if method_path.ident.name == sym!(cast); + if let Some(generic_args) = method_path.args; + if let [GenericArg::Type(cast_to)] = generic_args.args; + // There probably is no obvious reason to do this, just to be consistent with `as` cases. + if !is_hir_ty_cfg_dependant(cx, cast_to); + then { + let (cast_from, cast_to) = + (cx.typeck_results().expr_ty(&args[0]), cx.typeck_results().expr_ty(expr)); + lint_cast_ptr_alignment(cx, expr, cast_from, cast_to); + } + } + } +} + +fn lint_cast_ptr_alignment<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, cast_from: Ty<'tcx>, cast_to: Ty<'tcx>) { + if_chain! { + if let ty::RawPtr(from_ptr_ty) = &cast_from.kind(); + if let ty::RawPtr(to_ptr_ty) = &cast_to.kind(); + if let Ok(from_layout) = cx.layout_of(from_ptr_ty.ty); + if let Ok(to_layout) = cx.layout_of(to_ptr_ty.ty); + if from_layout.align.abi < to_layout.align.abi; + // with c_void, we inherently need to trust the user + if !is_c_void(cx, from_ptr_ty.ty); + // when casting from a ZST, we don't know enough to properly lint + if !from_layout.is_zst(); + then { + span_lint( + cx, + CAST_PTR_ALIGNMENT, + expr.span, + &format!( + "casting from `{}` to a more-strictly-aligned pointer (`{}`) ({} < {} bytes)", + cast_from, + cast_to, + from_layout.align.abi.bytes(), + to_layout.align.abi.bytes(), + ), + ); + } + } +} + +/// Check if the given type is either `core::ffi::c_void` or +/// one of the platform specific `libc::::c_void` of libc. +fn is_c_void(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { + if let ty::Adt(adt, _) = ty.kind() { + let names = cx.get_def_path(adt.did); + + if names.is_empty() { + return false; + } + if names[0] == sym::libc || names[0] == sym::core && *names.last().unwrap() == sym!(c_void) { + return true; + } + } + false +} diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index a80de01cfaf3..cf8a64e1fd84 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -2,6 +2,7 @@ mod cast_lossless; mod cast_possible_truncation; mod cast_possible_wrap; mod cast_precision_loss; +mod cast_ptr_alignment; mod cast_sign_loss; mod fn_to_numeric_cast; mod fn_to_numeric_cast_with_truncation; @@ -13,22 +14,18 @@ use std::borrow::Cow; use if_chain::if_chain; use rustc_ast::LitKind; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind, GenericArg, MutTy, Mutability, TyKind, UnOp}; +use rustc_hir::{Expr, ExprKind, MutTy, Mutability, TyKind, UnOp}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; -use rustc_middle::ty::{self, Ty, TypeAndMut, UintTy}; +use rustc_middle::ty::{self, TypeAndMut, UintTy}; use rustc_semver::RustcVersion; use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; -use rustc_span::symbol::sym; -use rustc_target::abi::LayoutOf; use crate::utils::sugg::Sugg; use crate::utils::{ is_hir_ty_cfg_dependant, meets_msrv, snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then, }; -use utils::int_ty_to_nbits; - declare_clippy_lint! { /// **What it does:** Checks for casts from any numerical to a float type where /// the receiving type cannot store all values from the original type without @@ -270,22 +267,6 @@ declare_lint_pass!(Casts => [ FN_TO_NUMERIC_CAST_WITH_TRUNCATION, ]); -/// Check if the given type is either `core::ffi::c_void` or -/// one of the platform specific `libc::::c_void` of libc. -fn is_c_void(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { - if let ty::Adt(adt, _) = ty.kind() { - let names = cx.get_def_path(adt.did); - - if names.is_empty() { - return false; - } - if names[0] == sym::libc || names[0] == sym::core && *names.last().unwrap() == sym!(c_void) { - return true; - } - } - false -} - impl<'tcx> LateLintPass<'tcx> for Casts { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if expr.span.from_expansion() { @@ -306,7 +287,6 @@ impl<'tcx> LateLintPass<'tcx> for Casts { fn_to_numeric_cast::check(cx, expr, cast_expr, cast_from, cast_to); fn_to_numeric_cast_with_truncation::check(cx, expr, cast_expr, cast_from, cast_to); - lint_cast_ptr_alignment(cx, expr, cast_from, cast_to); if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) { cast_possible_truncation::check(cx, expr, cast_from, cast_to); cast_possible_wrap::check(cx, expr, cast_from, cast_to); @@ -314,48 +294,9 @@ impl<'tcx> LateLintPass<'tcx> for Casts { cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to); cast_sign_loss::check(cx, expr, cast_expr, cast_from, cast_to); } - } else if let ExprKind::MethodCall(method_path, _, args, _) = expr.kind { - if_chain! { - if method_path.ident.name == sym!(cast); - if let Some(generic_args) = method_path.args; - if let [GenericArg::Type(cast_to)] = generic_args.args; - // There probably is no obvious reason to do this, just to be consistent with `as` cases. - if !is_hir_ty_cfg_dependant(cx, cast_to); - then { - let (cast_from, cast_to) = - (cx.typeck_results().expr_ty(&args[0]), cx.typeck_results().expr_ty(expr)); - lint_cast_ptr_alignment(cx, expr, cast_from, cast_to); - } - } } - } -} -fn lint_cast_ptr_alignment<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, cast_from: Ty<'tcx>, cast_to: Ty<'tcx>) { - if_chain! { - if let ty::RawPtr(from_ptr_ty) = &cast_from.kind(); - if let ty::RawPtr(to_ptr_ty) = &cast_to.kind(); - if let Ok(from_layout) = cx.layout_of(from_ptr_ty.ty); - if let Ok(to_layout) = cx.layout_of(to_ptr_ty.ty); - if from_layout.align.abi < to_layout.align.abi; - // with c_void, we inherently need to trust the user - if !is_c_void(cx, from_ptr_ty.ty); - // when casting from a ZST, we don't know enough to properly lint - if !from_layout.is_zst(); - then { - span_lint( - cx, - CAST_PTR_ALIGNMENT, - expr.span, - &format!( - "casting from `{}` to a more-strictly-aligned pointer (`{}`) ({} < {} bytes)", - cast_from, - cast_to, - from_layout.align.abi.bytes(), - to_layout.align.abi.bytes(), - ), - ); - } + cast_ptr_alignment::check(cx, expr); } } From f33bb3d900f93b13f4a9060e0060df36ff3a7e3d Mon Sep 17 00:00:00 2001 From: Yoshitomo Nakanishi Date: Tue, 9 Mar 2021 17:22:21 +0900 Subject: [PATCH 13/15] Move cast_ref_to_mut to its own module --- clippy_lints/src/casts/cast_ref_to_mut.rs | 28 +++++++ clippy_lints/src/casts/mod.rs | 99 +++++++++-------------- clippy_lints/src/lib.rs | 1 - 3 files changed, 68 insertions(+), 60 deletions(-) create mode 100644 clippy_lints/src/casts/cast_ref_to_mut.rs diff --git a/clippy_lints/src/casts/cast_ref_to_mut.rs b/clippy_lints/src/casts/cast_ref_to_mut.rs new file mode 100644 index 000000000000..3fdc1c6168ba --- /dev/null +++ b/clippy_lints/src/casts/cast_ref_to_mut.rs @@ -0,0 +1,28 @@ +use rustc_hir::{Expr, ExprKind, MutTy, Mutability, TyKind, UnOp}; +use rustc_lint::LateContext; +use rustc_middle::ty; + +use if_chain::if_chain; + +use crate::utils::span_lint; + +use super::CAST_REF_TO_MUT; + +pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if_chain! { + if let ExprKind::Unary(UnOp::Deref, e) = &expr.kind; + if let ExprKind::Cast(e, t) = &e.kind; + if let TyKind::Ptr(MutTy { mutbl: Mutability::Mut, .. }) = t.kind; + if let ExprKind::Cast(e, t) = &e.kind; + if let TyKind::Ptr(MutTy { mutbl: Mutability::Not, .. }) = t.kind; + if let ty::Ref(..) = cx.typeck_results().node_type(e.hir_id).kind(); + then { + span_lint( + cx, + CAST_REF_TO_MUT, + expr.span, + "casting `&T` to `&mut T` may cause undefined behavior, consider instead using an `UnsafeCell`", + ); + } + } +} diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index cf8a64e1fd84..7d5ada18be3b 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -3,6 +3,7 @@ mod cast_possible_truncation; mod cast_possible_wrap; mod cast_precision_loss; mod cast_ptr_alignment; +mod cast_ref_to_mut; mod cast_sign_loss; mod fn_to_numeric_cast; mod fn_to_numeric_cast_with_truncation; @@ -14,7 +15,7 @@ use std::borrow::Cow; use if_chain::if_chain; use rustc_ast::LitKind; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind, MutTy, Mutability, TyKind, UnOp}; +use rustc_hir::{Expr, ExprKind, Mutability, TyKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::{self, TypeAndMut, UintTy}; @@ -23,7 +24,7 @@ use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; use crate::utils::sugg::Sugg; use crate::utils::{ - is_hir_ty_cfg_dependant, meets_msrv, snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then, + is_hir_ty_cfg_dependant, meets_msrv, snippet_with_applicability, span_lint_and_sugg, span_lint_and_then, }; declare_clippy_lint! { @@ -255,12 +256,47 @@ declare_clippy_lint! { "casting a function pointer to a numeric type not wide enough to store the address" } +declare_clippy_lint! { + /// **What it does:** Checks for casts of `&T` to `&mut T` anywhere in the code. + /// + /// **Why is this bad?** It’s basically guaranteed to be undefined behaviour. + /// `UnsafeCell` is the only way to obtain aliasable data that is considered + /// mutable. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust,ignore + /// fn x(r: &i32) { + /// unsafe { + /// *(r as *const _ as *mut _) += 1; + /// } + /// } + /// ``` + /// + /// Instead consider using interior mutability types. + /// + /// ```rust + /// use std::cell::UnsafeCell; + /// + /// fn x(r: &UnsafeCell) { + /// unsafe { + /// *r.get() += 1; + /// } + /// } + /// ``` + pub CAST_REF_TO_MUT, + correctness, + "a cast of reference to a mutable pointer" +} + declare_lint_pass!(Casts => [ CAST_PRECISION_LOSS, CAST_SIGN_LOSS, CAST_POSSIBLE_TRUNCATION, CAST_POSSIBLE_WRAP, CAST_LOSSLESS, + CAST_REF_TO_MUT, UNNECESSARY_CAST, CAST_PTR_ALIGNMENT, FN_TO_NUMERIC_CAST, @@ -269,6 +305,8 @@ declare_lint_pass!(Casts => [ impl<'tcx> LateLintPass<'tcx> for Casts { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + cast_ref_to_mut::check(cx, expr); + if expr.span.from_expansion() { return; } @@ -300,63 +338,6 @@ impl<'tcx> LateLintPass<'tcx> for Casts { } } -declare_clippy_lint! { - /// **What it does:** Checks for casts of `&T` to `&mut T` anywhere in the code. - /// - /// **Why is this bad?** It’s basically guaranteed to be undefined behaviour. - /// `UnsafeCell` is the only way to obtain aliasable data that is considered - /// mutable. - /// - /// **Known problems:** None. - /// - /// **Example:** - /// ```rust,ignore - /// fn x(r: &i32) { - /// unsafe { - /// *(r as *const _ as *mut _) += 1; - /// } - /// } - /// ``` - /// - /// Instead consider using interior mutability types. - /// - /// ```rust - /// use std::cell::UnsafeCell; - /// - /// fn x(r: &UnsafeCell) { - /// unsafe { - /// *r.get() += 1; - /// } - /// } - /// ``` - pub CAST_REF_TO_MUT, - correctness, - "a cast of reference to a mutable pointer" -} - -declare_lint_pass!(RefToMut => [CAST_REF_TO_MUT]); - -impl<'tcx> LateLintPass<'tcx> for RefToMut { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if_chain! { - if let ExprKind::Unary(UnOp::Deref, e) = &expr.kind; - if let ExprKind::Cast(e, t) = &e.kind; - if let TyKind::Ptr(MutTy { mutbl: Mutability::Mut, .. }) = t.kind; - if let ExprKind::Cast(e, t) = &e.kind; - if let TyKind::Ptr(MutTy { mutbl: Mutability::Not, .. }) = t.kind; - if let ty::Ref(..) = cx.typeck_results().node_type(e.hir_id).kind(); - then { - span_lint( - cx, - CAST_REF_TO_MUT, - expr.span, - "casting `&T` to `&mut T` may cause undefined behavior, consider instead using an `UnsafeCell`", - ); - } - } - } -} - const PTR_AS_PTR_MSRV: RustcVersion = RustcVersion::new(1, 38, 0); declare_clippy_lint! { diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index a46c56a01d5d..47afd7beb741 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1176,7 +1176,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box slow_vector_initialization::SlowVectorInit); store.register_late_pass(|| box unnecessary_sort_by::UnnecessarySortBy); store.register_late_pass(|| box unnecessary_wraps::UnnecessaryWraps); - store.register_late_pass(|| box casts::RefToMut); store.register_late_pass(|| box assertions_on_constants::AssertionsOnConstants); store.register_late_pass(|| box transmuting_null::TransmutingNull); store.register_late_pass(|| box path_buf_push_overwrite::PathBufPushOverwrite); From 43c88c60c152f55fb1ada9438ed2378c358b329a Mon Sep 17 00:00:00 2001 From: Yoshitomo Nakanishi Date: Tue, 9 Mar 2021 17:28:56 +0900 Subject: [PATCH 14/15] Move char_lit_as_u8 to its own module --- clippy_lints/src/casts/char_lit_as_u8.rs | 42 ++++++++++ clippy_lints/src/casts/mod.rs | 102 ++++++++--------------- clippy_lints/src/lib.rs | 1 - 3 files changed, 75 insertions(+), 70 deletions(-) create mode 100644 clippy_lints/src/casts/char_lit_as_u8.rs diff --git a/clippy_lints/src/casts/char_lit_as_u8.rs b/clippy_lints/src/casts/char_lit_as_u8.rs new file mode 100644 index 000000000000..ccaad1b8f2ac --- /dev/null +++ b/clippy_lints/src/casts/char_lit_as_u8.rs @@ -0,0 +1,42 @@ +use rustc_ast::LitKind; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, UintTy}; + +use if_chain::if_chain; + +use crate::utils::{snippet_with_applicability, span_lint_and_then}; + +use super::CHAR_LIT_AS_U8; + +pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if_chain! { + if let ExprKind::Cast(e, _) = &expr.kind; + if let ExprKind::Lit(l) = &e.kind; + if let LitKind::Char(c) = l.node; + if ty::Uint(UintTy::U8) == *cx.typeck_results().expr_ty(expr).kind(); + then { + let mut applicability = Applicability::MachineApplicable; + let snippet = snippet_with_applicability(cx, e.span, "'x'", &mut applicability); + + span_lint_and_then( + cx, + CHAR_LIT_AS_U8, + expr.span, + "casting a character literal to `u8` truncates", + |diag| { + diag.note("`char` is four bytes wide, but `u8` is a single byte"); + + if c.is_ascii() { + diag.span_suggestion( + expr.span, + "use a byte literal instead", + format!("b{}", snippet), + applicability, + ); + } + }); + } + } +} diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index 7d5ada18be3b..6d49af21a5f0 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -5,6 +5,7 @@ mod cast_precision_loss; mod cast_ptr_alignment; mod cast_ref_to_mut; mod cast_sign_loss; +mod char_lit_as_u8; mod fn_to_numeric_cast; mod fn_to_numeric_cast_with_truncation; mod unnecessary_cast; @@ -13,19 +14,16 @@ mod utils; use std::borrow::Cow; use if_chain::if_chain; -use rustc_ast::LitKind; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, Mutability, TyKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; -use rustc_middle::ty::{self, TypeAndMut, UintTy}; +use rustc_middle::ty::{self, TypeAndMut}; use rustc_semver::RustcVersion; use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; use crate::utils::sugg::Sugg; -use crate::utils::{ - is_hir_ty_cfg_dependant, meets_msrv, snippet_with_applicability, span_lint_and_sugg, span_lint_and_then, -}; +use crate::utils::{is_hir_ty_cfg_dependant, meets_msrv, span_lint_and_sugg}; declare_clippy_lint! { /// **What it does:** Checks for casts from any numerical to a float type where @@ -290,6 +288,33 @@ declare_clippy_lint! { "a cast of reference to a mutable pointer" } +declare_clippy_lint! { + /// **What it does:** Checks for expressions where a character literal is cast + /// to `u8` and suggests using a byte literal instead. + /// + /// **Why is this bad?** In general, casting values to smaller types is + /// error-prone and should be avoided where possible. In the particular case of + /// converting a character literal to u8, it is easy to avoid by just using a + /// byte literal instead. As an added bonus, `b'a'` is even slightly shorter + /// than `'a' as u8`. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust,ignore + /// 'x' as u8 + /// ``` + /// + /// A better version, using the byte literal: + /// + /// ```rust,ignore + /// b'x' + /// ``` + pub CHAR_LIT_AS_U8, + complexity, + "casting a character literal to `u8` truncates" +} + declare_lint_pass!(Casts => [ CAST_PRECISION_LOSS, CAST_SIGN_LOSS, @@ -297,10 +322,11 @@ declare_lint_pass!(Casts => [ CAST_POSSIBLE_WRAP, CAST_LOSSLESS, CAST_REF_TO_MUT, - UNNECESSARY_CAST, CAST_PTR_ALIGNMENT, + UNNECESSARY_CAST, FN_TO_NUMERIC_CAST, FN_TO_NUMERIC_CAST_WITH_TRUNCATION, + CHAR_LIT_AS_U8, ]); impl<'tcx> LateLintPass<'tcx> for Casts { @@ -335,74 +361,12 @@ impl<'tcx> LateLintPass<'tcx> for Casts { } cast_ptr_alignment::check(cx, expr); + char_lit_as_u8::check(cx, expr); } } const PTR_AS_PTR_MSRV: RustcVersion = RustcVersion::new(1, 38, 0); -declare_clippy_lint! { - /// **What it does:** Checks for expressions where a character literal is cast - /// to `u8` and suggests using a byte literal instead. - /// - /// **Why is this bad?** In general, casting values to smaller types is - /// error-prone and should be avoided where possible. In the particular case of - /// converting a character literal to u8, it is easy to avoid by just using a - /// byte literal instead. As an added bonus, `b'a'` is even slightly shorter - /// than `'a' as u8`. - /// - /// **Known problems:** None. - /// - /// **Example:** - /// ```rust,ignore - /// 'x' as u8 - /// ``` - /// - /// A better version, using the byte literal: - /// - /// ```rust,ignore - /// b'x' - /// ``` - pub CHAR_LIT_AS_U8, - complexity, - "casting a character literal to `u8` truncates" -} - -declare_lint_pass!(CharLitAsU8 => [CHAR_LIT_AS_U8]); - -impl<'tcx> LateLintPass<'tcx> for CharLitAsU8 { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if_chain! { - if !expr.span.from_expansion(); - if let ExprKind::Cast(e, _) = &expr.kind; - if let ExprKind::Lit(l) = &e.kind; - if let LitKind::Char(c) = l.node; - if ty::Uint(UintTy::U8) == *cx.typeck_results().expr_ty(expr).kind(); - then { - let mut applicability = Applicability::MachineApplicable; - let snippet = snippet_with_applicability(cx, e.span, "'x'", &mut applicability); - - span_lint_and_then( - cx, - CHAR_LIT_AS_U8, - expr.span, - "casting a character literal to `u8` truncates", - |diag| { - diag.note("`char` is four bytes wide, but `u8` is a single byte"); - - if c.is_ascii() { - diag.span_suggestion( - expr.span, - "use a byte literal instead", - format!("b{}", snippet), - applicability, - ); - } - }); - } - } - } -} - declare_clippy_lint! { /// **What it does:** /// Checks for `as` casts between raw pointers without changing its mutability, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 47afd7beb741..67c481145c34 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1108,7 +1108,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box panic_unimplemented::PanicUnimplemented); store.register_late_pass(|| box strings::StringLitAsBytes); store.register_late_pass(|| box derive::Derive); - store.register_late_pass(|| box casts::CharLitAsU8); store.register_late_pass(|| box get_last_with_len::GetLastWithLen); store.register_late_pass(|| box drop_forget_ref::DropForgetRef); store.register_late_pass(|| box empty_enum::EmptyEnum); From 9e631da454e3ab6f756228b438b61126a0e51518 Mon Sep 17 00:00:00 2001 From: Yoshitomo Nakanishi Date: Tue, 9 Mar 2021 20:04:19 +0900 Subject: [PATCH 15/15] Move ptr_as_ptr to its own module --- clippy_lints/src/casts/mod.rs | 143 +++++++++------------------ clippy_lints/src/casts/ptr_as_ptr.rs | 52 ++++++++++ clippy_lints/src/lib.rs | 3 +- 3 files changed, 100 insertions(+), 98 deletions(-) create mode 100644 clippy_lints/src/casts/ptr_as_ptr.rs diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index 6d49af21a5f0..b726bd75f1d8 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -8,22 +8,17 @@ mod cast_sign_loss; mod char_lit_as_u8; mod fn_to_numeric_cast; mod fn_to_numeric_cast_with_truncation; +mod ptr_as_ptr; mod unnecessary_cast; mod utils; -use std::borrow::Cow; - -use if_chain::if_chain; -use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind, Mutability, TyKind}; +use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; -use rustc_middle::ty::{self, TypeAndMut}; use rustc_semver::RustcVersion; -use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; -use crate::utils::sugg::Sugg; -use crate::utils::{is_hir_ty_cfg_dependant, meets_msrv, span_lint_and_sugg}; +use crate::utils::is_hir_ty_cfg_dependant; declare_clippy_lint! { /// **What it does:** Checks for casts from any numerical to a float type where @@ -315,58 +310,6 @@ declare_clippy_lint! { "casting a character literal to `u8` truncates" } -declare_lint_pass!(Casts => [ - CAST_PRECISION_LOSS, - CAST_SIGN_LOSS, - CAST_POSSIBLE_TRUNCATION, - CAST_POSSIBLE_WRAP, - CAST_LOSSLESS, - CAST_REF_TO_MUT, - CAST_PTR_ALIGNMENT, - UNNECESSARY_CAST, - FN_TO_NUMERIC_CAST, - FN_TO_NUMERIC_CAST_WITH_TRUNCATION, - CHAR_LIT_AS_U8, -]); - -impl<'tcx> LateLintPass<'tcx> for Casts { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - cast_ref_to_mut::check(cx, expr); - - if expr.span.from_expansion() { - return; - } - if let ExprKind::Cast(ref cast_expr, cast_to) = expr.kind { - if is_hir_ty_cfg_dependant(cx, cast_to) { - return; - } - let (cast_from, cast_to) = ( - cx.typeck_results().expr_ty(cast_expr), - cx.typeck_results().expr_ty(expr), - ); - - if unnecessary_cast::check(cx, expr, cast_expr, cast_from, cast_to) { - return; - } - - fn_to_numeric_cast::check(cx, expr, cast_expr, cast_from, cast_to); - fn_to_numeric_cast_with_truncation::check(cx, expr, cast_expr, cast_from, cast_to); - if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) { - cast_possible_truncation::check(cx, expr, cast_from, cast_to); - cast_possible_wrap::check(cx, expr, cast_from, cast_to); - cast_precision_loss::check(cx, expr, cast_from, cast_to); - cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to); - cast_sign_loss::check(cx, expr, cast_expr, cast_from, cast_to); - } - } - - cast_ptr_alignment::check(cx, expr); - char_lit_as_u8::check(cx, expr); - } -} - -const PTR_AS_PTR_MSRV: RustcVersion = RustcVersion::new(1, 38, 0); - declare_clippy_lint! { /// **What it does:** /// Checks for `as` casts between raw pointers without changing its mutability, @@ -398,58 +341,66 @@ declare_clippy_lint! { "casting using `as` from and to raw pointers that doesn't change its mutability, where `pointer::cast` could take the place of `as`" } -pub struct PtrAsPtr { +pub struct Casts { msrv: Option, } -impl PtrAsPtr { +impl Casts { #[must_use] pub fn new(msrv: Option) -> Self { Self { msrv } } } -impl_lint_pass!(PtrAsPtr => [PTR_AS_PTR]); +impl_lint_pass!(Casts => [ + CAST_PRECISION_LOSS, + CAST_SIGN_LOSS, + CAST_POSSIBLE_TRUNCATION, + CAST_POSSIBLE_WRAP, + CAST_LOSSLESS, + CAST_REF_TO_MUT, + CAST_PTR_ALIGNMENT, + UNNECESSARY_CAST, + FN_TO_NUMERIC_CAST, + FN_TO_NUMERIC_CAST_WITH_TRUNCATION, + CHAR_LIT_AS_U8, + PTR_AS_PTR, +]); -impl<'tcx> LateLintPass<'tcx> for PtrAsPtr { +impl<'tcx> LateLintPass<'tcx> for Casts { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if !meets_msrv(self.msrv.as_ref(), &PTR_AS_PTR_MSRV) { - return; - } - if expr.span.from_expansion() { return; } - if_chain! { - if let ExprKind::Cast(cast_expr, cast_to_hir_ty) = expr.kind; - let (cast_from, cast_to) = (cx.typeck_results().expr_ty(cast_expr), cx.typeck_results().expr_ty(expr)); - if let ty::RawPtr(TypeAndMut { mutbl: from_mutbl, .. }) = cast_from.kind(); - if let ty::RawPtr(TypeAndMut { ty: to_pointee_ty, mutbl: to_mutbl }) = cast_to.kind(); - if matches!((from_mutbl, to_mutbl), - (Mutability::Not, Mutability::Not) | (Mutability::Mut, Mutability::Mut)); - // The `U` in `pointer::cast` have to be `Sized` - // as explained here: https://github.com/rust-lang/rust/issues/60602. - if to_pointee_ty.is_sized(cx.tcx.at(expr.span), cx.param_env); - then { - let mut applicability = Applicability::MachineApplicable; - let cast_expr_sugg = Sugg::hir_with_applicability(cx, cast_expr, "_", &mut applicability); - let turbofish = match &cast_to_hir_ty.kind { - TyKind::Infer => Cow::Borrowed(""), - TyKind::Ptr(mut_ty) if matches!(mut_ty.ty.kind, TyKind::Infer) => Cow::Borrowed(""), - _ => Cow::Owned(format!("::<{}>", to_pointee_ty)), - }; - span_lint_and_sugg( - cx, - PTR_AS_PTR, - expr.span, - "`as` casting between raw pointers without changing its mutability", - "try `pointer::cast`, a safer alternative", - format!("{}.cast{}()", cast_expr_sugg.maybe_par(), turbofish), - applicability, - ); + if let ExprKind::Cast(ref cast_expr, cast_to) = expr.kind { + if is_hir_ty_cfg_dependant(cx, cast_to) { + return; + } + let (cast_from, cast_to) = ( + cx.typeck_results().expr_ty(cast_expr), + cx.typeck_results().expr_ty(expr), + ); + + if unnecessary_cast::check(cx, expr, cast_expr, cast_from, cast_to) { + return; + } + + fn_to_numeric_cast::check(cx, expr, cast_expr, cast_from, cast_to); + fn_to_numeric_cast_with_truncation::check(cx, expr, cast_expr, cast_from, cast_to); + if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) { + cast_possible_truncation::check(cx, expr, cast_from, cast_to); + cast_possible_wrap::check(cx, expr, cast_from, cast_to); + cast_precision_loss::check(cx, expr, cast_from, cast_to); + cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to); + cast_sign_loss::check(cx, expr, cast_expr, cast_from, cast_to); } } + + cast_ref_to_mut::check(cx, expr); + cast_ptr_alignment::check(cx, expr); + char_lit_as_u8::check(cx, expr); + ptr_as_ptr::check(cx, expr, &self.msrv); } extract_msrv_attr!(LateContext); diff --git a/clippy_lints/src/casts/ptr_as_ptr.rs b/clippy_lints/src/casts/ptr_as_ptr.rs new file mode 100644 index 000000000000..abfbadf3642b --- /dev/null +++ b/clippy_lints/src/casts/ptr_as_ptr.rs @@ -0,0 +1,52 @@ +use std::borrow::Cow; + +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, Mutability, TyKind}; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, TypeAndMut}; +use rustc_semver::RustcVersion; + +use if_chain::if_chain; + +use crate::utils::sugg::Sugg; +use crate::utils::{meets_msrv, span_lint_and_sugg}; + +use super::PTR_AS_PTR; + +const PTR_AS_PTR_MSRV: RustcVersion = RustcVersion::new(1, 38, 0); + +pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: &Option) { + if !meets_msrv(msrv.as_ref(), &PTR_AS_PTR_MSRV) { + return; + } + + if_chain! { + if let ExprKind::Cast(cast_expr, cast_to_hir_ty) = expr.kind; + let (cast_from, cast_to) = (cx.typeck_results().expr_ty(cast_expr), cx.typeck_results().expr_ty(expr)); + if let ty::RawPtr(TypeAndMut { mutbl: from_mutbl, .. }) = cast_from.kind(); + if let ty::RawPtr(TypeAndMut { ty: to_pointee_ty, mutbl: to_mutbl }) = cast_to.kind(); + if matches!((from_mutbl, to_mutbl), + (Mutability::Not, Mutability::Not) | (Mutability::Mut, Mutability::Mut)); + // The `U` in `pointer::cast` have to be `Sized` + // as explained here: https://github.com/rust-lang/rust/issues/60602. + if to_pointee_ty.is_sized(cx.tcx.at(expr.span), cx.param_env); + then { + let mut applicability = Applicability::MachineApplicable; + let cast_expr_sugg = Sugg::hir_with_applicability(cx, cast_expr, "_", &mut applicability); + let turbofish = match &cast_to_hir_ty.kind { + TyKind::Infer => Cow::Borrowed(""), + TyKind::Ptr(mut_ty) if matches!(mut_ty.ty.kind, TyKind::Infer) => Cow::Borrowed(""), + _ => Cow::Owned(format!("::<{}>", to_pointee_ty)), + }; + span_lint_and_sugg( + cx, + PTR_AS_PTR, + expr.span, + "`as` casting between raw pointers without changing its mutability", + "try `pointer::cast`, a safer alternative", + format!("{}.cast{}()", cast_expr_sugg.maybe_par(), turbofish), + applicability, + ); + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 67c481145c34..04e151df8e85 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1076,6 +1076,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || box use_self::UseSelf::new(msrv)); store.register_late_pass(move || box missing_const_for_fn::MissingConstForFn::new(msrv)); store.register_late_pass(move || box needless_question_mark::NeedlessQuestionMark::new(msrv)); + store.register_late_pass(move || box casts::Casts::new(msrv)); store.register_late_pass(|| box size_of_in_element_count::SizeOfInElementCount); store.register_late_pass(|| box map_clone::MapClone); @@ -1087,7 +1088,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box main_recursion::MainRecursion::default()); store.register_late_pass(|| box lifetimes::Lifetimes); store.register_late_pass(|| box entry::HashMapPass); - store.register_late_pass(|| box casts::Casts); let type_complexity_threshold = conf.type_complexity_threshold; store.register_late_pass(move || box types::TypeComplexity::new(type_complexity_threshold)); store.register_late_pass(|| box minmax::MinMaxPass); @@ -1276,7 +1276,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box strings::StringToString); store.register_late_pass(|| box zero_sized_map_values::ZeroSizedMapValues); store.register_late_pass(|| box vec_init_then_push::VecInitThenPush::default()); - store.register_late_pass(move || box casts::PtrAsPtr::new(msrv)); store.register_late_pass(|| box case_sensitive_file_extension_comparisons::CaseSensitiveFileExtensionComparisons); store.register_late_pass(|| box redundant_slicing::RedundantSlicing); store.register_late_pass(|| box from_str_radix_10::FromStrRadix10);