Skip to content

Commit

Permalink
Suggest pointer::cast when possible in transmute_ptr_to_ref
Browse files Browse the repository at this point in the history
Defensively add a cast to any type with lifetimes.
  • Loading branch information
Jarcho committed Jun 4, 2022
1 parent ebd357e commit 20e0273
Show file tree
Hide file tree
Showing 8 changed files with 266 additions and 66 deletions.
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(borrow_deref_ref::BorrowDerefRef));
store.register_late_pass(|| Box::new(no_effect::NoEffect));
store.register_late_pass(|| Box::new(temporary_assignment::TemporaryAssignment));
store.register_late_pass(|| Box::new(transmute::Transmute));
store.register_late_pass(move || Box::new(transmute::Transmute::new(msrv)));
let cognitive_complexity_threshold = conf.cognitive_complexity_threshold;
store.register_late_pass(move || {
Box::new(cognitive_complexity::CognitiveComplexity::new(
Expand Down
25 changes: 18 additions & 7 deletions clippy_lints/src/transmute/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ mod wrong_transmute;

use clippy_utils::in_constant;
use if_chain::if_chain;
use rustc_hir::{Expr, ExprKind};
use rustc_hir::{Expr, ExprKind, QPath};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::symbol::sym;

declare_clippy_lint! {
Expand Down Expand Up @@ -385,7 +386,10 @@ declare_clippy_lint! {
"transmute to or from a type with an undefined representation"
}

declare_lint_pass!(Transmute => [
pub struct Transmute {
msrv: Option<RustcVersion>,
}
impl_lint_pass!(Transmute => [
CROSSPOINTER_TRANSMUTE,
TRANSMUTE_PTR_TO_REF,
TRANSMUTE_PTR_TO_PTR,
Expand All @@ -401,13 +405,18 @@ declare_lint_pass!(Transmute => [
TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS,
TRANSMUTE_UNDEFINED_REPR,
]);

impl Transmute {
#[must_use]
pub fn new(msrv: Option<RustcVersion>) -> Self {
Self { msrv }
}
}
impl<'tcx> LateLintPass<'tcx> for Transmute {
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
if_chain! {
if let ExprKind::Call(path_expr, [arg]) = e.kind;
if let ExprKind::Path(ref qpath) = path_expr.kind;
if let Some(def_id) = cx.qpath_res(qpath, path_expr.hir_id).opt_def_id();
if let ExprKind::Path(QPath::Resolved(None, path)) = path_expr.kind;
if let Some(def_id) = path.res.opt_def_id();
if cx.tcx.is_diagnostic_item(sym::transmute, def_id);
then {
// Avoid suggesting non-const operations in const contexts:
Expand All @@ -427,7 +436,7 @@ impl<'tcx> LateLintPass<'tcx> for Transmute {

let linted = wrong_transmute::check(cx, e, from_ty, to_ty)
| crosspointer_transmute::check(cx, e, from_ty, to_ty)
| transmute_ptr_to_ref::check(cx, e, from_ty, to_ty, arg, qpath)
| transmute_ptr_to_ref::check(cx, e, from_ty, to_ty, arg, path, self.msrv)
| 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)
Expand All @@ -446,4 +455,6 @@ impl<'tcx> LateLintPass<'tcx> for Transmute {
}
}
}

extract_msrv_attr!(LateContext);
}
56 changes: 42 additions & 14 deletions clippy_lints/src/transmute/transmute_ptr_to_ref.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use super::utils::get_type_snippet;
use super::TRANSMUTE_PTR_TO_REF;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{meets_msrv, msrvs, sugg};
use rustc_errors::Applicability;
use rustc_hir::{Expr, Mutability, QPath};
use rustc_hir::{self as hir, Expr, GenericArg, Mutability, Path, TyKind};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty};
use rustc_middle::ty::{self, Ty, TypeFoldable};
use rustc_semver::RustcVersion;

/// Checks for `transmute_ptr_to_ref` lint.
/// Returns `true` if it's triggered, otherwise returns `false`.
Expand All @@ -15,7 +16,8 @@ pub(super) fn check<'tcx>(
from_ty: Ty<'tcx>,
to_ty: Ty<'tcx>,
arg: &'tcx Expr<'_>,
qpath: &'tcx QPath<'_>,
path: &'tcx Path<'_>,
msrv: Option<RustcVersion>,
) -> bool {
match (&from_ty.kind(), &to_ty.kind()) {
(ty::RawPtr(from_ptr_ty), ty::Ref(_, to_ref_ty, mutbl)) => {
Expand All @@ -34,23 +36,49 @@ pub(super) fn check<'tcx>(
} else {
("&*", "*const")
};
let mut app = Applicability::MachineApplicable;

let arg = if from_ptr_ty.ty == *to_ref_ty {
arg
let sugg = if let Some(ty) = get_explicit_type(path) {
let ty_snip = snippet_with_applicability(cx, ty.span, "..", &mut app);
if meets_msrv(msrv, msrvs::PTR_CAST) {
format!("{}{}.cast::<{}>()", deref, arg.maybe_par(), ty_snip)
} else if from_ptr_ty.has_erased_regions() {
sugg::make_unop(deref, arg.as_ty(format!("{} () as {} {}", cast, cast, ty_snip)))
.to_string()
} else {
sugg::make_unop(deref, arg.as_ty(format!("{} {}", cast, ty_snip))).to_string()
}
} else if from_ptr_ty.ty == *to_ref_ty {
if from_ptr_ty.has_erased_regions() {
if meets_msrv(msrv, msrvs::PTR_CAST) {
format!("{}{}.cast::<{}>()", deref, arg.maybe_par(), to_ref_ty)
} else {
sugg::make_unop(deref, arg.as_ty(format!("{} () as {} {}", cast, cast, to_ref_ty)))
.to_string()
}
} else {
sugg::make_unop(deref, arg).to_string()
}
} else {
arg.as_ty(&format!("{} {}", cast, get_type_snippet(cx, qpath, *to_ref_ty)))
sugg::make_unop(deref, arg.as_ty(format!("{} {}", cast, to_ref_ty))).to_string()
};

diag.span_suggestion(
e.span,
"try",
sugg::make_unop(deref, arg).to_string(),
Applicability::Unspecified,
);
diag.span_suggestion(e.span, "try", sugg, app);
},
);
true
},
_ => false,
}
}

/// Gets the type `Bar` in `…::transmute<Foo, &Bar>`.
fn get_explicit_type<'tcx>(path: &'tcx Path<'tcx>) -> Option<&'tcx hir::Ty<'tcx>> {
if let GenericArg::Type(ty) = path.segments.last()?.args?.args.get(1)?
&& let TyKind::Rptr(_, ty) = &ty.kind
{
Some(ty.ty)
} else {
None
}
}
28 changes: 1 addition & 27 deletions clippy_lints/src/transmute/utils.rs
Original file line number Diff line number Diff line change
@@ -1,35 +1,9 @@
use clippy_utils::last_path_segment;
use clippy_utils::source::snippet;
use if_chain::if_chain;
use rustc_hir::{Expr, GenericArg, QPath, TyKind};
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_middle::ty::{cast::CastKind, Ty};
use rustc_span::DUMMY_SP;
use rustc_typeck::check::{cast::CastCheck, FnCtxt, Inherited};

/// Gets the snippet of `Bar` in `…::transmute<Foo, &Bar>`. If that snippet is
/// not available , use
/// the type's `ToString` implementation. In weird cases it could lead to types
/// with invalid `'_`
/// lifetime, but it should be rare.
pub(super) fn get_type_snippet(cx: &LateContext<'_>, path: &QPath<'_>, to_ref_ty: Ty<'_>) -> String {
let seg = last_path_segment(path);
if_chain! {
if let Some(params) = seg.args;
if !params.parenthesized;
if let Some(to_ty) = params.args.iter().filter_map(|arg| match arg {
GenericArg::Type(ty) => Some(ty),
_ => None,
}).nth(1);
if let TyKind::Rptr(_, ref to_ty) = to_ty.kind;
then {
return snippet(cx, to_ty.ty.span, &to_ref_ty.to_string()).to_string();
}
}

to_ref_ty.to_string()
}

// check if the component types of the transmuted collection and the result have different ABI,
// size or alignment
pub(super) fn is_layout_incompatible<'tcx>(cx: &LateContext<'tcx>, from: Ty<'tcx>, to: Ty<'tcx>) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ msrv_aliases! {
1,40,0 { MEM_TAKE, NON_EXHAUSTIVE, OPTION_AS_DEREF }
1,38,0 { POINTER_CAST }
1,37,0 { TYPE_ALIAS_ENUM_VARIANTS }
1,36,0 { ITERATOR_COPIED }
1,36,0 { ITERATOR_COPIED, PTR_CAST }
1,35,0 { OPTION_COPIED, RANGE_CONTAINS }
1,34,0 { TRY_FROM }
1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES }
Expand Down
78 changes: 78 additions & 0 deletions tests/ui/transmute_ptr_to_ref.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// run-rustfix

#![feature(custom_inner_attributes)]
#![warn(clippy::transmute_ptr_to_ref)]
#![allow(clippy::match_single_binding)]

unsafe fn _ptr_to_ref<T, U>(p: *const T, m: *mut T, o: *const U, om: *mut U) {
let _: &T = &*p;
let _: &T = &*p;

let _: &mut T = &mut *m;
let _: &mut T = &mut *m;

let _: &T = &*m;
let _: &T = &*m;

let _: &mut T = &mut *(p as *mut T);
let _ = &mut *(p as *mut T);

let _: &T = &*(o as *const T);
let _: &T = &*(o as *const T);

let _: &mut T = &mut *(om as *mut T);
let _: &mut T = &mut *(om as *mut T);

let _: &T = &*(om as *const T);
let _: &T = &*(om as *const T);
}

fn _issue1231() {
struct Foo<'a, T> {
bar: &'a T,
}

let raw = 42 as *const i32;
let _: &Foo<u8> = unsafe { &*raw.cast::<Foo<_>>() };

let _: &Foo<&u8> = unsafe { &*raw.cast::<Foo<&_>>() };

type Bar<'a> = &'a u8;
let raw = 42 as *const i32;
unsafe { &*(raw as *const u8) };
}

unsafe fn _issue8924<'a, 'b, 'c>(x: *const &'a u32, y: *const &'b u32) -> &'c &'b u32 {
match 0 {
0 => &*x.cast::<&u32>(),
1 => &*y.cast::<&u32>(),
2 => &*x.cast::<&'b u32>(),
_ => &*y.cast::<&'b u32>(),
}
}

unsafe fn _meets_msrv<'a, 'b, 'c>(x: *const &'a u32) -> &'c &'b u32 {
#![clippy::msrv = "1.36"]
let a = 0u32;
let a = &a as *const u32;
let _: &u32 = &*a;
let _: &u32 = &*a.cast::<u32>();
match 0 {
0 => &*x.cast::<&u32>(),
_ => &*x.cast::<&'b u32>(),
}
}

