Auto merge of rust-lang#110496 - WaffleLapkin:🏳️‍⚧️sound, r=compiler-errors

Don't transmute `&List<GenericArg>` <-> `&List<Ty>`

In rust-lang#93505 we allowed safely transmuting between `&List<GenericArg<'_>>` and `&List<Ty<'_>>`. This was possible because `GenericArg` is a tagged pointer and the tag for types is `0b00`, such that a `GenericArg` with a type inside has the same layout as `Ty`.

While this was meant as an optimization, it doesn't look like it was actually any perf or max-rss win (see rust-lang#94799 (comment), rust-lang#94841, rust-lang#110496 (comment)).

Additionally the way it was done is quite fragile — `unsafe` code was not properly documented or contained in a module, types were not marked as `repr(C)` (making the transmutes possibly unsound). All of this makes the code maintenance harder and blocks other possible optimizations (as an example I've found out about these `transmutes` when my change caused them to sigsegv compiler).

Thus, I think we can safely (pun intended) remove those transmutes, making maintenance easier, optimizations possible, code less cursed, etc.

r? `@compiler-errors`

Author: bors

compiler/rustc_middle/src/ty/context.rs

@@ -141,6 +141,7 @@ pub struct CtxtInterners<'tcx> {
     type_: InternedSet<'tcx, WithCachedTypeInfo<TyKind<'tcx>>>,
     const_lists: InternedSet<'tcx, List<ty::Const<'tcx>>>,
     substs: InternedSet<'tcx, InternalSubsts<'tcx>>,
+    type_lists: InternedSet<'tcx, List<Ty<'tcx>>>,
     canonical_var_infos: InternedSet<'tcx, List<CanonicalVarInfo<'tcx>>>,
     region: InternedSet<'tcx, RegionKind<'tcx>>,
     poly_existential_predicates: InternedSet<'tcx, List<PolyExistentialPredicate<'tcx>>>,
@@ -163,6 +164,7 @@ impl<'tcx> CtxtInterners<'tcx> {
             type_: Default::default(),
             const_lists: Default::default(),
             substs: Default::default(),
+            type_lists: Default::default(),
             region: Default::default(),
             poly_existential_predicates: Default::default(),
             canonical_var_infos: Default::default(),
@@ -1278,25 +1280,6 @@ macro_rules! nop_lift {
     };
 }
 
-// Can't use the macros as we have reuse the `substs` here.
-//
-// See `mk_type_list` for more info.
-impl<'a, 'tcx> Lift<'tcx> for &'a List<Ty<'a>> {
-    type Lifted = &'tcx List<Ty<'tcx>>;
-    fn lift_to_tcx(self, tcx: TyCtxt<'tcx>) -> Option<Self::Lifted> {
-        if self.is_empty() {
-            return Some(List::empty());
-        }
-
-        tcx.interners
-            .substs
-            .contains_pointer_to(&InternedInSet(self.as_substs()))
-            // SAFETY: `self` is interned and therefore valid
-            // for the entire lifetime of the `TyCtxt`.
-            .then(|| unsafe { mem::transmute::<&'a List<Ty<'a>>, &'tcx List<Ty<'tcx>>>(self) })
-    }
-}
-
 macro_rules! nop_list_lift {
     ($set:ident; $ty:ty => $lifted:ty) => {
         impl<'a, 'tcx> Lift<'tcx> for &'a List<$ty> {
@@ -1320,6 +1303,7 @@ nop_lift! {const_; Const<'a> => Const<'tcx>}
 nop_lift! {const_allocation; ConstAllocation<'a> => ConstAllocation<'tcx>}
 nop_lift! {predicate; Predicate<'a> => Predicate<'tcx>}
 
+nop_list_lift! {type_lists; Ty<'a> => Ty<'tcx>}
 nop_list_lift! {poly_existential_predicates; PolyExistentialPredicate<'a> => PolyExistentialPredicate<'tcx>}
 nop_list_lift! {predicates; Predicate<'a> => Predicate<'tcx>}
 nop_list_lift! {canonical_var_infos; CanonicalVarInfo<'a> => CanonicalVarInfo<'tcx>}
@@ -1594,6 +1578,7 @@ macro_rules! slice_interners {
 slice_interners!(
     const_lists: pub mk_const_list(Const<'tcx>),
     substs: pub mk_substs(GenericArg<'tcx>),
+    type_lists: pub mk_type_list(Ty<'tcx>),
     canonical_var_infos: pub mk_canonical_var_infos(CanonicalVarInfo<'tcx>),
     poly_existential_predicates: intern_poly_existential_predicates(PolyExistentialPredicate<'tcx>),
     predicates: intern_predicates(Predicate<'tcx>),
@@ -2193,18 +2178,6 @@ impl<'tcx> TyCtxt<'tcx> {
         T::collect_and_apply(iter, |xs| self.mk_const_list(xs))
     }
 
-    pub fn mk_type_list(self, ts: &[Ty<'tcx>]) -> &'tcx List<Ty<'tcx>> {
-        // Actually intern type lists as lists of `GenericArg`s.
-        //
-        // Transmuting from `Ty<'tcx>` to `GenericArg<'tcx>` is sound
-        // as explained in `ty_slice_as_generic_arg`. With this,
-        // we guarantee that even when transmuting between `List<Ty<'tcx>>`
-        // and `List<GenericArg<'tcx>>`, the uniqueness requirement for
-        // lists is upheld.
-        let substs = self.mk_substs(ty::subst::ty_slice_as_generic_args(ts));
-        substs.try_as_type_list().unwrap()
-    }
-
     // Unlike various other `mk_*_from_iter` functions, this one uses `I:
     // IntoIterator` instead of `I: Iterator`, and it doesn't have a slice
     // variant, because of the need to combine `inputs` and `output`. This

compiler/rustc_middle/src/ty/subst.rs

@@ -67,19 +67,6 @@ pub fn ty_slice_as_generic_args<'a, 'tcx>(ts: &'a [Ty<'tcx>]) -> &'a [GenericArg
     unsafe { slice::from_raw_parts(ts.as_ptr().cast(), ts.len()) }
 }
 
-impl<'tcx> List<Ty<'tcx>> {
-    /// Allows to freely switch between `List<Ty<'tcx>>` and `List<GenericArg<'tcx>>`.
-    ///
-    /// As lists are interned, `List<Ty<'tcx>>` and `List<GenericArg<'tcx>>` have
-    /// be interned together, see `mk_type_list` for more details.
-    #[inline]
-    pub fn as_substs(&'tcx self) -> SubstsRef<'tcx> {
-        assert_eq!(TYPE_TAG, 0);
-        // SAFETY: `List<T>` is `#[repr(C)]`. `Ty` and `GenericArg` is explained above.
-        unsafe { &*(self as *const List<Ty<'tcx>> as *const List<GenericArg<'tcx>>) }
-    }
-}
-
 impl<'tcx> GenericArgKind<'tcx> {
     #[inline]
     fn pack(self) -> GenericArg<'tcx> {
@@ -268,13 +255,16 @@ pub type InternalSubsts<'tcx> = List<GenericArg<'tcx>>;
 pub type SubstsRef<'tcx> = &'tcx InternalSubsts<'tcx>;
 
 impl<'tcx> InternalSubsts<'tcx> {
-    /// Checks whether all elements of this list are types, if so, transmute.
-    pub fn try_as_type_list(&'tcx self) -> Option<&'tcx List<Ty<'tcx>>> {
-        self.iter().all(|arg| matches!(arg.unpack(), GenericArgKind::Type(_))).then(|| {
-            assert_eq!(TYPE_TAG, 0);
-            // SAFETY: All elements are types, see `List<Ty<'tcx>>::as_substs`.
-            unsafe { &*(self as *const List<GenericArg<'tcx>> as *const List<Ty<'tcx>>) }
-        })
+    /// Converts substs to a type list.
+    ///
+    /// # Panics
+    ///
+    /// If any of the generic arguments are not types.
+    pub fn into_type_list(&self, tcx: TyCtxt<'tcx>) -> &'tcx List<Ty<'tcx>> {
+        tcx.mk_type_list_from_iter(self.iter().map(|arg| match arg.unpack() {
+            GenericArgKind::Type(ty) => ty,
+            _ => bug!("`into_type_list` called on substs with non-types"),
+        }))
     }
 
     /// Interpret these substitutions as the substitutions of a closure type.

compiler/rustc_middle/src/ty/walk.rs

@@ -194,7 +194,7 @@ fn push_inner<'tcx>(stack: &mut TypeWalkerStack<'tcx>, parent: GenericArg<'tcx>)
             | ty::FnDef(_, substs) => {
                 stack.extend(substs.iter().rev());
             }
-            ty::Tuple(ts) => stack.extend(ts.as_substs().iter().rev()),
+            ty::Tuple(ts) => stack.extend(ts.iter().rev().map(GenericArg::from)),
             ty::GeneratorWitness(ts) => {
                 stack.extend(ts.skip_binder().iter().rev().map(|ty| ty.into()));
             }

compiler/rustc_traits/src/chalk/lowering.rs

@@ -60,6 +60,20 @@ impl<'tcx> LowerInto<'tcx, chalk_ir::Substitution<RustInterner<'tcx>>> for Subst
     }
 }
 
+impl<'tcx> LowerInto<'tcx, chalk_ir::Substitution<RustInterner<'tcx>>>
+    for &'tcx ty::List<Ty<'tcx>>
+{
+    fn lower_into(
+        self,
+        interner: RustInterner<'tcx>,
+    ) -> chalk_ir::Substitution<RustInterner<'tcx>> {
+        chalk_ir::Substitution::from_iter(
+            interner,
+            self.iter().map(|ty| GenericArg::from(ty).lower_into(interner)),
+        )
+    }
+}
+
 impl<'tcx> LowerInto<'tcx, SubstsRef<'tcx>> for &chalk_ir::Substitution<RustInterner<'tcx>> {
     fn lower_into(self, interner: RustInterner<'tcx>) -> SubstsRef<'tcx> {
         interner
@@ -351,9 +365,7 @@ impl<'tcx> LowerInto<'tcx, chalk_ir::Ty<RustInterner<'tcx>>> for Ty<'tcx> {
             ty::GeneratorWitness(_) => unimplemented!(),
             ty::GeneratorWitnessMIR(..) => unimplemented!(),
             ty::Never => chalk_ir::TyKind::Never,
-            ty::Tuple(types) => {
-                chalk_ir::TyKind::Tuple(types.len(), types.as_substs().lower_into(interner))
-            }
+            ty::Tuple(types) => chalk_ir::TyKind::Tuple(types.len(), types.lower_into(interner)),
             ty::Alias(ty::Projection, ty::AliasTy { def_id, substs, .. }) => {
                 chalk_ir::TyKind::Alias(chalk_ir::AliasTy::Projection(chalk_ir::ProjectionTy {
                     associated_ty_id: chalk_ir::AssocTypeId(def_id),
@@ -435,7 +447,7 @@ impl<'tcx> LowerInto<'tcx, Ty<'tcx>> for &chalk_ir::Ty<RustInterner<'tcx>> {
             TyKind::GeneratorWitness(..) => unimplemented!(),
             TyKind::Never => ty::Never,
             TyKind::Tuple(_len, substitution) => {
-                ty::Tuple(substitution.lower_into(interner).try_as_type_list().unwrap())
+                ty::Tuple(substitution.lower_into(interner).into_type_list(interner.tcx))
             }
             TyKind::Slice(ty) => ty::Slice(ty.lower_into(interner)),
             TyKind::Raw(mutbl, ty) => ty::RawPtr(ty::TypeAndMut {

src/tools/clippy/clippy_utils/src/ty.rs

@@ -975,7 +975,7 @@ pub fn approx_ty_size<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> u64 {
     }
     match (cx.layout_of(ty).map(|layout| layout.size.bytes()), ty.kind()) {
         (Ok(size), _) => size,
-        (Err(_), ty::Tuple(list)) => list.as_substs().types().map(|t| approx_ty_size(cx, t)).sum(),
+        (Err(_), ty::Tuple(list)) => list.iter().map(|t| approx_ty_size(cx, t)).sum(),
         (Err(_), ty::Array(t, n)) => {
             n.try_eval_target_usize(cx.tcx, cx.param_env).unwrap_or_default() * approx_ty_size(cx, *t)
         },