Skip to content

Commit

Permalink
Auto merge of #13143 - Alexendoo:transmute-ptr-to-ptr-suggestions, r=y21
Browse files Browse the repository at this point in the history
Suggest `.cast`/`.cast_const`/`.cast_mut` in `transmute_ptr_as_ptr`

Essentially pre-applies `ptr_as_ptr` - #6372 (comment)

changelog: none
  • Loading branch information
bors committed Jul 23, 2024
2 parents bd1224d + 7010d3c commit e91d82e
Show file tree
Hide file tree
Showing 11 changed files with 309 additions and 84 deletions.
3 changes: 2 additions & 1 deletion clippy_lints/src/casts/ptr_cast_constness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use clippy_utils::sugg::Sugg;
use rustc_errors::Applicability;
use rustc_hir::{Expr, Mutability};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty};
use rustc_middle::ty::{self, Ty, TypeVisitableExt};

use super::PTR_CAST_CONSTNESS;

Expand All @@ -24,6 +24,7 @@ pub(super) fn check<'tcx>(
(Mutability::Not, Mutability::Mut) | (Mutability::Mut, Mutability::Not)
)
&& from_ty == to_ty
&& !from_ty.has_erased_regions()
{
let sugg = Sugg::hir(cx, cast_expr, "_");
let constness = match *to_mutbl {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/transmute/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ impl<'tcx> LateLintPass<'tcx> for Transmute {
| missing_transmute_annotations::check(cx, path, from_ty, to_ty, e.hir_id)
| transmute_int_to_char::check(cx, e, from_ty, to_ty, arg, const_context)
| transmute_ref_to_ref::check(cx, e, from_ty, to_ty, arg, const_context)
| transmute_ptr_to_ptr::check(cx, e, from_ty, to_ty, arg)
| transmute_ptr_to_ptr::check(cx, e, from_ty, to_ty, arg, &self.msrv)
| transmute_int_to_bool::check(cx, e, from_ty, to_ty, arg)
| transmute_int_to_float::check(cx, e, from_ty, to_ty, arg, const_context)
| transmute_int_to_non_zero::check(cx, e, from_ty, to_ty, arg)
Expand Down
43 changes: 38 additions & 5 deletions clippy_lints/src/transmute/transmute_ptr_to_ptr.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use super::TRANSMUTE_PTR_TO_PTR;
use clippy_config::msrvs::{self, Msrv};
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::sugg;
use rustc_errors::Applicability;
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty};
use rustc_middle::ty::{self, Ty, TypeVisitableExt};

/// Checks for `transmute_ptr_to_ptr` lint.
/// Returns `true` if it's triggered, otherwise returns `false`.
Expand All @@ -14,18 +15,50 @@ pub(super) fn check<'tcx>(
from_ty: Ty<'tcx>,
to_ty: Ty<'tcx>,
arg: &'tcx Expr<'_>,
msrv: &Msrv,
) -> bool {
match (&from_ty.kind(), &to_ty.kind()) {
(ty::RawPtr(_, _), ty::RawPtr(to_ty, to_mutbl)) => {
match (from_ty.kind(), to_ty.kind()) {
(ty::RawPtr(from_pointee_ty, from_mutbl), ty::RawPtr(to_pointee_ty, to_mutbl)) => {
span_lint_and_then(
cx,
TRANSMUTE_PTR_TO_PTR,
e.span,
"transmute from a pointer to a pointer",
|diag| {
if let Some(arg) = sugg::Sugg::hir_opt(cx, arg) {
let sugg = arg.as_ty(Ty::new_ptr(cx.tcx, *to_ty, *to_mutbl));
diag.span_suggestion(e.span, "try", sugg, Applicability::Unspecified);
if from_mutbl == to_mutbl
&& to_pointee_ty.is_sized(cx.tcx, cx.param_env)
&& msrv.meets(msrvs::POINTER_CAST)
{
diag.span_suggestion_verbose(
e.span,
"use `pointer::cast` instead",
format!("{}.cast::<{to_pointee_ty}>()", arg.maybe_par()),
Applicability::MaybeIncorrect,
);
} else if from_pointee_ty == to_pointee_ty
&& let Some(method) = match (from_mutbl, to_mutbl) {
(ty::Mutability::Not, ty::Mutability::Mut) => Some("cast_mut"),
(ty::Mutability::Mut, ty::Mutability::Not) => Some("cast_const"),
_ => None,
}
&& !from_pointee_ty.has_erased_regions()
&& msrv.meets(msrvs::POINTER_CAST_CONSTNESS)
{
diag.span_suggestion_verbose(
e.span,
format!("use `pointer::{method}` instead"),
format!("{}.{method}()", arg.maybe_par()),
Applicability::MaybeIncorrect,
);
} else {
diag.span_suggestion_verbose(
e.span,
"use an `as` cast instead",
arg.as_ty(to_ty),
Applicability::MaybeIncorrect,
);
}
}
},
);
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/ptr_cast_constness.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ fn main() {
let _ = external!($ptr as *const u32);
}

fn lifetime_to_static(v: *mut &()) -> *const &'static () {
v as _
}

#[clippy::msrv = "1.64"]
fn _msrv_1_64() {
let ptr: *const u32 = &42_u32;
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/ptr_cast_constness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ fn main() {
let _ = external!($ptr as *const u32);
}

fn lifetime_to_static(v: *mut &()) -> *const &'static () {
v as _
}

#[clippy::msrv = "1.64"]
fn _msrv_1_64() {
let ptr: *const u32 = &42_u32;
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/ptr_cast_constness.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ LL | let _ = mut_ptr as *const u32;
| ^^^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast_const`, a safer alternative: `mut_ptr.cast_const()`

error: `as` casting between raw pointers while changing only its constness
--> tests/ui/ptr_cast_constness.rs:64:13
--> tests/ui/ptr_cast_constness.rs:68:13
|
LL | let _ = ptr as *mut u32;
| ^^^^^^^^^^^^^^^ help: try `pointer::cast_mut`, a safer alternative: `ptr.cast_mut()`

error: `as` casting between raw pointers while changing only its constness
--> tests/ui/ptr_cast_constness.rs:65:13
--> tests/ui/ptr_cast_constness.rs:69:13
|
LL | let _ = mut_ptr as *const u32;
| ^^^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast_const`, a safer alternative: `mut_ptr.cast_const()`
Expand Down
79 changes: 56 additions & 23 deletions tests/ui/transmute_ptr_to_ptr.fixed
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
#![warn(clippy::transmute_ptr_to_ptr)]
#![allow(clippy::borrow_as_ptr, clippy::missing_transmute_annotations)]

use std::mem::transmute;

// Make sure we can modify lifetimes, which is one of the recommended uses
// of transmute

// Make sure we can do static lifetime transmutes
unsafe fn transmute_lifetime_to_static<'a, T>(t: &'a T) -> &'static T {
std::mem::transmute::<&'a T, &'static T>(t)
transmute::<&'a T, &'static T>(t)
}

// Make sure we can do non-static lifetime transmutes
unsafe fn transmute_lifetime<'a, 'b, T>(t: &'a T, u: &'b T) -> &'b T {
std::mem::transmute::<&'a T, &'b T>(t)
transmute::<&'a T, &'b T>(t)
}

struct LifetimeParam<'a> {
Expand All @@ -27,47 +29,78 @@ fn transmute_ptr_to_ptr() {
let mut_ptr = &mut 1u32 as *mut u32;
unsafe {
// pointer-to-pointer transmutes; bad
let _: *const f32 = ptr as *const f32;
//~^ ERROR: transmute from a pointer to a pointer
//~| NOTE: `-D clippy::transmute-ptr-to-ptr` implied by `-D warnings`
let _: *mut f32 = mut_ptr as *mut f32;
//~^ ERROR: transmute from a pointer to a pointer
let _: *const f32 = ptr.cast::<f32>();
//~^ transmute_ptr_to_ptr
let _: *mut f32 = mut_ptr.cast::<f32>();
//~^ transmute_ptr_to_ptr
// ref-ref transmutes; bad
let _: &f32 = &*(&1u32 as *const u32 as *const f32);
//~^ ERROR: transmute from a reference to a reference
//~^ transmute_ptr_to_ptr
let _: &f32 = &*(&1f64 as *const f64 as *const f32);
//~^ ERROR: transmute from a reference to a reference
//~^ transmute_ptr_to_ptr
//:^ this test is here because both f32 and f64 are the same TypeVariant, but they are not
// the same type
let _: &mut f32 = &mut *(&mut 1u32 as *mut u32 as *mut f32);
//~^ ERROR: transmute from a reference to a reference
//~^ transmute_ptr_to_ptr
let _: &GenericParam<f32> = &*(&GenericParam { t: 1u32 } as *const GenericParam<u32> as *const GenericParam<f32>);
//~^ ERROR: transmute from a reference to a reference
//~^ transmute_ptr_to_ptr
let u64_ref: &u64 = &0u64;
let u8_ref: &u8 = unsafe { &*(u64_ref as *const u64 as *const u8) };
//~^ ERROR: transmute from a reference to a reference
let u8_ref: &u8 = &*(u64_ref as *const u64 as *const u8);
//~^ transmute_ptr_to_ptr
let _: *const u32 = mut_ptr.cast_const();
//~^ transmute_ptr_to_ptr
let _: *mut u32 = ptr.cast_mut();
//~^ transmute_ptr_to_ptr
}

// these are recommendations for solving the above; if these lint we need to update
// those suggestions
let _ = ptr as *const f32;
let _ = mut_ptr as *mut f32;
let _ = unsafe { &*(&1u32 as *const u32 as *const f32) };
let _ = unsafe { &mut *(&mut 1u32 as *mut u32 as *mut f32) };

// transmute internal lifetimes, should not lint
let s = "hello world".to_owned();
let lp = LifetimeParam { s: &s };
let _: &LifetimeParam<'static> = unsafe { std::mem::transmute(&lp) };
let _: &GenericParam<&LifetimeParam<'static>> = unsafe { std::mem::transmute(&GenericParam { t: &lp }) };
let _: &LifetimeParam<'static> = unsafe { transmute(&lp) };
let _: &GenericParam<&LifetimeParam<'static>> = unsafe { transmute(&GenericParam { t: &lp }) };
}

fn lifetime_to_static(v: *mut &()) -> *const &'static () {
unsafe { v as *const &() }
//~^ transmute_ptr_to_ptr
}

// dereferencing raw pointers in const contexts, should not lint as it's unstable (issue 5959)
const _: &() = {
struct Zst;
let zst = &Zst;

unsafe { std::mem::transmute::<&'static Zst, &'static ()>(zst) }
unsafe { transmute::<&'static Zst, &'static ()>(zst) }
};

#[clippy::msrv = "1.37"]
fn msrv_1_37(ptr: *const u8) {
unsafe {
let _: *const i8 = ptr as *const i8;
}
}

#[clippy::msrv = "1.38"]
fn msrv_1_38(ptr: *const u8) {
unsafe {
let _: *const i8 = ptr.cast::<i8>();
}
}

#[clippy::msrv = "1.64"]
fn msrv_1_64(ptr: *const u8, mut_ptr: *mut u8) {
unsafe {
let _: *mut u8 = ptr as *mut u8;
let _: *const u8 = mut_ptr as *const u8;
}
}

#[clippy::msrv = "1.65"]
fn msrv_1_65(ptr: *const u8, mut_ptr: *mut u8) {
unsafe {
let _: *mut u8 = ptr.cast_mut();
let _: *const u8 = mut_ptr.cast_const();
}
}

fn main() {}
87 changes: 60 additions & 27 deletions tests/ui/transmute_ptr_to_ptr.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
#![warn(clippy::transmute_ptr_to_ptr)]
#![allow(clippy::borrow_as_ptr, clippy::missing_transmute_annotations)]

use std::mem::transmute;

// Make sure we can modify lifetimes, which is one of the recommended uses
// of transmute

// Make sure we can do static lifetime transmutes
unsafe fn transmute_lifetime_to_static<'a, T>(t: &'a T) -> &'static T {
std::mem::transmute::<&'a T, &'static T>(t)
transmute::<&'a T, &'static T>(t)
}

