From 8e5208cbffd5ff43f80097d74a8802aad1d6190b Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Wed, 16 Mar 2022 12:54:32 -0400 Subject: [PATCH] Don't lint `transmute_undefined_repr` when changing the type of generic params --- .../src/transmute/transmute_undefined_repr.rs | 64 +++++++++++++------ tests/ui/transmute_undefined_repr.rs | 32 ++++++++++ tests/ui/transmute_undefined_repr.stderr | 34 +++++++--- 3 files changed, 102 insertions(+), 28 deletions(-) diff --git a/clippy_lints/src/transmute/transmute_undefined_repr.rs b/clippy_lints/src/transmute/transmute_undefined_repr.rs index 4922c40bc..f5e21267a 100644 --- a/clippy_lints/src/transmute/transmute_undefined_repr.rs +++ b/clippy_lints/src/transmute/transmute_undefined_repr.rs @@ -3,7 +3,7 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::ty::is_c_void; use rustc_hir::Expr; use rustc_lint::LateContext; -use rustc_middle::ty::subst::Subst; +use rustc_middle::ty::subst::{Subst, SubstsRef}; use rustc_middle::ty::{self, IntTy, Ty, TypeAndMut, UintTy}; use rustc_span::Span; @@ -127,6 +127,17 @@ pub(super) fn check<'tcx>( } => match (reduce_ty(cx, from_sub_ty), reduce_ty(cx, to_sub_ty)) { (ReducedTy::TypeErasure, _) | (_, ReducedTy::TypeErasure) => return false, (ReducedTy::UnorderedFields(from_ty), ReducedTy::UnorderedFields(to_ty)) if from_ty != to_ty => { + let same_adt_did = if let (ty::Adt(from_def, from_subs), ty::Adt(to_def, to_subs)) + = (from_ty.kind(), to_ty.kind()) + && from_def == to_def + { + if same_except_params(from_subs, to_subs) { + return false; + } + Some(from_def.did()) + } else { + None + }; span_lint_and_then( cx, TRANSMUTE_UNDEFINED_REPR, @@ -136,21 +147,17 @@ pub(super) fn check<'tcx>( from_ty_orig, to_ty_orig ), |diag| { - if_chain! { - if let (Some(from_def), Some(to_def)) = (from_ty.ty_adt_def(), to_ty.ty_adt_def()); - if from_def == to_def; - then { - diag.note(&format!( - "two instances of the same generic type (`{}`) may have different layouts", - cx.tcx.item_name(from_def.did()) - )); - } else { - if from_ty_orig.peel_refs() != from_ty { - diag.note(&format!("the contained type `{}` has an undefined layout", from_ty)); - } - if to_ty_orig.peel_refs() != to_ty { - diag.note(&format!("the contained type `{}` has an undefined layout", to_ty)); - } + if let Some(same_adt_did) = same_adt_did { + diag.note(&format!( + "two instances of the same generic type (`{}`) may have different layouts", + cx.tcx.item_name(same_adt_did) + )); + } else { + if from_ty_orig.peel_refs() != from_ty { + diag.note(&format!("the contained type `{}` has an undefined layout", from_ty)); + } + if to_ty_orig.peel_refs() != to_ty { + diag.note(&format!("the contained type `{}` has an undefined layout", to_ty)); } } }, @@ -197,10 +204,13 @@ pub(super) fn check<'tcx>( continue; }, ( - ReducedTy::OrderedFields(_) | ReducedTy::Ref(_) | ReducedTy::Other(_), - ReducedTy::OrderedFields(_) | ReducedTy::Ref(_) | ReducedTy::Other(_), + ReducedTy::OrderedFields(_) | ReducedTy::Ref(_) | ReducedTy::Other(_) | ReducedTy::Param, + ReducedTy::OrderedFields(_) | ReducedTy::Ref(_) | ReducedTy::Other(_) | ReducedTy::Param, ) - | (ReducedTy::UnorderedFields(_), ReducedTy::UnorderedFields(_)) => break, + | ( + ReducedTy::UnorderedFields(_) | ReducedTy::Param, + ReducedTy::UnorderedFields(_) | ReducedTy::Param, + ) => break, }, } } @@ -264,6 +274,8 @@ enum ReducedTy<'tcx> { UnorderedFields(Ty<'tcx>), /// The type is a reference to the contained type. Ref(Ty<'tcx>), + /// The type is a generic parameter. + Param, /// Any other type. Other(Ty<'tcx>), } @@ -317,6 +329,7 @@ fn reduce_ty<'tcx>(cx: &LateContext<'tcx>, mut ty: Ty<'tcx>) -> ReducedTy<'tcx> ty::Foreign(_) => ReducedTy::TypeErasure, ty::Ref(_, ty, _) => ReducedTy::Ref(ty), ty::RawPtr(ty) => ReducedTy::Ref(ty.ty), + ty::Param(_) => ReducedTy::Param, _ => ReducedTy::Other(ty), }; } @@ -344,3 +357,16 @@ fn is_size_pair(ty: Ty<'_>) -> bool { false } } + +fn same_except_params(subs1: SubstsRef<'_>, subs2: SubstsRef<'_>) -> bool { + // TODO: check const parameters as well. Currently this will consider `Array<5>` the same as + // `Array<6>` + for (ty1, ty2) in subs1.types().zip(subs2.types()).filter(|(ty1, ty2)| ty1 != ty2) { + match (ty1.kind(), ty2.kind()) { + (ty::Param(_), _) | (_, ty::Param(_)) => (), + (ty::Adt(adt1, subs1), ty::Adt(adt2, subs2)) if adt1 == adt2 && same_except_params(subs1, subs2) => (), + _ => return false, + } + } + true +} diff --git a/tests/ui/transmute_undefined_repr.rs b/tests/ui/transmute_undefined_repr.rs index 7cc03b89f..b06ed4a91 100644 --- a/tests/ui/transmute_undefined_repr.rs +++ b/tests/ui/transmute_undefined_repr.rs @@ -1,6 +1,7 @@ #![warn(clippy::transmute_undefined_repr)] #![allow(clippy::unit_arg, clippy::transmute_ptr_to_ref)] +use core::any::TypeId; use core::ffi::c_void; use core::mem::{size_of, transmute, MaybeUninit}; @@ -110,3 +111,34 @@ fn main() { let _: Ty<&[u32]> = transmute::<&[u32], _>(value::<&Vec>()); // Ok } } + +fn _with_generics() { + if TypeId::of::() != TypeId::of::() || TypeId::of::() != TypeId::of::() { + return; + } + unsafe { + let _: &u32 = transmute(value::<&T>()); // Ok + let _: &T = transmute(value::<&u32>()); // Ok + + let _: Vec = transmute(value::>()); // Ok + let _: Vec = transmute(value::>()); // Ok + + let _: Ty<&u32> = transmute(value::<&T>()); // Ok + let _: Ty<&T> = transmute(value::<&u32>()); // Ok + + let _: Vec = transmute(value::>()); // Ok + let _: Vec = transmute(value::>()); // Ok + + let _: &Ty2 = transmute(value::<&Ty2>()); // Ok + let _: &Ty2 = transmute(value::<&Ty2>()); // Ok + + let _: Vec> = transmute(value::>>()); // Ok + let _: Vec> = transmute(value::>>()); // Ok + + let _: Vec> = transmute(value::>>()); // Err + let _: Vec> = transmute(value::>>()); // Err + + let _: *const u32 = transmute(value::>()); // Ok + let _: Box = transmute(value::<*const u32>()); // Ok + } +} diff --git a/tests/ui/transmute_undefined_repr.stderr b/tests/ui/transmute_undefined_repr.stderr index 42d544fc9..28bfba6c7 100644 --- a/tests/ui/transmute_undefined_repr.stderr +++ b/tests/ui/transmute_undefined_repr.stderr @@ -1,5 +1,5 @@ error: transmute from `Ty2` which has an undefined layout - --> $DIR/transmute_undefined_repr.rs:26:33 + --> $DIR/transmute_undefined_repr.rs:27:33 | LL | let _: Ty2C = transmute(value::>()); // Lint, Ty2 is unordered | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -7,13 +7,13 @@ LL | let _: Ty2C = transmute(value::>()); // Lin = note: `-D clippy::transmute-undefined-repr` implied by `-D warnings` error: transmute into `Ty2` which has an undefined layout - --> $DIR/transmute_undefined_repr.rs:27:32 + --> $DIR/transmute_undefined_repr.rs:28:32 | LL | let _: Ty2 = transmute(value::>()); // Lint, Ty2 is unordered | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: transmute from `Ty>` to `Ty2`, both of which have an undefined layout - --> $DIR/transmute_undefined_repr.rs:32:32 + --> $DIR/transmute_undefined_repr.rs:33:32 | LL | let _: Ty2 = transmute(value::>>()); // Lint, different Ty2 instances | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -21,7 +21,7 @@ LL | let _: Ty2 = transmute(value::>>()); // = 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:33:36 + --> $DIR/transmute_undefined_repr.rs:34:36 | LL | let _: Ty> = transmute(value::>()); // Lint, different Ty2 instances | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -29,7 +29,7 @@ LL | let _: Ty> = transmute(value::>()); // = note: two instances of the same generic type (`Ty2`) may have different layouts error: transmute from `Ty<&Ty2>` to `&Ty2`, both of which have an undefined layout - --> $DIR/transmute_undefined_repr.rs:38:33 + --> $DIR/transmute_undefined_repr.rs:39:33 | LL | let _: &Ty2 = transmute(value::>>()); // Lint, different Ty2 instances | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -37,7 +37,7 @@ LL | let _: &Ty2 = transmute(value::>>()); / = note: two instances of the same generic type (`Ty2`) may have different layouts error: transmute from `&Ty2` to `Ty<&Ty2>`, both of which have an undefined layout - --> $DIR/transmute_undefined_repr.rs:39:37 + --> $DIR/transmute_undefined_repr.rs:40:37 | LL | let _: Ty<&Ty2> = transmute(value::<&Ty2>()); // Lint, different Ty2 instances | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -45,7 +45,7 @@ LL | let _: Ty<&Ty2> = transmute(value::<&Ty2>()); / = note: two instances of the same generic type (`Ty2`) may have different layouts error: transmute from `std::boxed::Box>` to `&mut Ty2`, both of which have an undefined layout - --> $DIR/transmute_undefined_repr.rs:56:45 + --> $DIR/transmute_undefined_repr.rs:57:45 | LL | let _: &'static mut Ty2 = transmute(value::>>()); // Lint, different Ty2 instances | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -53,12 +53,28 @@ LL | let _: &'static mut Ty2 = transmute(value::` to `std::boxed::Box>`, both of which have an undefined layout - --> $DIR/transmute_undefined_repr.rs:57:37 + --> $DIR/transmute_undefined_repr.rs:58:37 | LL | let _: Box> = transmute(value::<&'static mut Ty2>()); // Lint, different Ty2 instances | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: two instances of the same generic type (`Ty2`) may have different layouts -error: aborting due to 8 previous errors +error: transmute from `std::vec::Vec>` to `std::vec::Vec>`, both of which have an undefined layout + --> $DIR/transmute_undefined_repr.rs:138:35 + | +LL | let _: Vec> = transmute(value::>>()); // Err + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: two instances of the same generic type (`Vec`) may have different layouts + +error: transmute from `std::vec::Vec>` to `std::vec::Vec>`, both of which have an undefined layout + --> $DIR/transmute_undefined_repr.rs:139:35 + | +LL | let _: Vec> = transmute(value::>>()); // Err + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: two instances of the same generic type (`Vec`) may have different layouts + +error: aborting due to 10 previous errors