Skip to content

Commit

Permalink
Auto merge of #10821 - Centri3:unnecessary_operation_cast_underscore,…
Browse files Browse the repository at this point in the history
… r=dswij

Emit `unnecessary_cast` on raw pointers as well

Supersedes(?) #10782, since this and #10567 will cover the original issue.
Does not lint on type aliases or inferred types.

changelog: [`unnecessary_cast`]: Also emit on casts between raw pointers with the same type and constness
  • Loading branch information
bors committed Jun 2, 2023
2 parents 00001d6 + d7a98f5 commit c85ceea
Show file tree
Hide file tree
Showing 12 changed files with 188 additions and 56 deletions.
4 changes: 2 additions & 2 deletions clippy_lints/src/casts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,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.
Expand Down
38 changes: 34 additions & 4 deletions clippy_lints/src/casts/unnecessary_cast.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
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::{get_parent_expr, is_ty_alias, path_to_local};
use if_chain::if_chain;
use rustc_ast::{LitFloatType, LitIntType, LitKind};
use rustc_errors::Applicability;
Expand All @@ -20,19 +20,49 @@ pub(super) fn check<'tcx>(
cast_from: Ty<'tcx>,
cast_to: Ty<'tcx>,
) -> bool {
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;
then {
if_chain! {
if let TyKind::Path(qpath) = cast_to_hir.kind;
if is_ty_alias(&qpath);
then {
return false;
}
}

if let TyKind::Infer = cast_to_hir.kind {
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 non-primitive type cast
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;

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/as_ptr_cast_mut.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![allow(unused)]
#![warn(clippy::as_ptr_cast_mut)]
#![allow(clippy::wrong_self_convention)]
#![allow(clippy::wrong_self_convention, clippy::unnecessary_cast)]

struct MutPtrWrapper(Vec<u8>);
impl MutPtrWrapper {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/cast_slice_different_sizes.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![allow(clippy::let_unit_value)]
#![allow(clippy::let_unit_value, clippy::unnecessary_cast)]

fn main() {
let x: [i32; 3] = [1_i32, 2, 3];
Expand Down
1 change: 1 addition & 0 deletions tests/ui/crashes/ice-1782.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![allow(dead_code, unused_variables)]
#![allow(clippy::unnecessary_cast)]

/// Should not trigger an ICE in `SpanlessEq` / `consts::constant`
///
Expand Down
1 change: 1 addition & 0 deletions tests/ui/from_raw_with_void_ptr.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![warn(clippy::from_raw_with_void_ptr)]
#![allow(clippy::unnecessary_cast)]

use std::ffi::c_void;
use std::rc::Rc;
Expand Down
20 changes: 10 additions & 10 deletions tests/ui/from_raw_with_void_ptr.stderr
Original file line number Diff line number Diff line change
@@ -1,60 +1,60 @@
error: creating a `Box` from a void raw pointer
--> $DIR/from_raw_with_void_ptr.rs:10:22
--> $DIR/from_raw_with_void_ptr.rs:11:22
|
LL | let _ = unsafe { Box::from_raw(ptr) };
| ^^^^^^^^^^^^^^^^^^
|
help: cast this to a pointer of the appropriate type
--> $DIR/from_raw_with_void_ptr.rs:10:36
--> $DIR/from_raw_with_void_ptr.rs:11:36
|
LL | let _ = unsafe { Box::from_raw(ptr) };
| ^^^
= note: `-D clippy::from-raw-with-void-ptr` implied by `-D warnings`

error: creating a `Rc` from a void raw pointer
--> $DIR/from_raw_with_void_ptr.rs:21:22
--> $DIR/from_raw_with_void_ptr.rs:22:22
|
LL | let _ = unsafe { Rc::from_raw(ptr) };
| ^^^^^^^^^^^^^^^^^
|
help: cast this to a pointer of the appropriate type
--> $DIR/from_raw_with_void_ptr.rs:21:35
--> $DIR/from_raw_with_void_ptr.rs:22:35
|
LL | let _ = unsafe { Rc::from_raw(ptr) };
| ^^^

error: creating a `Arc` from a void raw pointer
--> $DIR/from_raw_with_void_ptr.rs:25:22
--> $DIR/from_raw_with_void_ptr.rs:26:22
|
LL | let _ = unsafe { Arc::from_raw(ptr) };
| ^^^^^^^^^^^^^^^^^^
|
help: cast this to a pointer of the appropriate type
--> $DIR/from_raw_with_void_ptr.rs:25:36
--> $DIR/from_raw_with_void_ptr.rs:26:36
|
LL | let _ = unsafe { Arc::from_raw(ptr) };
| ^^^

error: creating a `Weak` from a void raw pointer
--> $DIR/from_raw_with_void_ptr.rs:29:22
--> $DIR/from_raw_with_void_ptr.rs:30:22
|
LL | let _ = unsafe { std::rc::Weak::from_raw(ptr) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: cast this to a pointer of the appropriate type
--> $DIR/from_raw_with_void_ptr.rs:29:46
--> $DIR/from_raw_with_void_ptr.rs:30:46
|
LL | let _ = unsafe { std::rc::Weak::from_raw(ptr) };
| ^^^

error: creating a `Weak` from a void raw pointer
--> $DIR/from_raw_with_void_ptr.rs:33:22
--> $DIR/from_raw_with_void_ptr.rs:34:22
|
LL | let _ = unsafe { std::sync::Weak::from_raw(ptr) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: cast this to a pointer of the appropriate type
--> $DIR/from_raw_with_void_ptr.rs:33:48
--> $DIR/from_raw_with_void_ptr.rs:34:48
|
LL | let _ = unsafe { std::sync::Weak::from_raw(ptr) };
| ^^^
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/transmute_ptr_to_ref.fixed
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//@run-rustfix

#![warn(clippy::transmute_ptr_to_ref)]
#![allow(clippy::match_single_binding)]
#![allow(clippy::match_single_binding, clippy::unnecessary_cast)]

unsafe fn _ptr_to_ref<T, U>(p: *const T, m: *mut T, o: *const U, om: *mut U) {
let _: &T = &*p;
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/transmute_ptr_to_ref.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//@run-rustfix

#![warn(clippy::transmute_ptr_to_ref)]
#![allow(clippy::match_single_binding)]
#![allow(clippy::match_single_binding, clippy::unnecessary_cast)]

unsafe fn _ptr_to_ref<T, U>(p: *const T, m: *mut T, o: *const U, om: *mut U) {
let _: &T = std::mem::transmute(p);
Expand Down
31 changes: 30 additions & 1 deletion tests/ui/unnecessary_cast.fixed
Original file line number Diff line number Diff line change
@@ -1,13 +1,24 @@
//@run-rustfix
#![warn(clippy::unnecessary_cast)]
#![allow(
unused_must_use,
unused,
clippy::borrow_as_ptr,
clippy::no_effect,
clippy::nonstandard_macro_braces,
clippy::unnecessary_operation
)]

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

fn owo<T>(ptr: *const T) -> *const T {
ptr
}

fn uwu<T, U>(ptr: *const T) -> *const U {
ptr as *const U
}

#[rustfmt::skip]
fn main() {
// Test cast_unnecessary
Expand All @@ -22,6 +33,24 @@ fn main() {
1_i32;
1_f32;

let _: *mut u8 = [1u8, 2].as_ptr() as *mut u8;

[1u8, 2].as_ptr();
[1u8, 2].as_ptr() as *mut u8;
[1u8, 2].as_mut_ptr();
[1u8, 2].as_mut_ptr() as *const u8;
[1u8, 2].as_ptr() as PtrConstU8;
[1u8, 2].as_ptr() as PtrMutU8;
[1u8, 2].as_mut_ptr() as PtrMutU8;
[1u8, 2].as_mut_ptr() as PtrConstU8;
let _: *const u8 = [1u8, 2].as_ptr() as _;
let _: *mut u8 = [1u8, 2].as_mut_ptr() as _;

owo::<u32>([1u32].as_ptr());
uwu::<u32, u8>([1u32].as_ptr());
// this will not lint in the function body even though they have the same type, instead here
uwu::<u32, u32>([1u32].as_ptr());

// macro version
macro_rules! foo {
($a:ident, $b:ident) => {
Expand Down
31 changes: 30 additions & 1 deletion tests/ui/unnecessary_cast.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,24 @@
//@run-rustfix
#![warn(clippy::unnecessary_cast)]
#![allow(
unused_must_use,
unused,
clippy::borrow_as_ptr,
clippy::no_effect,
clippy::nonstandard_macro_braces,
clippy::unnecessary_operation
)]

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

fn owo<T>(ptr: *const T) -> *const T {
ptr as *const T
}

fn uwu<T, U>(ptr: *const T) -> *const U {
ptr as *const U
}

#[rustfmt::skip]
fn main() {
// Test cast_unnecessary
Expand All @@ -22,6 +33,24 @@ fn main() {
1_i32 as i32;
1_f32 as f32;

let _: *mut u8 = [1u8, 2].as_ptr() as *const u8 as *mut u8;

[1u8, 2].as_ptr() as *const u8;
[1u8, 2].as_ptr() as *mut u8;
[1u8, 2].as_mut_ptr() as *mut u8;
[1u8, 2].as_mut_ptr() as *const u8;
[1u8, 2].as_ptr() as PtrConstU8;
[1u8, 2].as_ptr() as PtrMutU8;
[1u8, 2].as_mut_ptr() as PtrMutU8;
[1u8, 2].as_mut_ptr() as PtrConstU8;
let _: *const u8 = [1u8, 2].as_ptr() as _;
let _: *mut u8 = [1u8, 2].as_mut_ptr() as _;

owo::<u32>([1u32].as_ptr()) as *const u32;
uwu::<u32, u8>([1u32].as_ptr()) as *const u8;
// this will not lint in the function body even though they have the same type, instead here
uwu::<u32, u32>([1u32].as_ptr()) as *const u32;

// macro version
macro_rules! foo {
($a:ident, $b:ident) => {
Expand Down
Loading

0 comments on commit c85ceea

Please sign in to comment.