about summary refs log tree commit diff
path: root/clippy_lints/src/casts
diff options
context:
space:
mode:
authorPhilipp Krones <hello@philkrones.com>2023-07-02 14:35:19 +0200
committerPhilipp Krones <hello@philkrones.com>2023-07-02 14:59:02 +0200
commitcb3ecf7b792fdc4b00e61935b9e40ca836752492 (patch)
treea183611f56d9139413f6ab1c78c0619512d3c751 /clippy_lints/src/casts
parentbb33e0343fe37815f6180a861619a9fca6771ce9 (diff)
downloadrust-cb3ecf7b792fdc4b00e61935b9e40ca836752492.tar.gz
rust-cb3ecf7b792fdc4b00e61935b9e40ca836752492.zip
Merge commit '37f4c1725d3fd7e9c3ffd8783246bc5589debc53' into clippyup
Diffstat (limited to 'clippy_lints/src/casts')
-rw-r--r--clippy_lints/src/casts/borrow_as_ptr.rs10
-rw-r--r--clippy_lints/src/casts/cast_possible_wrap.rs95
-rw-r--r--clippy_lints/src/casts/mod.rs13
-rw-r--r--clippy_lints/src/casts/ptr_cast_constness.rs13
-rw-r--r--clippy_lints/src/casts/unnecessary_cast.rs140
5 files changed, 229 insertions, 42 deletions
diff --git a/clippy_lints/src/casts/borrow_as_ptr.rs b/clippy_lints/src/casts/borrow_as_ptr.rs
index 294d22d34de..b7256dd2eae 100644
--- a/clippy_lints/src/casts/borrow_as_ptr.rs
+++ b/clippy_lints/src/casts/borrow_as_ptr.rs
@@ -4,6 +4,7 @@ use clippy_utils::source::snippet_with_context;
 use rustc_errors::Applicability;
 use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, Ty, TyKind};
 use rustc_lint::LateContext;
+use rustc_middle::ty::adjustment::Adjust;
 
 use super::BORROW_AS_PTR;
 
@@ -23,6 +24,15 @@ pub(super) fn check<'tcx>(
         };
         let mut app = Applicability::MachineApplicable;
         let snip = snippet_with_context(cx, e.span, cast_expr.span.ctxt(), "..", &mut app).0;
