Auto merge of #75346 - davidtwco:issue-69925-polymorphic-instancedef-fnptrshim, r=nikomatsakis

shim: monomorphic `FnPtrShim`s during construction

Fixes #69925.

This PR adjusts MIR shim construction so that substitutions are applied to function pointer shims during construction, rather than during codegen (as determined by `substs_for_mir_body`).

r? `@eddyb`

3 files changed, 58 insertions, 59 deletions

diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs
index c8b6705b35f..a6b62097d5b 100644
--- a/compiler/rustc_middle/src/ty/instance.rs
+++ b/compiler/rustc_middle/src/ty/instance.rs
@@ -62,10 +62,6 @@ pub enum InstanceDef<'tcx> {
     /// `<fn() as FnTrait>::call_*` (generated `FnTrait` implementation for `fn()` pointers).
     ///
     /// `DefId` is `FnTrait::call_*`.
-    ///
-    /// NB: the (`fn` pointer) type must currently be monomorphic to avoid double substitution
-    /// problems with the MIR shim bodies. `Instance::resolve` enforces this.
-    // FIXME(#69925) support polymorphic MIR shim bodies properly instead.
     FnPtrShim(DefId, Ty<'tcx>),
 
     /// Dynamic dispatch to `<dyn Trait as Trait>::fn`.
@@ -87,10 +83,6 @@ pub enum InstanceDef<'tcx> {
     /// The `DefId` is for `core::ptr::drop_in_place`.
     /// The `Option<Ty<'tcx>>` is either `Some(T)`, or `None` for empty drop
     /// glue.
-    ///
-    /// NB: the type must currently be monomorphic to avoid double substitution
-    /// problems with the MIR shim bodies. `Instance::resolve` enforces this.
-    // FIXME(#69925) support polymorphic MIR shim bodies properly instead.
     DropGlue(DefId, Option<Ty<'tcx>>),
 
     /// Compiler-generated `<T as Clone>::clone` implementation.
@@ -99,10 +91,6 @@ pub enum InstanceDef<'tcx> {
     /// Additionally, arrays, tuples, and closures get a `Clone` shim even if they aren't `Copy`.
     ///
     /// The `DefId` is for `Clone::clone`, the `Ty` is the type `T` with the builtin `Clone` impl.
-    ///
-    /// NB: the type must currently be monomorphic to avoid double substitution
-    /// problems with the MIR shim bodies. `Instance::resolve` enforces this.
-    // FIXME(#69925) support polymorphic MIR shim bodies properly instead.
     CloneShim(DefId, Ty<'tcx>),
 }
 
@@ -243,6 +231,27 @@ impl<'tcx> InstanceDef<'tcx> {
             _ => false,
         }
     }
+
+    /// Returns `true` when the MIR body associated with this instance should be monomorphized
+    /// by its users (e.g. codegen or miri) by substituting the `substs` from `Instance` (see
+    /// `Instance::substs_for_mir_body`).
+    ///
+    /// Otherwise, returns `false` only for some kinds of shims where the construction of the MIR
+    /// body should perform necessary substitutions.
+    pub fn has_polymorphic_mir_body(&self) -> bool {
+        match *self {
+            InstanceDef::CloneShim(..)
+            | InstanceDef::FnPtrShim(..)
+            | InstanceDef::DropGlue(_, Some(_)) => false,
+            InstanceDef::ClosureOnceShim { .. }
+            | InstanceDef::DropGlue(..)
+            | InstanceDef::Item(_)
+            | InstanceDef::Intrinsic(..)
+            | InstanceDef::ReifyShim(..)
+            | InstanceDef::Virtual(..)
+            | InstanceDef::VtableShim(..) => true,
+        }
+    }
 }
 
 impl<'tcx> fmt::Display for Instance<'tcx> {
@@ -440,30 +449,18 @@ impl<'tcx> Instance<'tcx> {
         Instance { def, substs }
     }
 
-    /// FIXME(#69925) Depending on the kind of `InstanceDef`, the MIR body associated with an
+    /// Depending on the kind of `InstanceDef`, the MIR body associated with an
     /// instance is expressed in terms of the generic parameters of `self.def_id()`, and in other
     /// cases the MIR body is expressed in terms of the types found in the substitution array.
     /// In the former case, we want to substitute those generic types and replace them with the
     /// values from the substs when monomorphizing the function body. But in the latter case, we
     /// don't want to do that substitution, since it has already been done effectively.
     ///
-    /// This function returns `Some(substs)` in the former case and None otherwise -- i.e., if
+    /// This function returns `Some(substs)` in the former case and `None` otherwise -- i.e., if
     /// this function returns `None`, then the MIR body does not require substitution during
-    /// monomorphization.
+    /// codegen.
     pub fn substs_for_mir_body(&self) -> Option<SubstsRef<'tcx>> {
-        match self.def {
-            InstanceDef::CloneShim(..)
-            | InstanceDef::DropGlue(_, Some(_)) => None,
-            InstanceDef::ClosureOnceShim { .. }
-            | InstanceDef::DropGlue(..)
-            // FIXME(#69925): `FnPtrShim` should be in the other branch.
-            | InstanceDef::FnPtrShim(..)
-            | InstanceDef::Item(_)
-            | InstanceDef::Intrinsic(..)
-            | InstanceDef::ReifyShim(..)
-            | InstanceDef::Virtual(..)
-            | InstanceDef::VtableShim(..) => Some(self.substs),
-        }
+        if self.def.has_polymorphic_mir_body() { Some(self.substs) } else { None }
     }
 
     /// Returns a new `Instance` where generic parameters in `instance.substs` are replaced by
