Auto merge of rust-lang#65947 - eddyb:fn-abi, r=oli-obk,nagisa

rustc: split FnAbi's into definitions/direct calls ("of_instance") and indirect calls ("of_fn_ptr").

After this PR:
* `InstanceDef::Virtual` is only used for "direct" virtual calls, and shims around those calls use `InstanceDef::ReifyShim` (i.e. for `<dyn Trait as Trait>::f as fn(_)`)
  * this could easily be done for intrinsics as well, to allow their reification, but I didn't do it
* `FnAbi::of_instance` is **always** used for declaring/defining an `fn`, and for direct calls to an `fn`
  * this is great for e.g. rust-lang#65881 (`#[track_caller]`), which can introduce the "caller location" argument into "codegen signatures" by only changing `FnAbi::of_instance`, after this PR
* `FnAbi::of_fn_ptr` is used primarily for indirect calls, i.e. to `fn` pointers
  * *not* virtual calls (which use `FnAbi::of_instance` with `InstanceDef::Virtual`)
  * there's also a couple uses where the `rustc_codegen_llvm` needs to declare (i.e. FFI-import) an LLVM function that has no Rust declaration available at all
    * at least one of them could probably be a "weak lang item" instead

As there are many steps, this PR is best reviewed commit by commit - some of which arguably should be in their own PRs, I may have gotten carried away a bit.

cc @nagisa @rkruppe @oli-obk @anp

Author: bors

src/librustc/mir/cache.rs

@@ -279,18 +279,10 @@ impl<'a, 'b, 'tcx> graph::GraphSuccessors<'b> for ReadOnlyBodyCache<'a, 'tcx> {
 
 
 impl Deref for ReadOnlyBodyCache<'a, 'tcx> {
-    type Target = Body<'tcx>;
+    type Target = &'a Body<'tcx>;
 
     fn deref(&self) -> &Self::Target {
-        self.body
-    }
-}
-
-impl Index<BasicBlock> for ReadOnlyBodyCache<'a, 'tcx> {
-    type Output = BasicBlockData<'tcx>;
-
-    fn index(&self, index: BasicBlock) -> &BasicBlockData<'tcx> {
-        &self.body[index]
+        &self.body
     }
 }
 

src/librustc/ty/instance.rs

@@ -1,16 +1,14 @@
 use crate::hir::CodegenFnAttrFlags;
-use crate::hir::Unsafety;
 use crate::hir::def::Namespace;
 use crate::hir::def_id::DefId;
-use crate::ty::{self, Ty, PolyFnSig, TypeFoldable, SubstsRef, TyCtxt};
+use crate::ty::{self, Ty, TypeFoldable, SubstsRef, TyCtxt};
 use crate::ty::print::{FmtPrinter, Printer};
 use crate::traits;
 use crate::middle::lang_items::DropInPlaceFnLangItem;
 use rustc_target::spec::abi::Abi;
 use rustc_macros::HashStable;
 
 use std::fmt;
-use std::iter;
 
 #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug, RustcEncodable, RustcDecodable)]
 #[derive(HashStable, Lift)]
@@ -29,17 +27,26 @@ pub enum InstanceDef<'tcx> {
 
     /// `fn()` pointer where the function itself cannot be turned into a pointer.
     ///
-    /// One example in the compiler today is functions annotated with `#[track_caller]`, which
-    /// must have their implicit caller location argument populated for a call. Because this is a
-    /// required part of the function's ABI but can't be tracked as a property of the function
-    /// pointer, we create a single "caller location" at the site where the function is reified.
+    /// One example is `<dyn Trait as Trait>::fn`, where the shim contains
+    /// a virtual call, which codegen supports only via a direct call to the
+    /// `<dyn Trait as Trait>::fn` instance (an `InstanceDef::Virtual`).
+    ///
+    /// Another example is functions annotated with `#[track_caller]`, which
+    /// must have their implicit caller location argument populated for a call.
+    /// Because this is a required part of the function's ABI but can't be tracked
+    /// as a property of the function pointer, we use a single "caller location"
+    /// (the definition of the function itself).
     ReifyShim(DefId),
 
     /// `<fn() as FnTrait>::call_*`
     /// `DefId` is `FnTrait::call_*`.
     FnPtrShim(DefId, Ty<'tcx>),
 
-    /// `<dyn Trait as Trait>::fn`
+    /// `<dyn Trait as Trait>::fn`, "direct calls" of which are implicitly
+    /// codegen'd as virtual calls.
+    ///
+    /// NB: if this is reified to a `fn` pointer, a `ReifyShim` is used
+    /// (see `ReifyShim` above for more details on that).
     Virtual(DefId, usize),
 
     /// `<[mut closure] as FnOnce>::call_once`
@@ -61,70 +68,6 @@ impl<'tcx> Instance<'tcx> {
             &ty,
         )
     }