+        // Fix #9884
+        if !e.is_place_expr(|base| {
+            cx.typeck_results()
+                .adjustments()
+                .get(base.hir_id)
+                .is_some_and(|x| x.iter().any(|adj| matches!(adj.kind, Adjust::Deref(_))))
+        }) {
+            return;
+        }
 
         span_lint_and_sugg(
             cx,
diff --git a/clippy_lints/src/casts/cast_possible_wrap.rs b/clippy_lints/src/casts/cast_possible_wrap.rs
index 28ecdea7ea0..ffa571abb39 100644
--- a/clippy_lints/src/casts/cast_possible_wrap.rs
+++ b/clippy_lints/src/casts/cast_possible_wrap.rs
@@ -1,41 +1,90 @@
-use clippy_utils::diagnostics::span_lint;
-use clippy_utils::ty::is_isize_or_usize;
 use rustc_hir::Expr;
-use rustc_lint::LateContext;
+use rustc_lint::{LateContext, LintContext};
 use rustc_middle::ty::Ty;
 
 use super::{utils, CAST_POSSIBLE_WRAP};
 
+// this should be kept in sync with the allowed bit widths of `usize` and `isize`
+const ALLOWED_POINTER_SIZES: [u64; 3] = [16, 32, 64];
+
+// whether the lint should be emitted, and the required pointer size, if it matters
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+enum EmitState {
+    NoLint,
+    LintAlways,
+    LintOnPtrSize(u64),
+}
+
 pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
     if !(cast_from.is_integral() && cast_to.is_integral()) {
         return;
     }
 
-    let arch_64_suffix = " on targets with 64-bit wide pointers";
-    let arch_32_suffix = " on targets with 32-bit wide pointers";
-    let cast_unsigned_to_signed = !cast_from.is_signed() && cast_to.is_signed();
+    // emit a lint if a cast is:
+    // 1. unsigned to signed
+    // and
+    // 2. either:
+    //
+    //    2a. between two types of constant size that are always the same size
+    //    2b. between one target-dependent size and one constant size integer,
+    //        and the constant integer is in the allowed set of target dependent sizes
+    //        (the ptr size could be chosen to be the same as the constant size)
+
+    if cast_from.is_signed() || !cast_to.is_signed() {
+        return;
+    }
+
     let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
     let to_nbits = utils::int_ty_to_nbits(cast_to, cx.tcx);
 
-    let (should_lint, suffix) = match (is_isize_or_usize(cast_from), is_isize_or_usize(cast_to)) {
-        (true, true) | (false, false) => (to_nbits == from_nbits && cast_unsigned_to_signed, ""),
-        (true, false) => (to_nbits <= 32 && cast_unsigned_to_signed, arch_32_suffix),
-        (false, true) => (
-            cast_unsigned_to_signed,
-            if from_nbits == 64 {
-                arch_64_suffix
+    let should_lint = match (cast_from.is_ptr_sized_integral(), cast_to.is_ptr_sized_integral()) {
+        (true, true) => {
+            // casts between two ptr sized integers are trivially always the same size
+            // so do not depend on any specific pointer size to be the same
+            EmitState::LintAlways
+        },
+        (true, false) => {
+            // the first type is `usize` and the second is a constant sized signed integer
+            if ALLOWED_POINTER_SIZES.contains(&to_nbits) {
+                EmitState::LintOnPtrSize(to_nbits)
+            } else {
+                EmitState::NoLint
+            }
+        },
+        (false, true) => {
+            // the first type is a constant sized unsigned integer, and the second is `isize`
+            if ALLOWED_POINTER_SIZES.contains(&from_nbits) {
+                EmitState::LintOnPtrSize(from_nbits)
+            } else {
+                EmitState::NoLint
+            }
+        },
+        (false, false) => {
+            // the types are both a constant known size
+            // and do not depend on any specific pointer size to be the same
+            if from_nbits == to_nbits {
+                EmitState::LintAlways
             } else {
-                arch_32_suffix
-            },
+                EmitState::NoLint
+            }
+        },
+    };
+
+    let message = match should_lint {
+        EmitState::NoLint => return,
+        EmitState::LintAlways => format!("casting `{cast_from}` to `{cast_to}` may wrap around the value"),
+        EmitState::LintOnPtrSize(ptr_size) => format!(
+            "casting `{cast_from}` to `{cast_to}` may wrap around the value on targets with {ptr_size}-bit wide pointers",
         ),
     };
 
-    if should_lint {
-        span_lint(
-            cx,
-            CAST_POSSIBLE_WRAP,
-            expr.span,
-            &format!("casting `{cast_from}` to `{cast_to}` may wrap around the value{suffix}",),
-        );
-    }
+    cx.struct_span_lint(CAST_POSSIBLE_WRAP, expr.span, message, |diag| {
+        if let EmitState::LintOnPtrSize(16) = should_lint {
+            diag
+            .note("`usize` and `isize` may be as small as 16 bits on some platforms")
+            .note("for more information see https://doc.rust-lang.org/reference/types/numeric.html#machine-dependent-integer-types")
+        } else {
+            diag
+        }
+    });
 }
diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs
index b90dab07a27..0ac6ef6496a 100644
--- a/clippy_lints/src/casts/mod.rs
+++ b/clippy_lints/src/casts/mod.rs
@@ -118,9 +118,10 @@ declare_clippy_lint! {
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for casts from an unsigned type to a signed type of
-    /// the same size. Performing such a cast is a 'no-op' for the compiler,
-    /// i.e., nothing is changed at the bit level, and the binary representation of
-    /// the value is reinterpreted. This can cause wrapping if the value is too big
+    /// the same size, or possibly smaller due to target dependent integers.
+    /// Performing such a cast is a 'no-op' for the compiler, i.e., nothing is
+    /// changed at the bit level, and the binary representation of the value is
+    /// reinterpreted. This can cause wrapping if the value is too big
     /// for the target signed type. However, the cast works as defined, so this lint
     /// is `Allow` by default.
     ///
@@ -174,8 +175,8 @@ declare_clippy_lint! {
 
 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.
+    /// Checks for casts to the same type, casts of int literals to integer types, casts of float
+    /// literals to float types and casts between raw pointers without changing type or constness.
     ///
     /// ### Why is this bad?
     /// It's just unnecessary.
@@ -522,7 +523,7 @@ declare_clippy_lint! {
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Check for the usage of `as _` conversion using inferred type.
+    /// Checks for the usage of `as _` conversion using inferred type.
     ///
     /// ### Why is this bad?
     /// The conversion might include lossy conversion and dangerous cast that might go
diff --git a/clippy_lints/src/casts/ptr_cast_constness.rs b/clippy_lints/src/casts/ptr_cast_constness.rs
index ab015f8822e..f0c1df01430 100644
--- a/clippy_lints/src/casts/ptr_cast_constness.rs
+++ b/clippy_lints/src/casts/ptr_cast_constness.rs
@@ -9,20 +9,21 @@ use rustc_middle::ty::{self, Ty, TypeAndMut};
 
 use super::PTR_CAST_CONSTNESS;
 
-pub(super) fn check(
+pub(super) fn check<'tcx>(
     cx: &LateContext<'_>,
     expr: &Expr<'_>,
     cast_expr: &Expr<'_>,
-    cast_from: Ty<'_>,
-    cast_to: Ty<'_>,
+    cast_from: Ty<'tcx>,
+    cast_to: Ty<'tcx>,
     msrv: &Msrv,
 ) {
     if_chain! {
         if msrv.meets(POINTER_CAST_CONSTNESS);
-        if let ty::RawPtr(TypeAndMut { mutbl: from_mutbl, .. }) = cast_from.kind();
-        if let ty::RawPtr(TypeAndMut { mutbl: to_mutbl, .. }) = cast_to.kind();
+        if let ty::RawPtr(TypeAndMut { mutbl: from_mutbl, ty: from_ty }) = cast_from.kind();
+        if let ty::RawPtr(TypeAndMut { mutbl: to_mutbl, ty: to_ty }) = cast_to.kind();
         if matches!((from_mutbl, to_mutbl),
             (Mutability::Not, Mutability::Mut) | (Mutability::Mut, Mutability::Not));
+        if from_ty == to_ty;
         then {
             let sugg = Sugg::hir(cx, cast_expr, "_");
             let constness = match *to_mutbl {
@@ -34,7 +35,7 @@ pub(super) fn check(
                 cx,
                 PTR_CAST_CONSTNESS,
                 expr.span,
-                "`as` casting between raw pointers while changing its constness",
+                "`as` casting between raw pointers while changing only its constness",
                 &format!("try `pointer::cast_{constness}`, a safer alternative"),
                 format!("{}.cast_{constness}()", sugg.maybe_par()),
                 Applicability::MachineApplicable,
diff --git a/clippy_lints/src/casts/unnecessary_cast.rs b/clippy_lints/src/casts/unnecessary_cast.rs
index 804ae841100..71cf2aea0f8 100644
--- a/clippy_lints/src/casts/unnecessary_cast.rs
+++ b/clippy_lints/src/casts/unnecessary_cast.rs
@@ -1,18 +1,21 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::numeric_literal::NumericLiteral;
 use clippy_utils::source::snippet_opt;
-use clippy_utils::{get_parent_expr, path_to_local};
+use clippy_utils::visitors::{for_each_expr, Visitable};
+use clippy_utils::{get_parent_expr, get_parent_node, is_hir_ty_cfg_dependant, is_ty_alias, path_to_local};
 use if_chain::if_chain;
 use rustc_ast::{LitFloatType, LitIntType, LitKind};
 use rustc_errors::Applicability;
-use rustc_hir::def::Res;
-use rustc_hir::{Expr, ExprKind, Lit, QPath, TyKind, UnOp};
+use rustc_hir::def::{DefKind, Res};
+use rustc_hir::{Expr, ExprKind, Lit, Node, Path, QPath, TyKind, UnOp};
 use rustc_lint::{LateContext, LintContext};
 use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty::{self, FloatTy, InferTy, Ty};
+use std::ops::ControlFlow;
 
 use super::UNNECESSARY_CAST;
 
+#[expect(clippy::too_many_lines)]
 pub(super) fn check<'tcx>(
     cx: &LateContext<'tcx>,
     expr: &Expr<'tcx>,
@@ -20,19 +23,84 @@ pub(super) fn check<'tcx>(
     cast_from: Ty<'tcx>,
     cast_to: Ty<'tcx>,
 ) -> bool {
-    // skip non-primitive type cast
+    let cast_str = snippet_opt(cx, cast_expr.span).unwrap_or_default();
+
+    if_chain! {
+        if let ty::RawPtr(..) = cast_from.kind();
+        // check both mutability and type are the same
+        if cast_from.kind() == cast_to.kind();
+        if let ExprKind::Cast(_, cast_to_hir) = expr.kind;
+        // Ignore casts to e.g. type aliases and infer types
+        // - p as pointer_alias
+        // - p as _
+        if let TyKind::Ptr(to_pointee) = cast_to_hir.kind;
+        then {
+            match to_pointee.ty.kind {
+                // Ignore casts to pointers that are aliases or cfg dependant, e.g.
+                // - p as *const std::ffi::c_char (alias)
+                // - p as *const std::os::raw::c_char (cfg dependant)
+                TyKind::Path(qpath) => {
+                    if is_ty_alias(&qpath) || is_hir_ty_cfg_dependant(cx, to_pointee.ty) {
+                        return false;
+                    }
+                },
+                // Ignore `p as *const _`
+                TyKind::Infer => return false,
+                _ => {},
+            }
+
+            span_lint_and_sugg(
+                cx,
+                UNNECESSARY_CAST,
+                expr.span,
+                &format!("casting raw pointers to the same type and constness is unnecessary (`{cast_from}` -> `{cast_to}`)"),
+                "try",
+                cast_str.clone(),
+                Applicability::MachineApplicable,
+            );
+        }
+    }
+
+    // skip cast of local that is a type alias
+    if let ExprKind::Cast(inner, ..) = expr.kind
+        && let ExprKind::Path(qpath) = inner.kind
+        && let QPath::Resolved(None, Path { res, .. }) = qpath
+        && let Res::Local(hir_id) = res
+        && let parent = cx.tcx.hir().get_parent(*hir_id)
+        && let Node::Local(local) = parent
+    {
+        if let Some(ty) = local.ty
+            && let TyKind::Path(qpath) = ty.kind
+            && is_ty_alias(&qpath)
+        {
+            return false;
+        }
+
+        if let Some(expr) = local.init
+            && let ExprKind::Cast(.., cast_to) = expr.kind
+            && let TyKind::Path(qpath) = cast_to.kind
+            && is_ty_alias(&qpath)
+        {
+            return false;
+        }
+    }
+
+    // skip cast of fn call that returns type alias
+    if let ExprKind::Cast(inner, ..) = expr.kind && is_cast_from_ty_alias(cx, inner, cast_from) {
+        return false;
+    }
+
+    // skip cast to non-primitive type
     if_chain! {
         if let ExprKind::Cast(_, cast_to) = expr.kind;
         if let TyKind::Path(QPath::Resolved(_, path)) = &cast_to.kind;
         if let Res::PrimTy(_) = path.res;
         then {}
         else {
-            return false
+            return false;
         }
     }
 
-    let cast_str = snippet_opt(cx, cast_expr.span).unwrap_or_default();
-
     if let Some(lit) = get_numeric_literal(cast_expr) {
         let literal_str = &cast_str;
 
@@ -162,3 +230,61 @@ fn fp_ty_mantissa_nbits(typ: Ty<'_>) -> u32 {
         _ => 0,
     }
 }
+
+/// Finds whether an `Expr` returns a type alias.
+///
+/// TODO: Maybe we should move this to `clippy_utils` so others won't need to go down this dark,
+/// dark path reimplementing this (or something similar).
+fn is_cast_from_ty_alias<'tcx>(cx: &LateContext<'tcx>, expr: impl Visitable<'tcx>, cast_from: Ty<'tcx>) -> bool {
+    for_each_expr(expr, |expr| {
+        // Calls are a `Path`, and usage of locals are a `Path`. So, this checks
+        // - call() as i32
+        // - local as i32
+        if let ExprKind::Path(qpath) = expr.kind {
+            let res = cx.qpath_res(&qpath, expr.hir_id);
+            // Function call
+            if let Res::Def(DefKind::Fn, def_id) = res {
+                let Some(snippet) = snippet_opt(cx, cx.tcx.def_span(def_id)) else {
+                    return ControlFlow::Continue(());
+                };
+                // This is the worst part of this entire function. This is the only way I know of to
+                // check whether a function returns a type alias. Sure, you can get the return type
+                // from a function in the current crate as an hir ty, but how do you get it for
+                // external functions?? Simple: It's impossible. So, we check whether a part of the
+                // function's declaration snippet is exactly equal to the `Ty`. That way, we can
+                // see whether it's a type alias.
+                //
+                // Will this work for more complex types? Probably not!
+                if !snippet
+                    .split("->")
+                    .skip(0)
+                    .map(|s| {
+                        s.trim() == cast_from.to_string()
+                            || s.split("where").any(|ty| ty.trim() == cast_from.to_string())
+                    })
+                    .any(|a| a)
+                {
+                    return ControlFlow::Break(());
+                }
+            // Local usage
+            } else if let Res::Local(hir_id) = res
+                && let Some(parent) = get_parent_node(cx.tcx, hir_id)
+                && let Node::Local(l) = parent
+            {
+                if let Some(e) = l.init && is_cast_from_ty_alias(cx, e, cast_from) {
+                    return ControlFlow::Break::<()>(());
+                }
+
+                if let Some(ty) = l.ty
+                    && let TyKind::Path(qpath) = ty.kind
+                    && is_ty_alias(&qpath)
+                {
+                    return ControlFlow::Break::<()>(());
+                }
+            }
+        }
+
+        ControlFlow::Continue(())
+    })
+    .is_some()
+}