Auto merge of #89514 - davidtwco:polymorphize-shims-and-predicates, r…

…=lcnr

polymorphization: shims and predicates

Supersedes #75737 and #75414. This pull request includes up some changes to polymorphization which hadn't landed previously and gets stage2 bootstrapping and the test suite passing when polymorphization is enabled. There are still issues with `type_id` and polymorphization to investigate but this should get polymorphization in a reasonable state to work on.

- #75737 and #75414 both worked but were blocked on having the rest of the test suite pass (with polymorphization enabled) with and without the PRs. It makes more sense to just land these so that the changes are in.
- #75737's changes remove the restriction of `InstanceDef::Item` on polymorphization, so that shims can now be polymorphized. This won't have much of an effect until polymorphization's analysis is more advanced, but it doesn't hurt.
- #75414's changes remove all logic which marks parameters as used based on their presence in predicates - given #75675, this will enable more polymorphization and avoid the symbol clashes that predicate logic previously sidestepped.
- Polymorphization now explicitly checks (and skips) foreign items, this is necessary for stage2 bootstrapping to work when polymorphization is enabled.
- The conditional determining the emission of a note adding context to a post-monomorphization error has been modified. Polymorphization results in `optimized_mir` running for shims during collection where that wouldn't happen previously, some errors are emitted during `optimized_mir` and these were considered post-monomorphization errors with the existing logic (more errors and shims have a `DefId` coming from the std crate, not the local crate), adding a note that resulted in tests failing. It isn't particularly feasible to change where polymorphization runs or prevent it from using `optimized_mir`, so it seemed more reasonable to not change the conditional.
- `characteristic_def_id_of_type` was being invoked during partitioning for self types of impl blocks which had projections that depended on the value of unused generic parameters of a function - this caused a ICE in a debuginfo test. If partitioning is enabled and the instance needs substitution then this is skipped. That test still fails for me locally, but not with an ICE, but it fails in a fresh checkout too, so 🤷‍♂️.

r? `@lcnr`

compiler/rustc_const_eval/src/interpret/util.rs

