Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Suggest .cast/.cast_const/.cast_mut in transmute_ptr_as_ptr #13143

Merged
merged 2 commits into from
Jul 23, 2024
Merged

Suggest .cast/.cast_const/.cast_mut in transmute_ptr_as_ptr #13143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
afe811a
Suggest `.cast`/`.cast_const`/`.cast_mut` in `transmute_ptr_as_ptr`
Alexendoo Jul 22, 2024
7010d3c
Don't suggest `.cast_mut`/`.cast_const` for types with erased lifetimes
Alexendoo Jul 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion clippy_lints/src/casts/ptr_cast_constness.rs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could end up as a wrong suggestion for lifetime transmutes (*mut Lt<'a> to *const Lt<'static>). Maybe we can add a check if the from- or to type has TypeFlags::RE_ERASED?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, is there a good example? I assume this would also effect ptr_as_ptr

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't think of a good (practical) example, but I was thinking of something like:

struct Lt<'a>(&'a ());

unsafe fn f(v: *mut Lt<'_>) -> *const Lt<'static> {
  std::mem::transmute(v)
}

This looks like it has .cast_const() for the suggestion, but that doesn't change the lifetime

Basically just this test case but with raw pointers and with differing mutability (so it doesn't go into the first if for pointer::cast):

rust-clippy/tests/ui/transmute_ptr_to_ptr.fixed

Line 10 in afe811a

unsafe fn transmute_lifetime_to_static<'a, T>(t: &'a T) -> &'static T {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, unrealistic is fine I just can never remember which lifetimes are erased or early/late or any of that stuff 😅

Fixed it for ptr_cast_constness too

&& 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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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