Auto merge of #12550 - Jarcho:issue_10508, r=y21

Remove `is_normalizable`

fixes #11915
fixes #9798
Fixes only the first ICE in #10508

`is_normalizable` is used in a few places to avoid an ICE due to a delayed bug in normalization which occurs when a projection type is used on a type which doesn't implement the correct trait. The only part of clippy that actually needed this check is `zero_sized_map_values` due to a quirk of how type aliases work (they don't a real `ParamEnv`). This fixes the need for the check there by manually walking the type to determine if it's zero sized, rather than attempting to compute the type's layout thereby avoid the normalization that triggers the delayed bug.

For an example of the issue with type aliases:
```rust
trait Foo { type Foo; }
struct Bar<T: Foo>(T::Foo);

// The `ParamEnv` here just has `T: Sized`, not `T: Sized + Foo`.
type Baz<T> = &'static Bar<T>;
```

When trying to compute the layout of `&'static Bar<T>` we need to determine if what type `<Bar<T> as Pointee>::Metadata` is. Doing this requires knowing if `T::Foo: Sized`, but since `T` doesn't have an associated type `Foo` in the context of the type alias a delayed bug is issued.

changelog: [`large_enum_variant`]: correctly get the size of `bytes::Bytes`.

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))

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,

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
+    }
+}

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() {}

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
 

tests/ui/large_enum_variant.rs

@@ -163,3 +163,8 @@ fn main() {
         }
     );
 }
+
+enum SelfRef<'a> {
+    Small,
+    Large([&'a SelfRef<'a>; 1024]),
+}