Skip to content

Commit

Permalink
Auto merge of rust-lang#10942 - Centri3:unnecessary_cast, r=llogiq
Browse files Browse the repository at this point in the history
Ignore more type aliases in `unnecessary_cast`

This is potentially the worst code I've ever written, and even if not, it's very close to being on par with starb. This will ignore `call() as i32` and `local_obtained_from_call as i32` now.

This should fix every reasonable way to reproduce rust-lang#10555, but likely not entirely.

changelog: Ignore more type aliases in `unnecessary_cast`
  • Loading branch information
bors committed Jun 16, 2023
2 parents e11f36c + 357e80e commit 3217f8a
Show file tree
Hide file tree
Showing 5 changed files with 166 additions and 45 deletions.
71 changes: 68 additions & 3 deletions clippy_lints/src/casts/unnecessary_cast.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
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, is_hir_ty_cfg_dependant, is_ty_alias, 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::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;

Expand Down Expand Up @@ -59,7 +61,7 @@ pub(super) fn check<'tcx>(
}
}

// skip cast of local to type alias
// 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
Expand All @@ -83,6 +85,11 @@ pub(super) fn check<'tcx>(
}
}

// 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;
Expand Down Expand Up @@ -223,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()
}
10 changes: 10 additions & 0 deletions tests/ui/auxiliary/extern_fake_libc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#![allow(nonstandard_style)]
#![allow(clippy::missing_safety_doc, unused)]

type pid_t = i32;
pub unsafe fn getpid() -> pid_t {
pid_t::from(0)
}
pub fn getpid_SAFE_TRUTH() -> pid_t {
unsafe { getpid() }
}
27 changes: 25 additions & 2 deletions tests/ui/unnecessary_cast.fixed
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
//@run-rustfix
//@aux-build:extern_fake_libc.rs
#![warn(clippy::unnecessary_cast)]
#![allow(
unused,
clippy::borrow_as_ptr,
clippy::no_effect,
clippy::nonstandard_macro_braces,
clippy::unnecessary_operation
clippy::unnecessary_operation,
nonstandard_style,
unused
)]

extern crate extern_fake_libc;

type PtrConstU8 = *const u8;
type PtrMutU8 = *mut u8;

Expand All @@ -19,6 +23,21 @@ fn uwu<T, U>(ptr: *const T) -> *const U {
ptr as *const U
}

mod fake_libc {
type pid_t = i32;
pub unsafe fn getpid() -> pid_t {
pid_t::from(0)
}
// Make sure a where clause does not break it
pub fn getpid_SAFE_TRUTH<T: Clone>(t: &T) -> pid_t
where
T: Clone,
{
t;
unsafe { getpid() }
}
}

#[rustfmt::skip]
fn main() {
// Test cast_unnecessary
Expand Down Expand Up @@ -82,6 +101,10 @@ fn main() {
// or from
let x: I32Alias = 1;
let y = x as u64;
fake_libc::getpid_SAFE_TRUTH(&0u32) as i32;
extern_fake_libc::getpid_SAFE_TRUTH() as i32;
let pid = unsafe { fake_libc::getpid() };
pid as i32;

let i8_ptr: *const i8 = &1;
let u8_ptr: *const u8 = &1;
Expand Down
27 changes: 25 additions & 2 deletions tests/ui/unnecessary_cast.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
//@run-rustfix
//@aux-build:extern_fake_libc.rs
#![warn(clippy::unnecessary_cast)]
#![allow(
unused,
clippy::borrow_as_ptr,
clippy::no_effect,
clippy::nonstandard_macro_braces,
clippy::unnecessary_operation
clippy::unnecessary_operation,
nonstandard_style,
unused
)]

extern crate extern_fake_libc;

type PtrConstU8 = *const u8;
type PtrMutU8 = *mut u8;

Expand All @@ -19,6 +23,21 @@ fn uwu<T, U>(ptr: *const T) -> *const U {
ptr as *const U
}

mod fake_libc {
type pid_t = i32;
pub unsafe fn getpid() -> pid_t {
pid_t::from(0)
}
// Make sure a where clause does not break it
pub fn getpid_SAFE_TRUTH<T: Clone>(t: &T) -> pid_t
where
T: Clone,
{
t;
unsafe { getpid() }
}
}

#[rustfmt::skip]
fn main() {
// Test cast_unnecessary
Expand Down Expand Up @@ -82,6 +101,10 @@ fn main() {
// or from
let x: I32Alias = 1;
let y = x as u64;
fake_libc::getpid_SAFE_TRUTH(&0u32) as i32;
extern_fake_libc::getpid_SAFE_TRUTH() as i32;
let pid = unsafe { fake_libc::getpid() };
pid as i32;

let i8_ptr: *const i8 = &1;
let u8_ptr: *const u8 = &1;
Expand Down
Loading

0 comments on commit 3217f8a

Please sign in to comment.