@@ -35,7 +35,8 @@ where
                 ty::Closure(def_id, substs)
                 | ty::Generator(def_id, substs, ..)
                 | ty::FnDef(def_id, substs) => {
-                    let unused_params = self.tcx.unused_generic_params(def_id);
+                    let instance = ty::InstanceDef::Item(ty::WithOptConstParam::unknown(def_id));
+                    let unused_params = self.tcx.unused_generic_params(instance);
                     for (index, subst) in substs.into_iter().enumerate() {
                         let index = index
                             .try_into()

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

@@ -83,6 +83,12 @@ impl IntoArgs for (CrateNum, DefId) {
     }
 }
 
+impl IntoArgs for ty::InstanceDef<'tcx> {
+    fn into_args(self) -> (DefId, DefId) {
+        (self.def_id(), self.def_id())
+    }
+}
+
 provide! { <'tcx> tcx, def_id, other, cdata,
     type_of => { cdata.get_type(def_id.index, tcx) }
     generics_of => { cdata.get_generics(def_id.index, tcx.sess) }

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -1319,7 +1319,9 @@ impl EncodeContext<'a, 'tcx> {
             }
             record!(self.tables.promoted_mir[def_id.to_def_id()] <- self.tcx.promoted_mir(def_id));
 
-            let unused = self.tcx.unused_generic_params(def_id);
+            let instance =
+                ty::InstanceDef::Item(ty::WithOptConstParam::unknown(def_id.to_def_id()));
+            let unused = self.tcx.unused_generic_params(instance);
             if !unused.is_empty() {
                 record!(self.tables.unused_generic_params[def_id.to_def_id()] <- unused);
             }

compiler/rustc_middle/src/mir/mono.rs

@@ -47,6 +47,14 @@ pub enum MonoItem<'tcx> {
 }
 
 impl<'tcx> MonoItem<'tcx> {
+    /// Returns `true` if the mono item is user-defined (i.e. not compiler-generated, like shims).
+    pub fn is_user_defined(&self) -> bool {
+        match *self {
+            MonoItem::Fn(instance) => matches!(instance.def, InstanceDef::Item(..)),
+            MonoItem::Static(..) | MonoItem::GlobalAsm(..) => true,
+        }
+    }
+
     pub fn size_estimate(&self, tcx: TyCtxt<'tcx>) -> usize {
         match *self {
             MonoItem::Fn(instance) => {

compiler/rustc_middle/src/query/mod.rs

@@ -1558,11 +1558,11 @@ rustc_queries! {
     query codegen_unit(_: Symbol) -> &'tcx CodegenUnit<'tcx> {
         desc { "codegen_unit" }
     }
-    query unused_generic_params(key: DefId) -> FiniteBitSet<u32> {
-        cache_on_disk_if { key.is_local() }
+    query unused_generic_params(key: ty::InstanceDef<'tcx>) -> FiniteBitSet<u32> {
+        cache_on_disk_if { key.def_id().is_local() }
         desc {
             |tcx| "determining which generic parameters are unused by `{}`",
-                tcx.def_path_str(key)
+                tcx.def_path_str(key.def_id())
         }
     }
     query backend_optimization_level(_: ()) -> OptLevel {

compiler/rustc_middle/src/ty/instance.rs

@@ -152,6 +152,22 @@ impl<'tcx> InstanceDef<'tcx> {
         }
     }
 
+    /// Returns the `DefId` of instances which might not require codegen locally.
+    pub fn def_id_if_not_guaranteed_local_codegen(self) -> Option<DefId> {
+        match self {
+            ty::InstanceDef::Item(def) => Some(def.did),
+            ty::InstanceDef::DropGlue(def_id, Some(_)) => Some(def_id),
+            InstanceDef::VtableShim(..)
+            | InstanceDef::ReifyShim(..)
+            | InstanceDef::FnPtrShim(..)
+            | InstanceDef::Virtual(..)
+            | InstanceDef::Intrinsic(..)
+            | InstanceDef::ClosureOnceShim { .. }
+            | InstanceDef::DropGlue(..)
+            | InstanceDef::CloneShim(..) => None,
+        }
+    }
+
     #[inline]
     pub fn with_opt_param(self) -> ty::WithOptConstParam<DefId> {
         match self {
@@ -567,29 +583,26 @@ impl<'tcx> Instance<'tcx> {
             return self;
         }
 
-        if let InstanceDef::Item(def) = self.def {
-            let polymorphized_substs = polymorphize(tcx, def.did, self.substs);
-            debug!("polymorphize: self={:?} polymorphized_substs={:?}", self, polymorphized_substs);
-            Self { def: self.def, substs: polymorphized_substs }
-        } else {
-            self
-        }
+        let polymorphized_substs = polymorphize(tcx, self.def, self.substs);
+        debug!("polymorphize: self={:?} polymorphized_substs={:?}", self, polymorphized_substs);
+        Self { def: self.def, substs: polymorphized_substs }
     }
 }
 
 fn polymorphize<'tcx>(
     tcx: TyCtxt<'tcx>,
-    def_id: DefId,
+    instance: ty::InstanceDef<'tcx>,
     substs: SubstsRef<'tcx>,
 ) -> SubstsRef<'tcx> {
-    debug!("polymorphize({:?}, {:?})", def_id, substs);
-    let unused = tcx.unused_generic_params(def_id);
+    debug!("polymorphize({:?}, {:?})", instance, substs);
+    let unused = tcx.unused_generic_params(instance);
     debug!("polymorphize: unused={:?}", unused);
 
     // If this is a closure or generator then we need to handle the case where another closure
     // from the function is captured as an upvar and hasn't been polymorphized. In this case,
     // the unpolymorphized upvar closure would result in a polymorphized closure producing
     // multiple mono items (and eventually symbol clashes).
+    let def_id = instance.def_id();
     let upvars_ty = if tcx.is_closure(def_id) {
         Some(substs.as_closure().tupled_upvars_ty())
     } else if tcx.type_of(def_id).is_generator() {
@@ -613,15 +626,23 @@ fn polymorphize<'tcx>(
             debug!("fold_ty: ty={:?}", ty);
             match ty.kind {
                 ty::Closure(def_id, substs) => {
-                    let polymorphized_substs = polymorphize(self.tcx, def_id, substs);
+                    let polymorphized_substs = polymorphize(
+                        self.tcx,
+                        ty::InstanceDef::Item(ty::WithOptConstParam::unknown(def_id)),
+                        substs,
+                    );
                     if substs == polymorphized_substs {
                         ty
                     } else {
                         self.tcx.mk_closure(def_id, polymorphized_substs)
                     }
                 }
                 ty::Generator(def_id, substs, movability) => {
-                    let polymorphized_substs = polymorphize(self.tcx, def_id, substs);
+                    let polymorphized_substs = polymorphize(
+                        self.tcx,
+                        ty::InstanceDef::Item(ty::WithOptConstParam::unknown(def_id)),
+                        substs,
+                    );
                     if substs == polymorphized_substs {
                         ty
                     } else {

compiler/rustc_monomorphize/src/collector.rs

@@ -450,7 +450,9 @@ fn collect_items_rec<'tcx>(
     // involving a dependency, and the lack of context is confusing) in this MVP, we focus on
     // diagnostics on edges crossing a crate boundary: the collected mono items which are not
     // defined in the local crate.
-    if tcx.sess.diagnostic().err_count() > error_count && starting_point.node.krate() != LOCAL_CRATE
+    if tcx.sess.diagnostic().err_count() > error_count
+        && starting_point.node.krate() != LOCAL_CRATE
+        && starting_point.node.is_user_defined()
     {
         let formatted_item = with_no_trimmed_paths(|| starting_point.node.to_string());
         tcx.sess.span_note_without_error(
@@ -934,21 +936,13 @@ fn visit_instance_use<'tcx>(
     }
 }
 
-// Returns `true` if we should codegen an instance in the local crate.
-// Returns `false` if we can just link to the upstream crate and therefore don't
-// need a mono item.
+/// Returns `true` if we should codegen an instance in the local crate, or returns `false` if we
+/// can just link to the upstream crate and therefore don't need a mono item.
 fn should_codegen_locally<'tcx>(tcx: TyCtxt<'tcx>, instance: &Instance<'tcx>) -> bool {
-    let def_id = match instance.def {
-        ty::InstanceDef::Item(def) => def.did,
-        ty::InstanceDef::DropGlue(def_id, Some(_)) => def_id,
-        ty::InstanceDef::VtableShim(..)
-        | ty::InstanceDef::ReifyShim(..)
-        | ty::InstanceDef::ClosureOnceShim { .. }
-        | ty::InstanceDef::Virtual(..)
-        | ty::InstanceDef::FnPtrShim(..)
-        | ty::InstanceDef::DropGlue(..)
-        | ty::InstanceDef::Intrinsic(_)
-        | ty::InstanceDef::CloneShim(..) => return true,
+    let def_id = if let Some(def_id) = instance.def.def_id_if_not_guaranteed_local_codegen() {
+        def_id
+    } else {
+        return true;
     };
 
     if tcx.is_foreign_item(def_id) {

compiler/rustc_monomorphize/src/partitioning/default.rs

@@ -9,7 +9,7 @@ use rustc_middle::middle::exported_symbols::SymbolExportLevel;
 use rustc_middle::mir::mono::{CodegenUnit, CodegenUnitNameBuilder, Linkage, Visibility};
 use rustc_middle::mir::mono::{InstantiationMode, MonoItem};
 use rustc_middle::ty::print::characteristic_def_id_of_type;
-use rustc_middle::ty::{self, DefIdTree, InstanceDef, TyCtxt};
+use rustc_middle::ty::{self, fold::TypeFoldable, DefIdTree, InstanceDef, TyCtxt};
 use rustc_span::symbol::Symbol;
 
 use super::PartitioningCx;
@@ -300,14 +300,21 @@ fn characteristic_def_id_of_mono_item<'tcx>(
                     // call it.
                     return None;
                 }
-                // This is a method within an impl, find out what the self-type is:
-                let impl_self_ty = tcx.subst_and_normalize_erasing_regions(
-                    instance.substs,
-                    ty::ParamEnv::reveal_all(),
-                    tcx.type_of(impl_def_id),
-                );
-                if let Some(def_id) = characteristic_def_id_of_type(impl_self_ty) {
-                    return Some(def_id);
+
+                // When polymorphization is enabled, methods which do not depend on their generic
+                // parameters, but the self-type of their impl block do will fail to normalize.
+                if !tcx.sess.opts.debugging_opts.polymorphize
+                    || !instance.definitely_needs_subst(tcx)
+                {
+                    // This is a method within an impl, find out what the self-type is:
+                    let impl_self_ty = tcx.subst_and_normalize_erasing_regions(
+                        instance.substs,
+                        ty::ParamEnv::reveal_all(),
+                        tcx.type_of(impl_def_id),
+                    );
+                    if let Some(def_id) = characteristic_def_id_of_type(impl_self_ty) {
+                        return Some(def_id);
+                    }
                 }
             }