From 8ef87455b07e527dd636423751591910dac2e166 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Tue, 1 Feb 2022 10:53:25 -0500 Subject: [PATCH 1/3] Minor cleanup on transmute lints --- clippy_lints/src/transmute/mod.rs | 31 +++++++++---------- .../src/transmute/transmute_float_to_int.rs | 4 +-- .../src/transmute/transmute_int_to_bool.rs | 4 +-- .../src/transmute/transmute_int_to_char.rs | 4 +-- .../src/transmute/transmute_int_to_float.rs | 4 +-- .../src/transmute/transmute_num_to_bytes.rs | 4 +-- .../src/transmute/transmute_ptr_to_ptr.rs | 4 +-- .../src/transmute/transmute_ptr_to_ref.rs | 4 +-- .../src/transmute/transmute_ref_to_ref.rs | 6 ++-- .../transmutes_expressible_as_ptr_casts.rs | 4 +-- .../src/transmute/useless_transmute.rs | 6 ++-- 11 files changed, 37 insertions(+), 38 deletions(-) diff --git a/clippy_lints/src/transmute/mod.rs b/clippy_lints/src/transmute/mod.rs index 3ad4ec74b..c370941dd 100644 --- a/clippy_lints/src/transmute/mod.rs +++ b/clippy_lints/src/transmute/mod.rs @@ -372,10 +372,9 @@ declare_lint_pass!(Transmute => [ ]); impl<'tcx> LateLintPass<'tcx> for Transmute { - #[allow(clippy::similar_names, clippy::too_many_lines)] fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { if_chain! { - if let ExprKind::Call(path_expr, args) = e.kind; + 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 cx.tcx.is_diagnostic_item(sym::transmute, def_id); @@ -385,28 +384,28 @@ impl<'tcx> LateLintPass<'tcx> for Transmute { // And see https://github.com/rust-lang/rust/issues/51911 for dereferencing raw pointers. let const_context = in_constant(cx, e.hir_id); - let from_ty = cx.typeck_results().expr_ty(&args[0]); + let from_ty = cx.typeck_results().expr_ty(arg); let to_ty = cx.typeck_results().expr_ty(e); // If useless_transmute is triggered, the other lints can be skipped. - if useless_transmute::check(cx, e, from_ty, to_ty, args) { + if useless_transmute::check(cx, e, from_ty, to_ty, arg) { return; } - let mut linted = wrong_transmute::check(cx, e, from_ty, to_ty); - linted |= crosspointer_transmute::check(cx, e, from_ty, to_ty); - linted |= transmute_ptr_to_ref::check(cx, e, from_ty, to_ty, args, qpath); - linted |= transmute_int_to_char::check(cx, e, from_ty, to_ty, args); - linted |= transmute_ref_to_ref::check(cx, e, from_ty, to_ty, args, const_context); - linted |= transmute_ptr_to_ptr::check(cx, e, from_ty, to_ty, args); - linted |= transmute_int_to_bool::check(cx, e, from_ty, to_ty, args); - linted |= transmute_int_to_float::check(cx, e, from_ty, to_ty, args, const_context); - linted |= transmute_float_to_int::check(cx, e, from_ty, to_ty, args, const_context); - linted |= transmute_num_to_bytes::check(cx, e, from_ty, to_ty, args, const_context); - linted |= unsound_collection_transmute::check(cx, e, from_ty, to_ty); + 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_int_to_char::check(cx, e, from_ty, to_ty, arg) + | 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_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_float_to_int::check(cx, e, from_ty, to_ty, arg, const_context) + | transmute_num_to_bytes::check(cx, e, from_ty, to_ty, arg, const_context) + | unsound_collection_transmute::check(cx, e, from_ty, to_ty); if !linted { - transmutes_expressible_as_ptr_casts::check(cx, e, from_ty, to_ty, args); + transmutes_expressible_as_ptr_casts::check(cx, e, from_ty, to_ty, arg); } } } diff --git a/clippy_lints/src/transmute/transmute_float_to_int.rs b/clippy_lints/src/transmute/transmute_float_to_int.rs index 3aa3c393b..f4656283b 100644 --- a/clippy_lints/src/transmute/transmute_float_to_int.rs +++ b/clippy_lints/src/transmute/transmute_float_to_int.rs @@ -15,7 +15,7 @@ pub(super) fn check<'tcx>( e: &'tcx Expr<'_>, from_ty: Ty<'tcx>, to_ty: Ty<'tcx>, - args: &'tcx [Expr<'_>], + arg: &'tcx Expr<'_>, const_context: bool, ) -> bool { match (&from_ty.kind(), &to_ty.kind()) { @@ -26,7 +26,7 @@ pub(super) fn check<'tcx>( e.span, &format!("transmute from a `{}` to a `{}`", from_ty, to_ty), |diag| { - let mut expr = &args[0]; + let mut expr = arg; let mut arg = sugg::Sugg::hir(cx, expr, ".."); if let ExprKind::Unary(UnOp::Neg, inner_expr) = &expr.kind { diff --git a/clippy_lints/src/transmute/transmute_int_to_bool.rs b/clippy_lints/src/transmute/transmute_int_to_bool.rs index cc0a5643e..8c50b58ca 100644 --- a/clippy_lints/src/transmute/transmute_int_to_bool.rs +++ b/clippy_lints/src/transmute/transmute_int_to_bool.rs @@ -15,7 +15,7 @@ pub(super) fn check<'tcx>( e: &'tcx Expr<'_>, from_ty: Ty<'tcx>, to_ty: Ty<'tcx>, - args: &'tcx [Expr<'_>], + arg: &'tcx Expr<'_>, ) -> bool { match (&from_ty.kind(), &to_ty.kind()) { (ty::Int(ty::IntTy::I8) | ty::Uint(ty::UintTy::U8), ty::Bool) => { @@ -25,7 +25,7 @@ pub(super) fn check<'tcx>( e.span, &format!("transmute from a `{}` to a `bool`", from_ty), |diag| { - let arg = sugg::Sugg::hir(cx, &args[0], ".."); + let arg = sugg::Sugg::hir(cx, arg, ".."); let zero = sugg::Sugg::NonParen(Cow::from("0")); diag.span_suggestion( e.span, diff --git a/clippy_lints/src/transmute/transmute_int_to_char.rs b/clippy_lints/src/transmute/transmute_int_to_char.rs index e83d2e06b..3eb07b689 100644 --- a/clippy_lints/src/transmute/transmute_int_to_char.rs +++ b/clippy_lints/src/transmute/transmute_int_to_char.rs @@ -14,7 +14,7 @@ pub(super) fn check<'tcx>( e: &'tcx Expr<'_>, from_ty: Ty<'tcx>, to_ty: Ty<'tcx>, - args: &'tcx [Expr<'_>], + arg: &'tcx Expr<'_>, ) -> bool { match (&from_ty.kind(), &to_ty.kind()) { (ty::Int(ty::IntTy::I32) | ty::Uint(ty::UintTy::U32), &ty::Char) => { @@ -24,7 +24,7 @@ pub(super) fn check<'tcx>( e.span, &format!("transmute from a `{}` to a `char`", from_ty), |diag| { - let arg = sugg::Sugg::hir(cx, &args[0], ".."); + let arg = sugg::Sugg::hir(cx, arg, ".."); let arg = if let ty::Int(_) = from_ty.kind() { arg.as_ty(ast::UintTy::U32.name_str()) } else { diff --git a/clippy_lints/src/transmute/transmute_int_to_float.rs b/clippy_lints/src/transmute/transmute_int_to_float.rs index 05eee380d..b8703052e 100644 --- a/clippy_lints/src/transmute/transmute_int_to_float.rs +++ b/clippy_lints/src/transmute/transmute_int_to_float.rs @@ -13,7 +13,7 @@ pub(super) fn check<'tcx>( e: &'tcx Expr<'_>, from_ty: Ty<'tcx>, to_ty: Ty<'tcx>, - args: &'tcx [Expr<'_>], + arg: &'tcx Expr<'_>, const_context: bool, ) -> bool { match (&from_ty.kind(), &to_ty.kind()) { @@ -24,7 +24,7 @@ pub(super) fn check<'tcx>( e.span, &format!("transmute from a `{}` to a `{}`", from_ty, to_ty), |diag| { - let arg = sugg::Sugg::hir(cx, &args[0], ".."); + let arg = sugg::Sugg::hir(cx, arg, ".."); let arg = if let ty::Int(int_ty) = from_ty.kind() { arg.as_ty(format!( "u{}", diff --git a/clippy_lints/src/transmute/transmute_num_to_bytes.rs b/clippy_lints/src/transmute/transmute_num_to_bytes.rs index 5ba58a764..52d193d11 100644 --- a/clippy_lints/src/transmute/transmute_num_to_bytes.rs +++ b/clippy_lints/src/transmute/transmute_num_to_bytes.rs @@ -13,7 +13,7 @@ pub(super) fn check<'tcx>( e: &'tcx Expr<'_>, from_ty: Ty<'tcx>, to_ty: Ty<'tcx>, - args: &'tcx [Expr<'_>], + arg: &'tcx Expr<'_>, const_context: bool, ) -> bool { match (&from_ty.kind(), &to_ty.kind()) { @@ -33,7 +33,7 @@ pub(super) fn check<'tcx>( e.span, &format!("transmute from a `{}` to a `{}`", from_ty, to_ty), |diag| { - let arg = sugg::Sugg::hir(cx, &args[0], ".."); + let arg = sugg::Sugg::hir(cx, arg, ".."); diag.span_suggestion( e.span, "consider using `to_ne_bytes()`", diff --git a/clippy_lints/src/transmute/transmute_ptr_to_ptr.rs b/clippy_lints/src/transmute/transmute_ptr_to_ptr.rs index 7b646bfc0..d712b33de 100644 --- a/clippy_lints/src/transmute/transmute_ptr_to_ptr.rs +++ b/clippy_lints/src/transmute/transmute_ptr_to_ptr.rs @@ -13,7 +13,7 @@ pub(super) fn check<'tcx>( e: &'tcx Expr<'_>, from_ty: Ty<'tcx>, to_ty: Ty<'tcx>, - args: &'tcx [Expr<'_>], + arg: &'tcx Expr<'_>, ) -> bool { match (&from_ty.kind(), &to_ty.kind()) { (ty::RawPtr(_), ty::RawPtr(to_ty)) => { @@ -23,7 +23,7 @@ pub(super) fn check<'tcx>( e.span, "transmute from a pointer to a pointer", |diag| { - if let Some(arg) = sugg::Sugg::hir_opt(cx, &args[0]) { + if let Some(arg) = sugg::Sugg::hir_opt(cx, arg) { let sugg = arg.as_ty(cx.tcx.mk_ptr(*to_ty)); diag.span_suggestion(e.span, "try", sugg.to_string(), Applicability::Unspecified); } diff --git a/clippy_lints/src/transmute/transmute_ptr_to_ref.rs b/clippy_lints/src/transmute/transmute_ptr_to_ref.rs index f14eef936..5699f8e92 100644 --- a/clippy_lints/src/transmute/transmute_ptr_to_ref.rs +++ b/clippy_lints/src/transmute/transmute_ptr_to_ref.rs @@ -14,7 +14,7 @@ pub(super) fn check<'tcx>( e: &'tcx Expr<'_>, from_ty: Ty<'tcx>, to_ty: Ty<'tcx>, - args: &'tcx [Expr<'_>], + arg: &'tcx Expr<'_>, qpath: &'tcx QPath<'_>, ) -> bool { match (&from_ty.kind(), &to_ty.kind()) { @@ -28,7 +28,7 @@ pub(super) fn check<'tcx>( from_ty, to_ty ), |diag| { - let arg = sugg::Sugg::hir(cx, &args[0], ".."); + let arg = sugg::Sugg::hir(cx, arg, ".."); let (deref, cast) = if *mutbl == Mutability::Mut { ("&mut *", "*mut") } else { diff --git a/clippy_lints/src/transmute/transmute_ref_to_ref.rs b/clippy_lints/src/transmute/transmute_ref_to_ref.rs index d105e37ab..fdef8bac7 100644 --- a/clippy_lints/src/transmute/transmute_ref_to_ref.rs +++ b/clippy_lints/src/transmute/transmute_ref_to_ref.rs @@ -15,7 +15,7 @@ pub(super) fn check<'tcx>( e: &'tcx Expr<'_>, from_ty: Ty<'tcx>, to_ty: Ty<'tcx>, - args: &'tcx [Expr<'_>], + arg: &'tcx Expr<'_>, const_context: bool, ) -> bool { let mut triggered = false; @@ -41,7 +41,7 @@ pub(super) fn check<'tcx>( format!( "std::str::from_utf8{}({}).unwrap()", postfix, - snippet(cx, args[0].span, ".."), + snippet(cx, arg.span, ".."), ), Applicability::Unspecified, ); @@ -54,7 +54,7 @@ pub(super) fn check<'tcx>( TRANSMUTE_PTR_TO_PTR, e.span, "transmute from a reference to a reference", - |diag| if let Some(arg) = sugg::Sugg::hir_opt(cx, &args[0]) { + |diag| if let Some(arg) = sugg::Sugg::hir_opt(cx, arg) { let ty_from_and_mut = ty::TypeAndMut { ty: ty_from, mutbl: *from_mutbl diff --git a/clippy_lints/src/transmute/transmutes_expressible_as_ptr_casts.rs b/clippy_lints/src/transmute/transmutes_expressible_as_ptr_casts.rs index e2c6d130f..626d7cd46 100644 --- a/clippy_lints/src/transmute/transmutes_expressible_as_ptr_casts.rs +++ b/clippy_lints/src/transmute/transmutes_expressible_as_ptr_casts.rs @@ -14,7 +14,7 @@ pub(super) fn check<'tcx>( e: &'tcx Expr<'_>, from_ty: Ty<'tcx>, to_ty: Ty<'tcx>, - args: &'tcx [Expr<'_>], + arg: &'tcx Expr<'_>, ) -> bool { if can_be_expressed_as_pointer_cast(cx, e, from_ty, to_ty) { span_lint_and_then( @@ -26,7 +26,7 @@ pub(super) fn check<'tcx>( from_ty, to_ty ), |diag| { - if let Some(arg) = sugg::Sugg::hir_opt(cx, &args[0]) { + if let Some(arg) = sugg::Sugg::hir_opt(cx, arg) { let sugg = arg.as_ty(&to_ty.to_string()).to_string(); diag.span_suggestion(e.span, "try", sugg, Applicability::MachineApplicable); } diff --git a/clippy_lints/src/transmute/useless_transmute.rs b/clippy_lints/src/transmute/useless_transmute.rs index 445bcf60f..998f97eb5 100644 --- a/clippy_lints/src/transmute/useless_transmute.rs +++ b/clippy_lints/src/transmute/useless_transmute.rs @@ -13,7 +13,7 @@ pub(super) fn check<'tcx>( e: &'tcx Expr<'_>, from_ty: Ty<'tcx>, to_ty: Ty<'tcx>, - args: &'tcx [Expr<'_>], + arg: &'tcx Expr<'_>, ) -> bool { match (&from_ty.kind(), &to_ty.kind()) { _ if from_ty == to_ty => { @@ -32,7 +32,7 @@ pub(super) fn check<'tcx>( e.span, "transmute from a reference to a pointer", |diag| { - if let Some(arg) = sugg::Sugg::hir_opt(cx, &args[0]) { + if let Some(arg) = sugg::Sugg::hir_opt(cx, arg) { let rty_and_mut = ty::TypeAndMut { ty: rty, mutbl: *rty_mutbl, @@ -57,7 +57,7 @@ pub(super) fn check<'tcx>( e.span, "transmute from an integer to a pointer", |diag| { - if let Some(arg) = sugg::Sugg::hir_opt(cx, &args[0]) { + if let Some(arg) = sugg::Sugg::hir_opt(cx, arg) { diag.span_suggestion( e.span, "try", From 3403b3e7175936f694403767b93803df3e606fc0 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Tue, 1 Feb 2022 14:53:12 -0500 Subject: [PATCH 2/3] Add lint `transumte_undefined_repr` --- CHANGELOG.md | 1 + clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_correctness.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.rs | 1 + clippy_lints/src/transmute/mod.rs | 31 +- .../src/transmute/transmute_undefined_repr.rs | 291 ++++++++++++++++++ tests/ui/crashes/ice-4968.rs | 1 + tests/ui/transmute_undefined_repr.rs | 44 +++ tests/ui/transmute_undefined_repr.stderr | 44 +++ 10 files changed, 415 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/transmute/transmute_undefined_repr.rs create mode 100644 tests/ui/transmute_undefined_repr.rs create mode 100644 tests/ui/transmute_undefined_repr.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 9548366da..7fb31f6d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3304,6 +3304,7 @@ Released 2018-09-13 [`transmute_num_to_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_num_to_bytes [`transmute_ptr_to_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ptr [`transmute_ptr_to_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ref +[`transmute_undefined_repr`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_undefined_repr [`transmutes_expressible_as_ptr_casts`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmutes_expressible_as_ptr_casts [`transmuting_null`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmuting_null [`trivial_regex`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivial_regex diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 4721b7f2b..d93e34e76 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -277,6 +277,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(transmute::TRANSMUTE_INT_TO_FLOAT), LintId::of(transmute::TRANSMUTE_NUM_TO_BYTES), LintId::of(transmute::TRANSMUTE_PTR_TO_REF), + LintId::of(transmute::TRANSMUTE_UNDEFINED_REPR), LintId::of(transmute::UNSOUND_COLLECTION_TRANSMUTE), LintId::of(transmute::WRONG_TRANSMUTE), LintId::of(transmuting_null::TRANSMUTING_NULL), diff --git a/clippy_lints/src/lib.register_correctness.rs b/clippy_lints/src/lib.register_correctness.rs index 4217fd3a3..d013daa8e 100644 --- a/clippy_lints/src/lib.register_correctness.rs +++ b/clippy_lints/src/lib.register_correctness.rs @@ -58,6 +58,7 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve LintId::of(size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT), LintId::of(swap::ALMOST_SWAPPED), LintId::of(to_string_in_display::TO_STRING_IN_DISPLAY), + LintId::of(transmute::TRANSMUTE_UNDEFINED_REPR), LintId::of(transmute::UNSOUND_COLLECTION_TRANSMUTE), LintId::of(transmute::WRONG_TRANSMUTE), LintId::of(transmuting_null::TRANSMUTING_NULL), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index e5e1c052c..a80320a57 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -473,6 +473,7 @@ store.register_lints(&[ transmute::TRANSMUTE_NUM_TO_BYTES, transmute::TRANSMUTE_PTR_TO_PTR, transmute::TRANSMUTE_PTR_TO_REF, + transmute::TRANSMUTE_UNDEFINED_REPR, transmute::UNSOUND_COLLECTION_TRANSMUTE, transmute::USELESS_TRANSMUTE, transmute::WRONG_TRANSMUTE, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 9d42185dc..0b0772651 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -5,6 +5,7 @@ #![feature(control_flow_enum)] #![feature(drain_filter)] #![feature(iter_intersperse)] +#![feature(let_chains)] #![feature(let_else)] #![feature(once_cell)] #![feature(rustc_private)] diff --git a/clippy_lints/src/transmute/mod.rs b/clippy_lints/src/transmute/mod.rs index c370941dd..4c320deec 100644 --- a/clippy_lints/src/transmute/mod.rs +++ b/clippy_lints/src/transmute/mod.rs @@ -7,6 +7,7 @@ mod transmute_num_to_bytes; mod transmute_ptr_to_ptr; mod transmute_ptr_to_ref; mod transmute_ref_to_ref; +mod transmute_undefined_repr; mod transmutes_expressible_as_ptr_casts; mod unsound_collection_transmute; mod useless_transmute; @@ -355,6 +356,30 @@ declare_clippy_lint! { "transmute between collections of layout-incompatible types" } +declare_clippy_lint! { + /// ### What it does + /// Checks for transmutes either to or from a type which does not have a defined representation. + /// + /// ### Why is this bad? + /// The results of such a transmute are not defined. + /// + /// ### Example + /// ```rust + /// struct Foo(u32, T); + /// let _ = unsafe { core::mem::transmute::, Foo>(Foo(0u32, 0u32)) }; + /// ``` + /// Use instead: + /// ```rust + /// #[repr(C)] + /// struct Foo(u32, T); + /// let _ = unsafe { core::mem::transmute::, Foo>(Foo(0u32, 0u32)) }; + /// ``` + #[clippy::version = "1.60.0"] + pub TRANSMUTE_UNDEFINED_REPR, + correctness, + "transmute to or from a type with an undefined representation" +} + declare_lint_pass!(Transmute => [ CROSSPOINTER_TRANSMUTE, TRANSMUTE_PTR_TO_REF, @@ -369,6 +394,7 @@ declare_lint_pass!(Transmute => [ TRANSMUTE_NUM_TO_BYTES, UNSOUND_COLLECTION_TRANSMUTE, TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS, + TRANSMUTE_UNDEFINED_REPR, ]); impl<'tcx> LateLintPass<'tcx> for Transmute { @@ -402,7 +428,10 @@ impl<'tcx> LateLintPass<'tcx> for Transmute { | transmute_int_to_float::check(cx, e, from_ty, to_ty, arg, const_context) | transmute_float_to_int::check(cx, e, from_ty, to_ty, arg, const_context) | transmute_num_to_bytes::check(cx, e, from_ty, to_ty, arg, const_context) - | unsound_collection_transmute::check(cx, e, from_ty, to_ty); + | ( + unsound_collection_transmute::check(cx, e, from_ty, to_ty) + || transmute_undefined_repr::check(cx, e, from_ty, to_ty) + ); if !linted { transmutes_expressible_as_ptr_casts::check(cx, e, from_ty, to_ty, arg); diff --git a/clippy_lints/src/transmute/transmute_undefined_repr.rs b/clippy_lints/src/transmute/transmute_undefined_repr.rs new file mode 100644 index 000000000..4b7c49327 --- /dev/null +++ b/clippy_lints/src/transmute/transmute_undefined_repr.rs @@ -0,0 +1,291 @@ +use super::TRANSMUTE_UNDEFINED_REPR; +use clippy_utils::diagnostics::span_lint_and_then; +use rustc_hir::Expr; +use rustc_lint::LateContext; +use rustc_middle::ty::subst::{GenericArg, Subst}; +use rustc_middle::ty::{self, Ty, TyS, TypeAndMut}; +use rustc_span::Span; + +#[allow(clippy::too_many_lines)] +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + e: &'tcx Expr<'_>, + from_ty_orig: Ty<'tcx>, + to_ty_orig: Ty<'tcx>, +) -> bool { + let mut from_ty = cx.tcx.erase_regions(from_ty_orig); + let mut to_ty = cx.tcx.erase_regions(to_ty_orig); + + while !TyS::same_type(from_ty, to_ty) { + match reduce_refs(cx, e.span, from_ty, to_ty) { + ReducedTys::FromFatPtr { unsized_ty, .. } => { + span_lint_and_then( + cx, + TRANSMUTE_UNDEFINED_REPR, + e.span, + &format!("transmute from `{}` which has an undefined layout", from_ty_orig), + |diag| { + if !TyS::same_type(from_ty_orig.peel_refs(), unsized_ty) { + diag.note(&format!("the contained type `&{}` has an undefined layout", unsized_ty)); + } + }, + ); + return true; + }, + ReducedTys::ToFatPtr { unsized_ty, .. } => { + span_lint_and_then( + cx, + TRANSMUTE_UNDEFINED_REPR, + e.span, + &format!("transmute to `{}` which has an undefined layout", to_ty_orig), + |diag| { + if !TyS::same_type(to_ty_orig.peel_refs(), unsized_ty) { + diag.note(&format!("the contained type `&{}` has an undefined layout", unsized_ty)); + } + }, + ); + return true; + }, + ReducedTys::ToPtr { + from_ty: from_sub_ty, + to_ty: to_sub_ty, + } => match reduce_ty(cx, from_sub_ty) { + ReducedTy::UnorderedFields(from_ty) => { + span_lint_and_then( + cx, + TRANSMUTE_UNDEFINED_REPR, + e.span, + &format!("transmute from `{}` which has an undefined layout", from_ty_orig), + |diag| { + if !TyS::same_type(from_ty_orig.peel_refs(), from_ty) { + diag.note(&format!("the contained type `{}` has an undefined layout", from_ty)); + } + }, + ); + return true; + }, + ReducedTy::Ref(from_sub_ty) => { + from_ty = from_sub_ty; + to_ty = to_sub_ty; + continue; + }, + _ => break, + }, + ReducedTys::FromPtr { + from_ty: from_sub_ty, + to_ty: to_sub_ty, + } => match reduce_ty(cx, to_sub_ty) { + ReducedTy::UnorderedFields(to_ty) => { + span_lint_and_then( + cx, + TRANSMUTE_UNDEFINED_REPR, + e.span, + &format!("transmute to `{}` which has an undefined layout", to_ty_orig), + |diag| { + if !TyS::same_type(to_ty_orig.peel_refs(), to_ty) { + diag.note(&format!("the contained type `{}` has an undefined layout", to_ty)); + } + }, + ); + return true; + }, + ReducedTy::Ref(to_sub_ty) => { + from_ty = from_sub_ty; + to_ty = to_sub_ty; + continue; + }, + _ => break, + }, + ReducedTys::Other { + from_ty: from_sub_ty, + to_ty: to_sub_ty, + } => match (reduce_ty(cx, from_sub_ty), reduce_ty(cx, to_sub_ty)) { + (ReducedTy::IntArray, _) | (_, ReducedTy::IntArray) => return false, + (ReducedTy::UnorderedFields(from_ty), ReducedTy::UnorderedFields(to_ty)) + if !TyS::same_type(from_ty, to_ty) => + { + span_lint_and_then( + cx, + TRANSMUTE_UNDEFINED_REPR, + e.span, + &format!( + "transmute from `{}` to `{}`, both of which have an undefined layout", + from_ty_orig, to_ty_orig + ), + |diag| { + if let (Some(from_def), Some(to_def)) = (from_ty.ty_adt_def(), to_ty.ty_adt_def()) + && from_def == to_def + { + diag.note(&format!( + "two instances of the same generic type (`{}`) may have different layouts", + cx.tcx.item_name(from_def.did) + )); + } else { + if !TyS::same_type(from_ty_orig.peel_refs(), from_ty) { + diag.note(&format!("the contained type `{}` has an undefined layout", from_ty)); + } + if !TyS::same_type(to_ty_orig.peel_refs(), to_ty) { + diag.note(&format!("the contained type `{}` has an undefined layout", to_ty)); + } + } + }, + ); + return true; + }, + ( + ReducedTy::UnorderedFields(from_ty), + ReducedTy::Other(_) | ReducedTy::OrderedFields(_) | ReducedTy::Ref(_), + ) => { + span_lint_and_then( + cx, + TRANSMUTE_UNDEFINED_REPR, + e.span, + &format!("transmute from `{}` which has an undefined layout", from_ty_orig), + |diag| { + if !TyS::same_type(from_ty_orig.peel_refs(), from_ty) { + diag.note(&format!("the contained type `{}` has an undefined layout", from_ty)); + } + }, + ); + return true; + }, + ( + ReducedTy::Other(_) | ReducedTy::OrderedFields(_) | ReducedTy::Ref(_), + ReducedTy::UnorderedFields(to_ty), + ) => { + span_lint_and_then( + cx, + TRANSMUTE_UNDEFINED_REPR, + e.span, + &format!("transmute into `{}` which has an undefined layout", to_ty_orig), + |diag| { + if !TyS::same_type(to_ty_orig.peel_refs(), to_ty) { + diag.note(&format!("the contained type `{}` has an undefined layout", to_ty)); + } + }, + ); + return true; + }, + (ReducedTy::Ref(from_sub_ty), ReducedTy::Ref(to_sub_ty)) => { + from_ty = from_sub_ty; + to_ty = to_sub_ty; + continue; + }, + ( + ReducedTy::OrderedFields(_) | ReducedTy::Ref(_) | ReducedTy::Other(_), + ReducedTy::OrderedFields(_) | ReducedTy::Ref(_) | ReducedTy::Other(_), + ) + | (ReducedTy::UnorderedFields(_), ReducedTy::UnorderedFields(_)) => break, + }, + } + } + + false +} + +enum ReducedTys<'tcx> { + FromFatPtr { unsized_ty: Ty<'tcx> }, + ToFatPtr { unsized_ty: Ty<'tcx> }, + ToPtr { from_ty: Ty<'tcx>, to_ty: Ty<'tcx> }, + FromPtr { from_ty: Ty<'tcx>, to_ty: Ty<'tcx> }, + Other { from_ty: Ty<'tcx>, to_ty: Ty<'tcx> }, +} + +fn reduce_refs<'tcx>( + cx: &LateContext<'tcx>, + span: Span, + mut from_ty: Ty<'tcx>, + mut to_ty: Ty<'tcx>, +) -> ReducedTys<'tcx> { + loop { + return match (from_ty.kind(), to_ty.kind()) { + ( + ty::Ref(_, from_sub_ty, _) | ty::RawPtr(TypeAndMut { ty: from_sub_ty, .. }), + ty::Ref(_, to_sub_ty, _) | ty::RawPtr(TypeAndMut { ty: to_sub_ty, .. }), + ) => { + from_ty = from_sub_ty; + to_ty = to_sub_ty; + continue; + }, + (ty::Ref(_, unsized_ty, _) | ty::RawPtr(TypeAndMut { ty: unsized_ty, .. }), _) + if !unsized_ty.is_sized(cx.tcx.at(span), cx.param_env) => + { + ReducedTys::FromFatPtr { unsized_ty } + }, + (_, ty::Ref(_, unsized_ty, _) | ty::RawPtr(TypeAndMut { ty: unsized_ty, .. })) + if !unsized_ty.is_sized(cx.tcx.at(span), cx.param_env) => + { + ReducedTys::ToFatPtr { unsized_ty } + }, + (ty::Ref(_, from_ty, _) | ty::RawPtr(TypeAndMut { ty: from_ty, .. }), _) => { + ReducedTys::FromPtr { from_ty, to_ty } + }, + (_, ty::Ref(_, to_ty, _) | ty::RawPtr(TypeAndMut { ty: to_ty, .. })) => { + ReducedTys::ToPtr { from_ty, to_ty } + }, + _ => ReducedTys::Other { from_ty, to_ty }, + }; + } +} + +enum ReducedTy<'tcx> { + OrderedFields(Ty<'tcx>), + UnorderedFields(Ty<'tcx>), + Ref(Ty<'tcx>), + Other(Ty<'tcx>), + IntArray, +} + +fn reduce_ty<'tcx>(cx: &LateContext<'tcx>, mut ty: Ty<'tcx>) -> ReducedTy<'tcx> { + loop { + ty = cx.tcx.try_normalize_erasing_regions(cx.param_env, ty).unwrap_or(ty); + return match *ty.kind() { + ty::Array(sub_ty, _) if matches!(sub_ty.kind(), ty::Int(_) | ty::Uint(_)) => ReducedTy::IntArray, + ty::Array(sub_ty, _) | ty::Slice(sub_ty) => { + ty = sub_ty; + continue; + }, + ty::Tuple(args) => { + let mut iter = args.iter().map(GenericArg::expect_ty); + let Some(sized_ty) = iter.find(|ty| !is_zero_sized_ty(cx, ty)) else { + return ReducedTy::OrderedFields(ty); + }; + if iter.all(|ty| is_zero_sized_ty(cx, ty)) { + ty = sized_ty; + continue; + } + ReducedTy::UnorderedFields(ty) + }, + ty::Adt(def, substs) if def.is_struct() => { + if def.repr.inhibit_struct_field_reordering_opt() { + return ReducedTy::OrderedFields(ty); + } + let mut iter = def + .non_enum_variant() + .fields + .iter() + .map(|f| cx.tcx.type_of(f.did).subst(cx.tcx, substs)); + let Some(sized_ty) = iter.find(|ty| !is_zero_sized_ty(cx, ty)) else { + return ReducedTy::OrderedFields(ty); + }; + if iter.all(|ty| is_zero_sized_ty(cx, ty)) { + ty = sized_ty; + continue; + } + ReducedTy::UnorderedFields(ty) + }, + ty::Ref(..) | ty::RawPtr(_) => ReducedTy::Ref(ty), + _ => ReducedTy::Other(ty), + }; + } +} + +fn is_zero_sized_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { + if let Ok(ty) = cx.tcx.try_normalize_erasing_regions(cx.param_env, ty) + && let Ok(layout) = cx.tcx.layout_of(cx.param_env.and(ty)) + { + layout.layout.size.bytes() == 0 + } else { + false + } +} diff --git a/tests/ui/crashes/ice-4968.rs b/tests/ui/crashes/ice-4968.rs index 3822f1745..e0510d942 100644 --- a/tests/ui/crashes/ice-4968.rs +++ b/tests/ui/crashes/ice-4968.rs @@ -3,6 +3,7 @@ // Test for https://github.com/rust-lang/rust-clippy/issues/4968 #![warn(clippy::unsound_collection_transmute)] +#![allow(clippy::transmute_undefined_repr)] trait Trait { type Assoc; diff --git a/tests/ui/transmute_undefined_repr.rs b/tests/ui/transmute_undefined_repr.rs new file mode 100644 index 000000000..71539940f --- /dev/null +++ b/tests/ui/transmute_undefined_repr.rs @@ -0,0 +1,44 @@ +#![warn(clippy::transmute_undefined_repr)] +#![allow(clippy::unit_arg)] + +fn value() -> T { + unimplemented!() +} + +struct Empty; +struct Ty(T); +struct Ty2(T, U); + +#[repr(C)] +struct Ty2C(T, U); + +fn main() { + unsafe { + let _: () = core::mem::transmute(value::()); + let _: Empty = core::mem::transmute(value::<()>()); + + let _: Ty = core::mem::transmute(value::()); + let _: Ty = core::mem::transmute(value::()); + + let _: Ty2C = core::mem::transmute(value::>()); // Lint, Ty2 is unordered + let _: Ty2 = core::mem::transmute(value::>()); // Lint, Ty2 is unordered + + let _: Ty2 = core::mem::transmute(value::>>()); // Ok, Ty2 types are the same + let _: Ty> = core::mem::transmute(value::>()); // Ok, Ty2 types are the same + + let _: Ty2 = core::mem::transmute(value::>>()); // Lint, different Ty2 instances + let _: Ty> = core::mem::transmute(value::>()); // Lint, different Ty2 instances + + let _: Ty<&()> = core::mem::transmute(value::<&()>()); + let _: &() = core::mem::transmute(value::>()); + + let _: &Ty2 = core::mem::transmute(value::>>()); // Lint, different Ty2 instances + let _: Ty<&Ty2> = core::mem::transmute(value::<&Ty2>()); // Lint, different Ty2 instances + + let _: Ty = core::mem::transmute(value::<&Ty2>()); // Ok, pointer to usize conversion + let _: &Ty2 = core::mem::transmute(value::>()); // Ok, pointer to usize conversion + + let _: Ty<[u8; 8]> = core::mem::transmute(value::>()); // Ok, transmute to byte array + let _: Ty2 = core::mem::transmute(value::>()); // Ok, transmute from byte array + } +} diff --git a/tests/ui/transmute_undefined_repr.stderr b/tests/ui/transmute_undefined_repr.stderr new file mode 100644 index 000000000..040c63c7a --- /dev/null +++ b/tests/ui/transmute_undefined_repr.stderr @@ -0,0 +1,44 @@ +error: transmute from `Ty2` which has an undefined layout + --> $DIR/transmute_undefined_repr.rs:23:33 + | +LL | let _: Ty2C = core::mem::transmute(value::>()); // Lint, Ty2 is unordered + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::transmute-undefined-repr` implied by `-D warnings` + +error: transmute into `Ty2` which has an undefined layout + --> $DIR/transmute_undefined_repr.rs:24:32 + | +LL | let _: Ty2 = core::mem::transmute(value::>()); // Lint, Ty2 is unordered + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: transmute from `Ty>` to `Ty2`, both of which have an undefined layout + --> $DIR/transmute_undefined_repr.rs:29:32 + | +LL | let _: Ty2 = core::mem::transmute(value::>>()); // Lint, different Ty2 instances + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: two instances of the same generic type (`Ty2`) may have different layouts + +error: transmute from `Ty2` to `Ty>`, both of which have an undefined layout + --> $DIR/transmute_undefined_repr.rs:30:36 + | +LL | let _: Ty> = core::mem::transmute(value::>()); // Lint, different Ty2 instances + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: two instances of the same generic type (`Ty2`) may have different layouts + +error: transmute to `&Ty2` which has an undefined layout + --> $DIR/transmute_undefined_repr.rs:35:33 + | +LL | let _: &Ty2 = core::mem::transmute(value::>>()); // Lint, different Ty2 instances + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: transmute from `&Ty2` which has an undefined layout + --> $DIR/transmute_undefined_repr.rs:36:37 + | +LL | let _: Ty<&Ty2> = core::mem::transmute(value::<&Ty2>()); // Lint, different Ty2 instances + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 6 previous errors + From 68993b1f6cf94701ff420e51aa927c568ae7eef2 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Sun, 6 Feb 2022 09:42:10 -0500 Subject: [PATCH 3/3] Small `transmute_float_to_int` cleanup --- .../src/transmute/transmute_float_to_int.rs | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/transmute/transmute_float_to_int.rs b/clippy_lints/src/transmute/transmute_float_to_int.rs index f4656283b..d5ef86dc4 100644 --- a/clippy_lints/src/transmute/transmute_float_to_int.rs +++ b/clippy_lints/src/transmute/transmute_float_to_int.rs @@ -15,7 +15,7 @@ pub(super) fn check<'tcx>( e: &'tcx Expr<'_>, from_ty: Ty<'tcx>, to_ty: Ty<'tcx>, - arg: &'tcx Expr<'_>, + mut arg: &'tcx Expr<'_>, const_context: bool, ) -> bool { match (&from_ty.kind(), &to_ty.kind()) { @@ -26,37 +26,36 @@ pub(super) fn check<'tcx>( e.span, &format!("transmute from a `{}` to a `{}`", from_ty, to_ty), |diag| { - let mut expr = arg; - let mut arg = sugg::Sugg::hir(cx, expr, ".."); + let mut sugg = sugg::Sugg::hir(cx, arg, ".."); - if let ExprKind::Unary(UnOp::Neg, inner_expr) = &expr.kind { - expr = inner_expr; + if let ExprKind::Unary(UnOp::Neg, inner_expr) = &arg.kind { + arg = inner_expr; } if_chain! { // if the expression is a float literal and it is unsuffixed then // add a suffix so the suggestion is valid and unambiguous - if let ExprKind::Lit(lit) = &expr.kind; + if let ExprKind::Lit(lit) = &arg.kind; if let ast::LitKind::Float(_, ast::LitFloatType::Unsuffixed) = lit.node; then { - let op = format!("{}{}", arg, float_ty.name_str()).into(); - match arg { - sugg::Sugg::MaybeParen(_) => arg = sugg::Sugg::MaybeParen(op), - _ => arg = sugg::Sugg::NonParen(op) + let op = format!("{}{}", sugg, float_ty.name_str()).into(); + match sugg { + sugg::Sugg::MaybeParen(_) => sugg = sugg::Sugg::MaybeParen(op), + _ => sugg = sugg::Sugg::NonParen(op) } } } - arg = sugg::Sugg::NonParen(format!("{}.to_bits()", arg.maybe_par()).into()); + sugg = sugg::Sugg::NonParen(format!("{}.to_bits()", sugg.maybe_par()).into()); // cast the result of `to_bits` if `to_ty` is signed - arg = if let ty::Int(int_ty) = to_ty.kind() { - arg.as_ty(int_ty.name_str().to_string()) + sugg = if let ty::Int(int_ty) = to_ty.kind() { + sugg.as_ty(int_ty.name_str().to_string()) } else { - arg + sugg }; - diag.span_suggestion(e.span, "consider using", arg.to_string(), Applicability::Unspecified); + diag.span_suggestion(e.span, "consider using", sugg.to_string(), Applicability::Unspecified); }, ); true