// Make sure we can do non-static lifetime transmutes
unsafe fn transmute_lifetime<'a, 'b, T>(t: &'a T, u: &'b T) -> &'b T {
std::mem::transmute::<&'a T, &'b T>(t)
transmute::<&'a T, &'b T>(t)
}

struct LifetimeParam<'a> {
Expand All @@ -27,47 +29,78 @@ fn transmute_ptr_to_ptr() {
let mut_ptr = &mut 1u32 as *mut u32;
unsafe {
// pointer-to-pointer transmutes; bad
let _: *const f32 = std::mem::transmute(ptr);
//~^ ERROR: transmute from a pointer to a pointer
//~| NOTE: `-D clippy::transmute-ptr-to-ptr` implied by `-D warnings`
let _: *mut f32 = std::mem::transmute(mut_ptr);
//~^ ERROR: transmute from a pointer to a pointer
let _: *const f32 = transmute(ptr);
//~^ transmute_ptr_to_ptr
let _: *mut f32 = transmute(mut_ptr);
//~^ transmute_ptr_to_ptr
// ref-ref transmutes; bad
let _: &f32 = std::mem::transmute(&1u32);
//~^ ERROR: transmute from a reference to a reference
let _: &f32 = std::mem::transmute(&1f64);
//~^ ERROR: transmute from a reference to a reference
let _: &f32 = transmute(&1u32);
//~^ transmute_ptr_to_ptr
let _: &f32 = transmute(&1f64);
//~^ transmute_ptr_to_ptr
//:^ this test is here because both f32 and f64 are the same TypeVariant, but they are not
// the same type
let _: &mut f32 = std::mem::transmute(&mut 1u32);
//~^ ERROR: transmute from a reference to a reference
let _: &GenericParam<f32> = std::mem::transmute(&GenericParam { t: 1u32 });
//~^ ERROR: transmute from a reference to a reference
let _: &mut f32 = transmute(&mut 1u32);
//~^ transmute_ptr_to_ptr
let _: &GenericParam<f32> = transmute(&GenericParam { t: 1u32 });
//~^ transmute_ptr_to_ptr
let u64_ref: &u64 = &0u64;
let u8_ref: &u8 = unsafe { std::mem::transmute(u64_ref) };
//~^ ERROR: transmute from a reference to a reference
let u8_ref: &u8 = transmute(u64_ref);
//~^ transmute_ptr_to_ptr
let _: *const u32 = transmute(mut_ptr);
//~^ transmute_ptr_to_ptr
let _: *mut u32 = transmute(ptr);
//~^ transmute_ptr_to_ptr
}

// these are recommendations for solving the above; if these lint we need to update
// those suggestions
let _ = ptr as *const f32;
let _ = mut_ptr as *mut f32;
let _ = unsafe { &*(&1u32 as *const u32 as *const f32) };
let _ = unsafe { &mut *(&mut 1u32 as *mut u32 as *mut f32) };

// transmute internal lifetimes, should not lint
let s = "hello world".to_owned();
let lp = LifetimeParam { s: &s };
let _: &LifetimeParam<'static> = unsafe { std::mem::transmute(&lp) };
let _: &GenericParam<&LifetimeParam<'static>> = unsafe { std::mem::transmute(&GenericParam { t: &lp }) };
let _: &LifetimeParam<'static> = unsafe { transmute(&lp) };
let _: &GenericParam<&LifetimeParam<'static>> = unsafe { transmute(&GenericParam { t: &lp }) };
}

