From d3ce4dd038b32a11708f38f3373b7d3d6cde8661 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Sun, 24 Mar 2024 18:28:58 -0400 Subject: [PATCH] Remove `is_normalizable`. Add `sizedness_of` to avoid `layout_of` query on type aliases. --- clippy_lints/src/transmute/eager_transmute.rs | 3 - clippy_lints/src/zero_sized_map_values.rs | 13 +- clippy_utils/src/ty.rs | 143 ++++++++++++------ tests/ui/crashes/ice-10508.rs | 19 +++ tests/ui/large_enum_variant.64bit.stderr | 18 ++- tests/ui/large_enum_variant.rs | 5 + 6 files changed, 138 insertions(+), 63 deletions(-) create mode 100644 tests/ui/crashes/ice-10508.rs diff --git a/clippy_lints/src/transmute/eager_transmute.rs b/clippy_lints/src/transmute/eager_transmute.rs index 1dfc9f7091e8..5d9954a2b371 100644 --- a/clippy_lints/src/transmute/eager_transmute.rs +++ b/clippy_lints/src/transmute/eager_transmute.rs @@ -1,5 +1,4 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::ty::is_normalizable; use clippy_utils::{eq_expr_value, path_to_local}; use rustc_abi::WrappingRange; use rustc_errors::Applicability; @@ -84,8 +83,6 @@ pub(super) fn check<'tcx>( && path.ident.name == sym!(then_some) && is_local_with_projections(transmutable) && binops_with_local(cx, transmutable, receiver) - && is_normalizable(cx, cx.param_env, from_ty) - && is_normalizable(cx, cx.param_env, to_ty) // we only want to lint if the target type has a niche that is larger than the one of the source type // e.g. `u8` to `NonZero` should lint, but `NonZero` to `u8` should not && let Ok(from_layout) = cx.tcx.layout_of(cx.param_env.and(from_ty)) diff --git a/clippy_lints/src/zero_sized_map_values.rs b/clippy_lints/src/zero_sized_map_values.rs index e14480b86556..9caaa6ab9f72 100644 --- a/clippy_lints/src/zero_sized_map_values.rs +++ b/clippy_lints/src/zero_sized_map_values.rs @@ -1,10 +1,9 @@ use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::ty::{is_normalizable, is_type_diagnostic_item}; +use clippy_utils::ty::{is_type_diagnostic_item, sizedness_of}; use rustc_hir::{self as hir, HirId, ItemKind, Node}; use rustc_hir_analysis::lower_ty; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::layout::LayoutOf as _; -use rustc_middle::ty::{self, Ty, TypeVisitableExt}; +use rustc_middle::ty::{self, Ty}; use rustc_session::declare_lint_pass; use rustc_span::sym; @@ -51,13 +50,7 @@ impl LateLintPass<'_> for ZeroSizedMapValues { && (is_type_diagnostic_item(cx, ty, sym::HashMap) || is_type_diagnostic_item(cx, ty, sym::BTreeMap)) && let ty::Adt(_, args) = ty.kind() && let ty = args.type_at(1) - // Fixes https://github.com/rust-lang/rust-clippy/issues/7447 because of - // https://github.com/rust-lang/rust/blob/master/compiler/rustc_middle/src/ty/sty.rs#L968 - && !ty.has_escaping_bound_vars() - // Do this to prevent `layout_of` crashing, being unable to fully normalize `ty`. - && is_normalizable(cx, cx.param_env, ty) - && let Ok(layout) = cx.layout_of(ty) - && layout.is_zst() + && sizedness_of(cx.tcx, cx.param_env, ty).is_zero() { span_lint_and_help( cx, diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index bd48990aea95..c68bd517d943 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -15,7 +15,7 @@ use rustc_lint::LateContext; use rustc_middle::mir::interpret::Scalar; use rustc_middle::mir::ConstValue; use rustc_middle::traits::EvaluationResult; -use rustc_middle::ty::layout::ValidityRequirement; +use rustc_middle::ty::layout::{LayoutOf, ValidityRequirement}; use rustc_middle::ty::{ self, AdtDef, AliasTy, AssocItem, AssocKind, Binder, BoundRegion, FnSig, GenericArg, GenericArgKind, GenericArgsRef, GenericParamDefKind, IntTy, ParamEnv, Region, RegionKind, TraitRef, Ty, TyCtxt, TypeSuperVisitable, @@ -353,50 +353,6 @@ pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { } } -// FIXME: Per https://doc.rust-lang.org/nightly/nightly-rustc/rustc_trait_selection/infer/at/struct.At.html#method.normalize -// this function can be removed once the `normalize` method does not panic when normalization does -// not succeed -/// Checks if `Ty` is normalizable. This function is useful -/// to avoid crashes on `layout_of`. -pub fn is_normalizable<'tcx>(cx: &LateContext<'tcx>, param_env: ParamEnv<'tcx>, ty: Ty<'tcx>) -> bool { - is_normalizable_helper(cx, param_env, ty, &mut FxHashMap::default()) -} - -fn is_normalizable_helper<'tcx>( - cx: &LateContext<'tcx>, - param_env: ParamEnv<'tcx>, - ty: Ty<'tcx>, - cache: &mut FxHashMap, bool>, -) -> bool { - if let Some(&cached_result) = cache.get(&ty) { - return cached_result; - } - // prevent recursive loops, false-negative is better than endless loop leading to stack overflow - cache.insert(ty, false); - let infcx = cx.tcx.infer_ctxt().build(); - let cause = ObligationCause::dummy(); - let result = if infcx.at(&cause, param_env).query_normalize(ty).is_ok() { - match ty.kind() { - ty::Adt(def, args) => def.variants().iter().all(|variant| { - variant - .fields - .iter() - .all(|field| is_normalizable_helper(cx, param_env, field.ty(cx.tcx, args), cache)) - }), - _ => ty.walk().all(|generic_arg| match generic_arg.unpack() { - GenericArgKind::Type(inner_ty) if inner_ty != ty => { - is_normalizable_helper(cx, param_env, inner_ty, cache) - }, - _ => true, // if inner_ty == ty, we've already checked it - }), - } - } else { - false - }; - cache.insert(ty, result); - result -} - /// Returns `true` if the given type is a non aggregate primitive (a `bool` or `char`, any /// integer or floating-point number type). For checking aggregation of primitive types (e.g. /// tuples and slices of primitive type) see `is_recursively_primitive_type` @@ -977,11 +933,12 @@ pub fn adt_and_variant_of_res<'tcx>(cx: &LateContext<'tcx>, res: Res) -> Option< /// Comes up with an "at least" guesstimate for the type's size, not taking into /// account the layout of type parameters. +/// +/// This function will ICE if called with an improper `ParamEnv`. This can happen +/// when linting in when item, but the type is retrieved from a different item +/// without instantiating the generic arguments. It can also happen when linting a +/// type alias as those do not have a `ParamEnv`. pub fn approx_ty_size<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> u64 { - use rustc_middle::ty::layout::LayoutOf; - if !is_normalizable(cx, cx.param_env, ty) { - return 0; - } match (cx.layout_of(ty).map(|layout| layout.size.bytes()), ty.kind()) { (Ok(size), _) => size, (Err(_), ty::Tuple(list)) => list.iter().map(|t| approx_ty_size(cx, t)).sum(), @@ -1340,3 +1297,91 @@ pub fn get_field_by_name<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, name: Symbol) -> _ => None, } } + +#[derive(Clone, Copy)] +pub enum Sizedness { + /// The type is uninhabited. (e.g. `!`) + Uninhabited, + /// The type is zero-sized. + Zero, + /// The type has some other size or an unknown size. + Other, +} +impl Sizedness { + pub fn is_zero(self) -> bool { + matches!(self, Self::Zero) + } + + pub fn is_uninhabited(self) -> bool { + matches!(self, Self::Uninhabited) + } +} + +/// Calculates the sizedness of a type. +pub fn sizedness_of<'tcx>(tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, ty: Ty<'tcx>) -> Sizedness { + fn is_zst<'tcx>(tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, ty: Ty<'tcx>) -> bool { + match *ty.kind() { + ty::FnDef(..) | ty::Never => true, + ty::Tuple(tys) => tys.iter().all(|ty| is_zst(tcx, param_env, ty)), + // Zero length arrays are always zero-sized, even for uninhabited types. + ty::Array(_, len) if len.try_eval_target_usize(tcx, param_env).is_some_and(|x| x == 0) => true, + ty::Array(ty, _) | ty::Pat(ty, _) => is_zst(tcx, param_env, ty), + ty::Adt(adt, args) => { + let mut iter = adt.variants().iter().filter(|v| { + !v.fields + .iter() + .any(|f| f.ty(tcx, args).is_privately_uninhabited(tcx, param_env)) + }); + let is_zst = iter.next().map_or(true, |v| { + v.fields.iter().all(|f| is_zst(tcx, param_env, f.ty(tcx, args))) + }); + is_zst && iter.next().is_none() + }, + ty::Closure(_, args) => args + .as_closure() + .upvar_tys() + .iter() + .all(|ty| is_zst(tcx, param_env, ty)), + ty::CoroutineWitness(_, args) => args + .iter() + .filter_map(GenericArg::as_type) + .all(|ty| is_zst(tcx, param_env, ty)), + ty::Alias(..) => { + if let Ok(normalized) = tcx.try_normalize_erasing_regions(param_env, ty) + && normalized != ty + { + is_zst(tcx, param_env, normalized) + } else { + false + } + }, + ty::Bool + | ty::Char + | ty::Int(_) + | ty::Uint(_) + | ty::Float(_) + | ty::RawPtr(..) + | ty::Ref(..) + | ty::FnPtr(..) + | ty::Param(_) + | ty::Bound(..) + | ty::Placeholder(_) + | ty::Infer(_) + | ty::Error(_) + | ty::Dynamic(..) + | ty::Slice(..) + | ty::Str + | ty::Foreign(_) + | ty::Coroutine(..) + | ty::CoroutineClosure(..) => false, + } + } + + if ty.is_privately_uninhabited(tcx, param_env) { + Sizedness::Uninhabited + } else if is_zst(tcx, param_env, ty) { + Sizedness::Zero + } else { + Sizedness::Other + } +} diff --git a/tests/ui/crashes/ice-10508.rs b/tests/ui/crashes/ice-10508.rs new file mode 100644 index 000000000000..f45057217b43 --- /dev/null +++ b/tests/ui/crashes/ice-10508.rs @@ -0,0 +1,19 @@ +// Used to overflow in `is_normalizable` + +use std::marker::PhantomData; + +struct Node { + m: PhantomData<&'static T>, +} + +struct Digit { + elem: T, +} + +enum FingerTree { + Single(T), + + Deep(Digit, Box>>), +} + +fn main() {} diff --git a/tests/ui/large_enum_variant.64bit.stderr b/tests/ui/large_enum_variant.64bit.stderr index 805cb406f834..201af733dba8 100644 --- a/tests/ui/large_enum_variant.64bit.stderr +++ b/tests/ui/large_enum_variant.64bit.stderr @@ -276,5 +276,21 @@ help: consider boxing the large fields to reduce the total size of the enum LL | Error(Box>), | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -error: aborting due to 16 previous errors +error: large size difference between variants + --> tests/ui/large_enum_variant.rs:167:1 + | +LL | / enum SelfRef<'a> { +LL | | Small, + | | ----- the second-largest variant carries no data at all +LL | | Large([&'a SelfRef<'a>; 1024]), + | | ------------------------------ the largest variant contains at least 8192 bytes +LL | | } + | |_^ the entire enum is at least 8192 bytes + | +help: consider boxing the large fields to reduce the total size of the enum + | +LL | Large(Box<[&'a SelfRef<'a>; 1024]>), + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: aborting due to 17 previous errors diff --git a/tests/ui/large_enum_variant.rs b/tests/ui/large_enum_variant.rs index 3625c011dbfa..2553702670aa 100644 --- a/tests/ui/large_enum_variant.rs +++ b/tests/ui/large_enum_variant.rs @@ -163,3 +163,8 @@ fn main() { } ); } + +enum SelfRef<'a> { + Small, + Large([&'a SelfRef<'a>; 1024]), +}