diff --git a/compiler/rustc_mir/src/shim.rs b/compiler/rustc_mir/src/shim.rs
index bfe0b85b5b1..7e4d189f0b7 100644
--- a/compiler/rustc_mir/src/shim.rs
+++ b/compiler/rustc_mir/src/shim.rs
@@ -33,7 +33,7 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<'
     let mut result = match instance {
         ty::InstanceDef::Item(..) => bug!("item {:?} passed to make_shim", instance),
         ty::InstanceDef::VtableShim(def_id) => {
-            build_call_shim(tcx, instance, Some(Adjustment::Deref), CallKind::Direct(def_id), None)
+            build_call_shim(tcx, instance, Some(Adjustment::Deref), CallKind::Direct(def_id))
         }
         ty::InstanceDef::FnPtrShim(def_id, ty) => {
             let trait_ = tcx.trait_of_item(def_id).unwrap();
@@ -42,16 +42,8 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<'
                 Some(ty::ClosureKind::FnMut | ty::ClosureKind::Fn) => Adjustment::Deref,
                 None => bug!("fn pointer {:?} is not an fn", ty),
             };
-            // HACK: we need the "real" argument types for the MIR,
-            // but because our substs are (Self, Args), where Args
-            // is a tuple, we must include the *concrete* argument
-            // types in the MIR. They will be substituted again with
-            // the param-substs, but because they are concrete, this
-            // will not do any harm.
-            let sig = tcx.erase_late_bound_regions(&ty.fn_sig(tcx));
-            let arg_tys = sig.inputs();
-
-            build_call_shim(tcx, instance, Some(adjustment), CallKind::Indirect(ty), Some(arg_tys))
+
+            build_call_shim(tcx, instance, Some(adjustment), CallKind::Indirect(ty))
         }
         // We are generating a call back to our def-id, which the
         // codegen backend knows to turn to an actual call, be it
@@ -59,7 +51,7 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<'
         // indirect calls must be codegen'd differently than direct ones
         // (such as `#[track_caller]`).
         ty::InstanceDef::ReifyShim(def_id) => {
-            build_call_shim(tcx, instance, None, CallKind::Direct(def_id), None)
+            build_call_shim(tcx, instance, None, CallKind::Direct(def_id))
         }
         ty::InstanceDef::ClosureOnceShim { call_once: _ } => {
             let fn_mut = tcx.require_lang_item(LangItem::FnMut, None);
@@ -70,13 +62,7 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<'
                 .unwrap()
                 .def_id;
 
-            build_call_shim(
-                tcx,
-                instance,
-                Some(Adjustment::RefMut),
-                CallKind::Direct(call_mut),
-                None,
-            )
+            build_call_shim(tcx, instance, Some(Adjustment::RefMut), CallKind::Direct(call_mut))
         }
         ty::InstanceDef::DropGlue(def_id, ty) => build_drop_shim(tcx, def_id, ty),
         ty::InstanceDef::CloneShim(def_id, ty) => build_clone_shim(tcx, def_id, ty),
