From d3ce4dd038b32a11708f38f3373b7d3d6cde8661 Mon Sep 17 00:00:00 2001
From: Jason Newcomb <jsnewcomb@pm.me>
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<u8>` should lint, but `NonZero<u8>` 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<Ty<'tcx>, 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<T: 'static> {
+ m: PhantomData<&'static T>,
+}
+
+struct Digit<T> {
+ elem: T,
+}
+
+enum FingerTree<T: 'static> {
+ Single(T),
+
+ Deep(Digit<T>, Box<FingerTree<Node<T>>>),
+}
+
+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<PossiblyLargeEnumWithConst<256>>),
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-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]),
+}