-
-    fn fn_sig_noadjust(&self, tcx: TyCtxt<'tcx>) -> PolyFnSig<'tcx> {
-        let ty = self.ty(tcx);
-        match ty.kind {
-            ty::FnDef(..) |
-            // Shims currently have type FnPtr. Not sure this should remain.
-            ty::FnPtr(_) => ty.fn_sig(tcx),
-            ty::Closure(def_id, substs) => {
-                let sig = substs.as_closure().sig(def_id, tcx);
-
-                let env_ty = tcx.closure_env_ty(def_id, substs).unwrap();
-                sig.map_bound(|sig| tcx.mk_fn_sig(
-                    iter::once(*env_ty.skip_binder()).chain(sig.inputs().iter().cloned()),
-                    sig.output(),
-                    sig.c_variadic,
-                    sig.unsafety,
-                    sig.abi
-                ))
-            }
-            ty::Generator(def_id, substs, _) => {
-                let sig = substs.as_generator().poly_sig(def_id, tcx);
-
-                let env_region = ty::ReLateBound(ty::INNERMOST, ty::BrEnv);
-                let env_ty = tcx.mk_mut_ref(tcx.mk_region(env_region), ty);
-
-                let pin_did = tcx.lang_items().pin_type().unwrap();
-                let pin_adt_ref = tcx.adt_def(pin_did);
-                let pin_substs = tcx.intern_substs(&[env_ty.into()]);
-                let env_ty = tcx.mk_adt(pin_adt_ref, pin_substs);
-
-                sig.map_bound(|sig| {
-                    let state_did = tcx.lang_items().gen_state().unwrap();
-                    let state_adt_ref = tcx.adt_def(state_did);
-                    let state_substs = tcx.intern_substs(&[
-                        sig.yield_ty.into(),
-                        sig.return_ty.into(),
-                    ]);
-                    let ret_ty = tcx.mk_adt(state_adt_ref, state_substs);
-
-                    tcx.mk_fn_sig(iter::once(env_ty),
-                        ret_ty,
-                        false,
-                        Unsafety::Normal,
-                        Abi::Rust
-                    )
-                })
-            }
-            _ => bug!("unexpected type {:?} in Instance::fn_sig_noadjust", ty)
-        }
-    }
-
-    pub fn fn_sig(&self, tcx: TyCtxt<'tcx>) -> ty::PolyFnSig<'tcx> {
-        let mut fn_sig = self.fn_sig_noadjust(tcx);
-        if let InstanceDef::VtableShim(..) = self.def {
-            // Modify `fn(self, ...)` to `fn(self: *mut Self, ...)`.
-            fn_sig = fn_sig.map_bound(|mut fn_sig| {
-                let mut inputs_and_output = fn_sig.inputs_and_output.to_vec();
-                inputs_and_output[0] = tcx.mk_mut_ptr(inputs_and_output[0]);
-                fn_sig.inputs_and_output = tcx.intern_type_list(&inputs_and_output);
-                fn_sig
-            });
-        }
-        fn_sig
-    }
 }
 
 impl<'tcx> InstanceDef<'tcx> {
@@ -196,7 +139,7 @@ impl<'tcx> fmt::Display for Instance<'tcx> {
                 write!(f, " - intrinsic")
             }
             InstanceDef::Virtual(_, num) => {
-                write!(f, " - shim(#{})", num)
+                write!(f, " - virtual#{}", num)
             }
             InstanceDef::FnPtrShim(_, ty) => {
                 write!(f, " - shim({:?})", ty)
@@ -311,20 +254,23 @@ impl<'tcx> Instance<'tcx> {
         substs: SubstsRef<'tcx>,
     ) -> Option<Instance<'tcx>> {
         debug!("resolve(def_id={:?}, substs={:?})", def_id, substs);
-        Instance::resolve(tcx, param_env, def_id, substs).map(|resolved| {
+        Instance::resolve(tcx, param_env, def_id, substs).map(|mut resolved| {
             let has_track_caller = |def| tcx.codegen_fn_attrs(def).flags
                 .contains(CodegenFnAttrFlags::TRACK_CALLER);
 
             match resolved.def {
                 InstanceDef::Item(def_id) if has_track_caller(def_id) => {
                     debug!(" => fn pointer created for function with #[track_caller]");
-                    Instance {
-                        def: InstanceDef::ReifyShim(def_id),
-                        substs,
-                    }
-                },
-                _ => resolved,
+                    resolved.def = InstanceDef::ReifyShim(def_id);
+                }
+                InstanceDef::Virtual(def_id, _) => {
+                    debug!(" => fn pointer created for virtual call");
+                    resolved.def = InstanceDef::ReifyShim(def_id);
+                }
+                _ => {}
             }
+
+            resolved
         })
     }
 

src/librustc/ty/layout.rs

@@ -2339,6 +2339,76 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for LayoutError<'tcx> {
     }
 }
 
+
+impl<'tcx> ty::Instance<'tcx> {
+    // NOTE(eddyb) this is private to avoid using it from outside of
+    // `FnAbi::of_instance` - any other uses are either too high-level
+    // for `Instance` (e.g. typeck would use `Ty::fn_sig` instead),
+    // or should go through `FnAbi` instead, to avoid losing any
+    // adjustments `FnAbi::of_instance` might be performing.
+    fn fn_sig_for_fn_abi(&self, tcx: TyCtxt<'tcx>) -> ty::PolyFnSig<'tcx> {
+        let ty = self.ty(tcx);
+        match ty.kind {
+            ty::FnDef(..) |
+            // Shims currently have type FnPtr. Not sure this should remain.
+            ty::FnPtr(_) => {
+                let mut sig = ty.fn_sig(tcx);
+                if let ty::InstanceDef::VtableShim(..) = self.def {
+                    // Modify `fn(self, ...)` to `fn(self: *mut Self, ...)`.
+                    sig = sig.map_bound(|mut sig| {
+                        let mut inputs_and_output = sig.inputs_and_output.to_vec();
+                        inputs_and_output[0] = tcx.mk_mut_ptr(inputs_and_output[0]);
+                        sig.inputs_and_output = tcx.intern_type_list(&inputs_and_output);
+                        sig
+                    });
+                }
+                sig
+            }
+            ty::Closure(def_id, substs) => {
+                let sig = substs.as_closure().sig(def_id, tcx);
+
+                let env_ty = tcx.closure_env_ty(def_id, substs).unwrap();
+                sig.map_bound(|sig| tcx.mk_fn_sig(
+                    iter::once(*env_ty.skip_binder()).chain(sig.inputs().iter().cloned()),
+                    sig.output(),
+                    sig.c_variadic,
+                    sig.unsafety,
+                    sig.abi
+                ))
+            }
+            ty::Generator(def_id, substs, _) => {
+                let sig = substs.as_generator().poly_sig(def_id, tcx);
+
+                let env_region = ty::ReLateBound(ty::INNERMOST, ty::BrEnv);
+                let env_ty = tcx.mk_mut_ref(tcx.mk_region(env_region), ty);
+
+                let pin_did = tcx.lang_items().pin_type().unwrap();
+                let pin_adt_ref = tcx.adt_def(pin_did);
+                let pin_substs = tcx.intern_substs(&[env_ty.into()]);
+                let env_ty = tcx.mk_adt(pin_adt_ref, pin_substs);
+
+                sig.map_bound(|sig| {
+                    let state_did = tcx.lang_items().gen_state().unwrap();
+                    let state_adt_ref = tcx.adt_def(state_did);
+                    let state_substs = tcx.intern_substs(&[
+                        sig.yield_ty.into(),
+                        sig.return_ty.into(),
+                    ]);
+                    let ret_ty = tcx.mk_adt(state_adt_ref, state_substs);
+
+                    tcx.mk_fn_sig(iter::once(env_ty),
+                        ret_ty,
+                        false,
+                        hir::Unsafety::Normal,
+                        rustc_target::spec::abi::Abi::Rust
+                    )
+                })
+            }
+            _ => bug!("unexpected type {:?} in Instance::fn_sig", ty)
+        }
+    }
+}
+
 pub trait FnAbiExt<'tcx, C>
 where
     C: LayoutOf<Ty = Ty<'tcx>, TyLayout = TyLayout<'tcx>>
@@ -2347,12 +2417,22 @@ where
         + HasTyCtxt<'tcx>
         + HasParamEnv<'tcx>,
 {
-    fn of_instance(cx: &C, instance: ty::Instance<'tcx>) -> Self;
-    fn new(cx: &C, sig: ty::FnSig<'tcx>, extra_args: &[Ty<'tcx>]) -> Self;
-    fn new_vtable(cx: &C, sig: ty::FnSig<'tcx>, extra_args: &[Ty<'tcx>]) -> Self;
+    /// Compute a `FnAbi` suitable for indirect calls, i.e. to `fn` pointers.
+    ///
+    /// NB: this doesn't handle virtual calls - those should use `FnAbi::of_instance`
+    /// instead, where the instance is a `InstanceDef::Virtual`.
+    fn of_fn_ptr(cx: &C, sig: ty::PolyFnSig<'tcx>, extra_args: &[Ty<'tcx>]) -> Self;
+
+    /// Compute a `FnAbi` suitable for declaring/defining an `fn` instance, and for
+    /// direct calls to an `fn`.
+    ///
+    /// NB: that includes virtual calls, which are represented by "direct calls"
+    /// to a `InstanceDef::Virtual` instance (of `<dyn Trait as Trait>::fn`).
+    fn of_instance(cx: &C, instance: ty::Instance<'tcx>, extra_args: &[Ty<'tcx>]) -> Self;
+
     fn new_internal(
         cx: &C,
-        sig: ty::FnSig<'tcx>,
+        sig: ty::PolyFnSig<'tcx>,
         extra_args: &[Ty<'tcx>],
         mk_arg_type: impl Fn(Ty<'tcx>, Option<usize>) -> ArgAbi<'tcx, Ty<'tcx>>,
     ) -> Self;
@@ -2367,25 +2447,19 @@ where
         + HasTyCtxt<'tcx>
         + HasParamEnv<'tcx>,
 {
-    fn of_instance(cx: &C, instance: ty::Instance<'tcx>) -> Self {
-        let sig = instance.fn_sig(cx.tcx());
-        let sig = cx
-            .tcx()
-            .normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), &sig);
-        call::FnAbi::new(cx, sig, &[])
-    }
-
-    fn new(cx: &C, sig: ty::FnSig<'tcx>, extra_args: &[Ty<'tcx>]) -> Self {
+    fn of_fn_ptr(cx: &C, sig: ty::PolyFnSig<'tcx>, extra_args: &[Ty<'tcx>]) -> Self {
         call::FnAbi::new_internal(cx, sig, extra_args, |ty, _| ArgAbi::new(cx.layout_of(ty)))
     }
 