@@ -641,29 +627,45 @@ impl CloneShimBuilder<'tcx> {
     }
 }
 
-/// Builds a "call" shim for `instance`. The shim calls the
-/// function specified by `call_kind`, first adjusting its first
-/// argument according to `rcvr_adjustment`.
-///
-/// If `untuple_args` is a vec of types, the second argument of the
-/// function will be untupled as these types.
+/// Builds a "call" shim for `instance`. The shim calls the function specified by `call_kind`,
+/// first adjusting its first argument according to `rcvr_adjustment`.
 fn build_call_shim<'tcx>(
     tcx: TyCtxt<'tcx>,
     instance: ty::InstanceDef<'tcx>,
     rcvr_adjustment: Option<Adjustment>,
     call_kind: CallKind<'tcx>,
-    untuple_args: Option<&[Ty<'tcx>]>,
 ) -> Body<'tcx> {
     debug!(
-        "build_call_shim(instance={:?}, rcvr_adjustment={:?}, \
-            call_kind={:?}, untuple_args={:?})",
-        instance, rcvr_adjustment, call_kind, untuple_args
+        "build_call_shim(instance={:?}, rcvr_adjustment={:?}, call_kind={:?})",
+        instance, rcvr_adjustment, call_kind
     );
 
+    // `FnPtrShim` contains the fn pointer type that a call shim is being built for - this is used
+    // to substitute into the signature of the shim. It is not necessary for users of this
+    // MIR body to perform further substitutions (see `InstanceDef::has_polymorphic_mir_body`).
+    let (sig_substs, untuple_args) = if let ty::InstanceDef::FnPtrShim(_, ty) = instance {
+        let sig = tcx.erase_late_bound_regions(&ty.fn_sig(tcx));
+
+        let untuple_args = sig.inputs();
+
+        // Create substitutions for the `Self` and `Args` generic parameters of the shim body.
+        let arg_tup = tcx.mk_tup(untuple_args.iter());
+        let sig_substs = tcx.mk_substs_trait(ty, &[ty::subst::GenericArg::from(arg_tup)]);
+
+        (Some(sig_substs), Some(untuple_args))
+    } else {
+        (None, None)
+    };
+
     let def_id = instance.def_id();
     let sig = tcx.fn_sig(def_id);
     let mut sig = tcx.erase_late_bound_regions(&sig);
 
+    assert_eq!(sig_substs.is_some(), !instance.has_polymorphic_mir_body());
+    if let Some(sig_substs) = sig_substs {
+        sig = sig.subst(tcx, sig_substs);
+    }
+
     if let CallKind::Indirect(fnty) = call_kind {
         // `sig` determines our local decls, and thus the callee type in the `Call` terminator. This
         // can only be an `FnDef` or `FnPtr`, but currently will be `Self` since the types come from
diff --git a/src/test/mir-opt/fn_ptr_shim.core.ops-function-Fn-call.AddMovesForPackedDrops.before.mir b/src/test/mir-opt/fn_ptr_shim.core.ops-function-Fn-call.AddMovesForPackedDrops.before.mir
index d3f92d389f5..bcc6042f2fb 100644
--- a/src/test/mir-opt/fn_ptr_shim.core.ops-function-Fn-call.AddMovesForPackedDrops.before.mir
+++ b/src/test/mir-opt/fn_ptr_shim.core.ops-function-Fn-call.AddMovesForPackedDrops.before.mir
@@ -1,7 +1,7 @@
 // MIR for `std::ops::Fn::call` before AddMovesForPackedDrops
 
-fn std::ops::Fn::call(_1: *const fn(), _2: Args) -> <Self as FnOnce<Args>>::Output {
-    let mut _0: <Self as std::ops::FnOnce<Args>>::Output; // return place in scope 0 at $SRC_DIR/core/src/ops/function.rs:LL:COL
+fn std::ops::Fn::call(_1: *const fn(), _2: ()) -> <fn() as FnOnce<()>>::Output {
+    let mut _0: <fn() as std::ops::FnOnce<()>>::Output; // return place in scope 0 at $SRC_DIR/core/src/ops/function.rs:LL:COL
 
     bb0: {
         _0 = move (*_1)() -> bb1;        // scope 0 at $SRC_DIR/core/src/ops/function.rs:LL:COL