unsafe fn _under_msrv<'a, 'b, 'c>(x: *const &'a u32) -> &'c &'b u32 {
#![clippy::msrv = "1.35"]
let a = 0u32;
let a = &a as *const u32;
let _: &u32 = &*a;
let _: &u32 = &*(a as *const u32);
match 0 {
0 => &*(x as *const () as *const &u32),
_ => &*(x as *const () as *const &'b u32),
}
}

fn main() {}
39 changes: 38 additions & 1 deletion tests/ui/transmute_ptr_to_ref.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
// run-rustfix

#![feature(custom_inner_attributes)]
#![warn(clippy::transmute_ptr_to_ref)]
#![allow(clippy::match_single_binding)]

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 All @@ -23,7 +27,7 @@ unsafe fn _ptr_to_ref<T, U>(p: *const T, m: *mut T, o: *const U, om: *mut U) {
let _: &T = &*(om as *const T);
}

fn issue1231() {
fn _issue1231() {
struct Foo<'a, T> {
bar: &'a T,
}
Expand All @@ -38,4 +42,37 @@ fn issue1231() {
unsafe { std::mem::transmute::<_, Bar>(raw) };
}

unsafe fn _issue8924<'a, 'b, 'c>(x: *const &'a u32, y: *const &'b u32) -> &'c &'b u32 {
match 0 {
0 => std::mem::transmute(x),
1 => std::mem::transmute(y),
2 => std::mem::transmute::<_, &&'b u32>(x),
_ => std::mem::transmute::<_, &&'b u32>(y),
}
}

unsafe fn _meets_msrv<'a, 'b, 'c>(x: *const &'a u32) -> &'c &'b u32 {
#![clippy::msrv = "1.36"]
let a = 0u32;
let a = &a as *const u32;
let _: &u32 = std::mem::transmute(a);
let _: &u32 = std::mem::transmute::<_, &u32>(a);
match 0 {
0 => std::mem::transmute(x),
_ => std::mem::transmute::<_, &&'b u32>(x),
}
}

unsafe fn _under_msrv<'a, 'b, 'c>(x: *const &'a u32) -> &'c &'b u32 {
#![clippy::msrv = "1.35"]
let a = 0u32;
let a = &a as *const u32;
let _: &u32 = std::mem::transmute(a);
let _: &u32 = std::mem::transmute::<_, &u32>(a);
match 0 {
0 => std::mem::transmute(x),
_ => std::mem::transmute::<_, &&'b u32>(x),
}
}

fn main() {}
Loading

0 comments on commit 20e0273

Please sign in to comment.