-    fn new_vtable(cx: &C, sig: ty::FnSig<'tcx>, extra_args: &[Ty<'tcx>]) -> Self {
-        FnAbiExt::new_internal(cx, sig, extra_args, |ty, arg_idx| {
+    fn of_instance(cx: &C, instance: ty::Instance<'tcx>, extra_args: &[Ty<'tcx>]) -> Self {
+        let sig = instance.fn_sig_for_fn_abi(cx.tcx());
+
+        call::FnAbi::new_internal(cx, sig, extra_args, |ty, arg_idx| {
             let mut layout = cx.layout_of(ty);
             // Don't pass the vtable, it's not an argument of the virtual fn.
             // Instead, pass just the data pointer, but give it the type `*const/mut dyn Trait`
             // or `&/&mut dyn Trait` because this is special-cased elsewhere in codegen
-            if arg_idx == Some(0) {
+            if let (ty::InstanceDef::Virtual(..), Some(0)) = (&instance.def, arg_idx) {
                 let fat_pointer_ty = if layout.is_unsized() {
                     // unsized `self` is passed as a pointer to `self`
                     // FIXME (mikeyhew) change this to use &own if it is ever added to the language
@@ -2436,15 +2510,19 @@ where
 
     fn new_internal(
         cx: &C,
-        sig: ty::FnSig<'tcx>,
+        sig: ty::PolyFnSig<'tcx>,
         extra_args: &[Ty<'tcx>],
         mk_arg_type: impl Fn(Ty<'tcx>, Option<usize>) -> ArgAbi<'tcx, Ty<'tcx>>,
     ) -> Self {
         debug!("FnAbi::new_internal({:?}, {:?})", sig, extra_args);
 
+        let sig = cx
+            .tcx()
+            .normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), &sig);
+
         use rustc_target::spec::abi::Abi::*;
         let conv = match cx.tcx().sess.target.target.adjust_abi(sig.abi) {
-            RustIntrinsic | PlatformIntrinsic | Rust | RustCall => Conv::C,
+            RustIntrinsic | PlatformIntrinsic | Rust | RustCall => Conv::Rust,
 
             // It's the ABI's job to select this, not ours.
             System => bug!("system abi should be selected elsewhere"),

src/librustc_codegen_llvm/abi.rs

@@ -372,7 +372,7 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> {
 
     fn llvm_cconv(&self) -> llvm::CallConv {
         match self.conv {
-            Conv::C => llvm::CCallConv,
+            Conv::C | Conv::Rust => llvm::CCallConv,
             Conv::AmdGpuKernel => llvm::AmdGpuKernel,
             Conv::ArmAapcs => llvm::ArmAapcsCallConv,
             Conv::Msp430Intr => llvm::Msp430Intr,
@@ -388,6 +388,11 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> {
     }
 
     fn apply_attrs_llfn(&self, cx: &CodegenCx<'ll, 'tcx>, llfn: &'ll Value) {
+        // FIXME(eddyb) can this also be applied to callsites?
+        if self.ret.layout.abi.is_uninhabited() {
+            llvm::Attribute::NoReturn.apply_llfn(llvm::AttributePlace::Function, llfn);
+        }
+
         let mut i = 0;
         let mut apply = |attrs: &ArgAttributes, ty: Option<&Type>| {
             attrs.apply_llfn(llvm::AttributePlace::Argument(i), llfn, ty);