fn lifetime_to_static(v: *mut &()) -> *const &'static () {
unsafe { transmute(v) }
//~^ transmute_ptr_to_ptr
}

// dereferencing raw pointers in const contexts, should not lint as it's unstable (issue 5959)
const _: &() = {
struct Zst;
let zst = &Zst;

unsafe { std::mem::transmute::<&'static Zst, &'static ()>(zst) }
unsafe { transmute::<&'static Zst, &'static ()>(zst) }
};

#[clippy::msrv = "1.37"]
fn msrv_1_37(ptr: *const u8) {
unsafe {
let _: *const i8 = transmute(ptr);
}
}

#[clippy::msrv = "1.38"]
fn msrv_1_38(ptr: *const u8) {
unsafe {
let _: *const i8 = transmute(ptr);
}
}

#[clippy::msrv = "1.64"]
fn msrv_1_64(ptr: *const u8, mut_ptr: *mut u8) {
unsafe {
let _: *mut u8 = transmute(ptr);
let _: *const u8 = transmute(mut_ptr);
}
}

#[clippy::msrv = "1.65"]
fn msrv_1_65(ptr: *const u8, mut_ptr: *mut u8) {
unsafe {
let _: *mut u8 = transmute(ptr);
let _: *const u8 = transmute(mut_ptr);
}
}

fn main() {}
Loading

0 comments on commit e91d82e

Please sign in to comment.